From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <stable-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id C3791A00B8
	for <public@inbox.dpdk.org>; Sun, 27 Oct 2019 20:41:20 +0100 (CET)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id B6E471BEF5;
	Sun, 27 Oct 2019 20:41:20 +0100 (CET)
Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com
 [207.211.31.120]) by dpdk.org (Postfix) with ESMTP id 7B72B1BEF5
 for <stable@dpdk.org>; Sun, 27 Oct 2019 20:41:19 +0100 (CET)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
 s=mimecast20190719; t=1572205278;
 h=from:from:reply-to:subject:subject:date:date:message-id:message-id:
 to:to:cc:cc:mime-version:mime-version:content-type:content-type:
 content-transfer-encoding:content-transfer-encoding:
 in-reply-to:in-reply-to:references:references;
 bh=EDAG7TU/dhnShlsLB23vmnF2uW4VAC50iJfJGRLJjC8=;
 b=UwJxqfzty6Q2/WtswKNlEK4pwB8pS7k+dHdkhhs2GmuwrJoF28F1dtgdxnXtUpNmpkiI04
 4BHMBbGWOJvd1ZfSiIBO3e/O2FU5aWfKW7wBhRku5w/QZWewyNZWe3gg9aN7qS4fioCoT/
 SSgXzH5UFl2hhiYGD9c1BL3/Q4FKYWc=
Received: from mail-ua1-f69.google.com (mail-ua1-f69.google.com
 [209.85.222.69]) (Using TLS) by relay.mimecast.com with ESMTP id
 us-mta-303-hA_JwvTHOFmDNJf6UZHgNg-1; Sun, 27 Oct 2019 15:41:17 -0400
Received: by mail-ua1-f69.google.com with SMTP id y4so1238481uaa.17
 for <stable@dpdk.org>; Sun, 27 Oct 2019 12:41:17 -0700 (PDT)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:mime-version:references:in-reply-to:from:date
 :message-id:subject:to:cc;
 bh=1JK/7+39M5/oia18nzpktG05YXp66qwD0Nfq5TBYuMA=;
 b=CDgN1PlWbtDJLXCi8wg+TD4vfVo3D370OkOSqAivSLmjdDtz+taYixl2dGVlXpxrDk
 pYg5V90NSx96sQmjNWJC+i4uZnE/T6u7ssRLdcsK4om2CYkJq2prA823MTMYIocwYIw0
 7XSkO7sJT/hrrOgV8oWGSENpsePn9Dk4OJHDBloWda9EGa4mXGRxrLqdW3Oil/l3ft1u
 m25QJzXbzLoNlYlGc8HjxIgMNbNBbRtUrHsq8y5NCB9QNDER4PKnr8X6NZ8XVBlMe7bs
 EW51OMZh92LKyPdn7NtcfhLe8egiMJgkP0oY3VhG5CcJueaODBpEN169oJmbL6tI5DIY
 oOsA==
X-Gm-Message-State: APjAAAWmYEmq20xfzbxlxsSZGql1NEOYWPXrJVNH2omQh4Cc/sZ0AIxR
 gWg1yBRR3FvY/Fcptc5uHGzQhr2lt8X28OxZBCuDlqTvEwEpcpO8VE/p2BAuv/E3Urm6JhyY8ii
 J/KBVuT4AiLqJglh0fOUUYN4=
X-Received: by 2002:a67:7790:: with SMTP id s138mr3092307vsc.198.1572205276751; 
 Sun, 27 Oct 2019 12:41:16 -0700 (PDT)
X-Google-Smtp-Source: APXvYqw5sPSju52TueOkNwrm2zgStuPSTfPXW9u9tALxp3zTWz4dxtXXkE0C7GiiNMVG5Uz6aDQwcFKiBU4ZpWuwoPc=
X-Received: by 2002:a67:7790:: with SMTP id s138mr3092292vsc.198.1572205276392; 
 Sun, 27 Oct 2019 12:41:16 -0700 (PDT)
MIME-Version: 1.0
References: <e76dcac7fe4ecd9f588253950b6496990581440a.1563984251.git.anatoly.burakov@intel.com>
 <67a795e77bc9f5ac79ab78a878ae19abbead9f50.1563984454.git.anatoly.burakov@intel.com>
 <CAJFAV8w1i_6v7oA01ETJgWcwSWh2m=+8jj+M7cn_BE36GXKbOg@mail.gmail.com>
 <27c0cf41-2dea-9e0e-7c25-df76796ca2e6@intel.com>
In-Reply-To: <27c0cf41-2dea-9e0e-7c25-df76796ca2e6@intel.com>
From: David Marchand <david.marchand@redhat.com>
Date: Sun, 27 Oct 2019 20:41:05 +0100
Message-ID: <CAJFAV8wWrjZAtpfYJ4gt1opx6O54EhWQf7YoQdHTDx5R6vhCUA@mail.gmail.com>
To: "Burakov, Anatoly" <anatoly.burakov@intel.com>
Cc: dev <dev@dpdk.org>, Bruce Richardson <bruce.richardson@intel.com>, 
 Stephen Hemminger <stephen@networkplumber.org>, dpdk stable <stable@dpdk.org>
X-MC-Unique: hA_JwvTHOFmDNJf6UZHgNg-1
X-Mimecast-Spam-Score: 0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH v4] eal: fix proc type auto
	detection
X-BeenThere: stable@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches for DPDK stable branches <stable.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/stable>,
 <mailto:stable-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/stable/>
List-Post: <mailto:stable@dpdk.org>
List-Help: <mailto:stable-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/stable>,
 <mailto:stable-request@dpdk.org?subject=subscribe>
Errors-To: stable-bounces@dpdk.org
Sender: "stable" <stable-bounces@dpdk.org>

On Thu, Oct 24, 2019 at 6:07 PM Burakov, Anatoly
<anatoly.burakov@intel.com> wrote:
>
> On 12-Aug-19 11:03 AM, David Marchand wrote:
> > On Wed, Jul 24, 2019 at 6:08 PM Anatoly Burakov
> > <anatoly.burakov@intel.com> wrote:
> >>
> >> Currently, primary process holds an exclusive lock on the config
> >> file, thereby preventing other primaries from spinning up. However,
> >> when the primary dies, the lock is no longer being held, even though
> >> there might be other secondary processes still running.
> >>
> >> The fix is two-fold. First of all, downgrade the primary process's
> >> exclusive lock to a shared lock once we have it. Second of all,
> >> also take out shared locks on the config from the secondaries. We
> >> are using fcntl() locks, which get dropped when the file handle is
> >> closed, so also remove the closure of config file handle.
> >>
> >> Fixes: af75078fece3 ("first public release")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> >> ---
> >>
>
> Hi David,
>
> I've been investigating how to improve this patch, and i've hit a dead en=
d.
>
> The problems here are two-fold. Using fcntl() and flock() locks together
> is not advisable, so both primary-secondary detection and
> rte_eal_primary_proc_alive() (as per Harry's point) have to use the same
> method for checking locks.
>
> Using flock() would work for this purpose. Unfortunately, on FreeBSD,
> converting exclusive lock into a shared lock involves unlocking first
> [1] (creating a race). On Linux it doesn't specifically say that it does
> that, but it does mention that it is not guaranteed to be atomic [2].
> So, we can't use flock() here.
>
> It seems that fcntl() lock conversions are atomic, however fcntl() locks
> on both Linux and FreeBSD are implemented in a very stupid way in that
> /any/ closure of a file descriptor drops /all/ locks on that file.
> Meaning, the moment secondary does the check in primary_proc_alive() and
> closes the config file fd, the process-wide lock drops. Mind you,
> primary_proc_alive() is implemented using lockf() rather than fcntl(),
> which is an issue in itself, but on Linux, lockf() is implemented on top
> of fcntl() locks and thus suffers from the same issue.
>
> So, unless you have better ideas, i think this patch can be marked as
> rejected.
>
> [1] https://www.freebsd.org/cgi/man.cgi?query=3Dflock&sektion=3D2
> [2] https://linux.die.net/man/2/flock

Sorry, hard to digest, I would need to look at this again later.
If you have no easy solution, let's revisit after 19.11.

Thanks Anatoly.


--=20
David Marchand