From: Thomas Monjalon <thomas@monjalon.net>
To: Anatoly Burakov <anatoly.burakov@intel.com>
Cc: dev@dpdk.org, stable@dpdk.org, ferruh.yigit@intel.com,
bruce.richardson@intel.com, arybchenko@solarflare.com,
olivier.matz@6wind.com
Subject: Re: [PATCH 18.11] malloc: fix deadlock when using malloc stats
Date: Fri, 21 Dec 2018 13:09:23 +0100 [thread overview]
Message-ID: <8322983.QTS3aZyW4V@xps> (raw)
In-Reply-To: <884a355bde19652d57c253c6b36036571c4f46ee.1543926108.git.anatoly.burakov@intel.com>
04/12/2018 13:22, Anatoly Burakov:
> Currently, malloc statistics and external heap creation code
> use memory hotplug lock as a way to synchronize accesses to
> heaps (as in, locking the hotplug lock to prevent list of heaps
> from changing under our feet). At the same time, malloc
> statistics code will also lock the heap because it needs to
> access heap data and does not want any other thread to allocate
> anything from that heap.
>
> In such scheme, it is possible to enter a deadlock with the
> following sequence of events:
>
> thread 1 thread 2
> rte_malloc()
> rte_malloc_dump_stats()
> take heap lock
> take hotplug lock
> failed to allocate,
> attempt to take
> hotplug lock
> attempt to take heap lock
>
> Neither thread will be able to continue, as both of them are
> waiting for the other one to drop the lock. Adding an
> additional lock will require an ABI change, so instead of
> that, make malloc statistics calls thread-unsafe with
> respect to creating/destroying heaps.
>
> Fixes: 72cf92b31855 ("malloc: index heaps using heap ID rather than NUMA node")
> Cc: stable@dpdk.org
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
>
> Notes:
> IMO this is the best we can do for 18.11 without breaking ABI.
> For 19.02, we can introduce a new global heap lock (something
> i should've done in the first place...), so this patch is
> not applicable to 19.02. For 19.02, we can fix this properly
> by introducing another lock and breaking the EAL ABI.
>
> Not sure where to put docs update, feedback welcome.
This patch is also changing the API, because functions become not thread-safe.
I think you should note it in the release notes.
About 19.02, do we want to take this patch (with release notes updated)?
next prev parent reply other threads:[~2018-12-21 12:09 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-04 12:22 [PATCH 18.11] malloc: fix deadlock when using malloc stats Anatoly Burakov
2018-12-21 12:09 ` Thomas Monjalon [this message]
2018-12-21 12:12 ` Burakov, Anatoly
2018-12-21 12:26 ` [PATCH 19.02 v2] " Anatoly Burakov
2018-12-21 13:35 ` Thomas Monjalon
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=8322983.QTS3aZyW4V@xps \
--to=thomas@monjalon.net \
--cc=anatoly.burakov@intel.com \
--cc=arybchenko@solarflare.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=olivier.matz@6wind.com \
--cc=stable@dpdk.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.