From: David Marchand <david.marchand@redhat.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>
Subject: Re: [dpdk-dev] [PATCH v4] eal: fix proc type auto detection
Date: Sun, 27 Oct 2019 20:41:05 +0100 [thread overview]
Message-ID: <CAJFAV8wWrjZAtpfYJ4gt1opx6O54EhWQf7YoQdHTDx5R6vhCUA@mail.gmail.com> (raw)
In-Reply-To: <27c0cf41-2dea-9e0e-7c25-df76796ca2e6@intel.com>
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 end.
>
> 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=flock&sektion=2
> [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.
--
David Marchand
prev parent reply other threads:[~2019-10-27 19:41 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-23 13:19 [dpdk-dev] [PATCH] " Anatoly Burakov
2019-07-23 18:38 ` Stephen Hemminger
2019-07-24 10:04 ` [dpdk-dev] [PATCH v2] " Anatoly Burakov
2019-07-24 16:04 ` [dpdk-dev] [PATCH v3] " Anatoly Burakov
2019-07-24 16:07 ` [dpdk-dev] [PATCH v4] " Anatoly Burakov
2019-07-30 8:13 ` Thomas Monjalon
2019-07-30 9:19 ` Burakov, Anatoly
2019-08-12 10:03 ` David Marchand
2019-08-12 10:21 ` Van Haaren, Harry
2019-08-12 13:30 ` Burakov, Anatoly
2019-08-12 13:31 ` Burakov, Anatoly
2019-10-24 16:07 ` Burakov, Anatoly
2019-10-27 19:41 ` David Marchand [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAJFAV8wWrjZAtpfYJ4gt1opx6O54EhWQf7YoQdHTDx5R6vhCUA@mail.gmail.com \
--to=david.marchand@redhat.com \
--cc=anatoly.burakov@intel.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=stable@dpdk.org \
--cc=stephen@networkplumber.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).