From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 5E1D31B49C for ; Fri, 4 Jan 2019 14:27:40 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BEDB83DBEC; Fri, 4 Jan 2019 13:27:39 +0000 (UTC) Received: from ktraynor.remote.csb (ovpn-117-13.ams2.redhat.com [10.36.117.13]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9E93F5C1A1; Fri, 4 Jan 2019 13:27:38 +0000 (UTC) From: Kevin Traynor To: Anatoly Burakov Cc: dpdk stable Date: Fri, 4 Jan 2019 13:24:25 +0000 Message-Id: <20190104132455.15170-43-ktraynor@redhat.com> In-Reply-To: <20190104132455.15170-1-ktraynor@redhat.com> References: <20190104132455.15170-1-ktraynor@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Fri, 04 Jan 2019 13:27:39 +0000 (UTC) Subject: [dpdk-stable] patch 'malloc: fix deadlock when reading stats' has been queued to LTS release 18.11.1 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 04 Jan 2019 13:27:41 -0000 Hi, FYI, your patch has been queued to LTS release 18.11.1 Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet. It will be pushed if I get no objections before 01/11/19. So please shout if anyone has objections. Also note that after the patch there's a diff of the upstream commit vs the patch applied to the branch. This will indicate if there was any rebasing needed to apply to the stable branch. If there were code changes for rebasing (ie: not only metadata diffs), please double check that the rebase was correctly done. Thanks. Kevin Traynor --- >>From ed54c0902f56a47e1d269824f543852fdac1796f Mon Sep 17 00:00:00 2001 From: Anatoly Burakov Date: Fri, 21 Dec 2018 12:26:05 +0000 Subject: [PATCH] malloc: fix deadlock when reading stats [ upstream commit ba731ea1dda3f1a1b7eb4248d5988de746d3ef0a ] 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") Signed-off-by: Anatoly Burakov --- lib/librte_eal/common/include/rte_malloc.h | 9 +++++++++ lib/librte_eal/common/rte_malloc.c | 19 +++---------------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/lib/librte_eal/common/include/rte_malloc.h b/lib/librte_eal/common/include/rte_malloc.h index a5290b074..54a12467a 100644 --- a/lib/librte_eal/common/include/rte_malloc.h +++ b/lib/librte_eal/common/include/rte_malloc.h @@ -252,4 +252,7 @@ rte_malloc_validate(const void *ptr, size_t *size); * Get heap statistics for the specified heap. * + * @note This function is not thread-safe with respect to + * ``rte_malloc_heap_create()``/``rte_malloc_heap_destroy()`` functions. + * * @param socket * An unsigned integer specifying the socket to get heap statistics for @@ -462,4 +465,7 @@ rte_malloc_heap_socket_is_external(int socket_id); * NULL, all memory types will be dumped. * + * @note This function is not thread-safe with respect to + * ``rte_malloc_heap_create()``/``rte_malloc_heap_destroy()`` functions. + * * @param f * A pointer to a file for output @@ -474,4 +480,7 @@ rte_malloc_dump_stats(FILE *f, const char *type); * Dump contents of all malloc heaps to a file. * + * @note This function is not thread-safe with respect to + * ``rte_malloc_heap_create()``/``rte_malloc_heap_destroy()`` functions. + * * @param f * A pointer to a file for output diff --git a/lib/librte_eal/common/rte_malloc.c b/lib/librte_eal/common/rte_malloc.c index 06cf1e666..47c2bec72 100644 --- a/lib/librte_eal/common/rte_malloc.c +++ b/lib/librte_eal/common/rte_malloc.c @@ -157,18 +157,12 @@ rte_malloc_get_socket_stats(int socket, { struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config; - int heap_idx, ret = -1; - - rte_rwlock_read_lock(&mcfg->memory_hotplug_lock); + int heap_idx; heap_idx = malloc_socket_to_heap_id(socket); if (heap_idx < 0) - goto unlock; + return -1; - ret = malloc_heap_get_stats(&mcfg->malloc_heaps[heap_idx], + return malloc_heap_get_stats(&mcfg->malloc_heaps[heap_idx], socket_stats); -unlock: - rte_rwlock_read_unlock(&mcfg->memory_hotplug_lock); - - return ret; } @@ -182,12 +176,8 @@ rte_malloc_dump_heaps(FILE *f) unsigned int idx; - rte_rwlock_read_lock(&mcfg->memory_hotplug_lock); - for (idx = 0; idx < RTE_MAX_HEAPS; idx++) { fprintf(f, "Heap id: %u\n", idx); malloc_heap_dump(&mcfg->malloc_heaps[idx], f); } - - rte_rwlock_read_unlock(&mcfg->memory_hotplug_lock); } @@ -263,6 +253,4 @@ rte_malloc_dump_stats(FILE *f, __rte_unused const char *type) struct rte_malloc_socket_stats sock_stats; - rte_rwlock_read_lock(&mcfg->memory_hotplug_lock); - /* Iterate through all initialised heaps */ for (heap_id = 0; heap_id < RTE_MAX_HEAPS; heap_id++) { @@ -281,5 +269,4 @@ rte_malloc_dump_stats(FILE *f, __rte_unused const char *type) fprintf(f, "\tFree_count:%u,\n", sock_stats.free_count); } - rte_rwlock_read_unlock(&mcfg->memory_hotplug_lock); return; } -- 2.19.0 --- Diff of the applied patch vs upstream commit (please double-check if non-empty: --- --- - 2019-01-04 13:23:08.478061537 +0000 +++ 0043-malloc-fix-deadlock-when-reading-stats.patch 2019-01-04 13:23:07.000000000 +0000 @@ -1,8 +1,10 @@ -From ba731ea1dda3f1a1b7eb4248d5988de746d3ef0a Mon Sep 17 00:00:00 2001 +From ed54c0902f56a47e1d269824f543852fdac1796f Mon Sep 17 00:00:00 2001 From: Anatoly Burakov Date: Fri, 21 Dec 2018 12:26:05 +0000 Subject: [PATCH] malloc: fix deadlock when reading stats +[ upstream commit ba731ea1dda3f1a1b7eb4248d5988de746d3ef0a ] + 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 @@ -31,28 +33,13 @@ 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 --- - doc/guides/rel_notes/release_19_02.rst | 4 ++++ lib/librte_eal/common/include/rte_malloc.h | 9 +++++++++ lib/librte_eal/common/rte_malloc.c | 19 +++---------------- - 3 files changed, 16 insertions(+), 16 deletions(-) + 2 files changed, 12 insertions(+), 16 deletions(-) -diff --git a/doc/guides/rel_notes/release_19_02.rst b/doc/guides/rel_notes/release_19_02.rst -index 47768288a..0b248d55d 100644 ---- a/doc/guides/rel_notes/release_19_02.rst -+++ b/doc/guides/rel_notes/release_19_02.rst -@@ -127,4 +127,8 @@ API Changes - fd's (such as in-memory or no-huge mode) - -+* eal: Functions ``rte_malloc_dump_stats()``, ``rte_malloc_dump_heaps()`` and -+ ``rte_malloc_get_socket_stats()`` are no longer safe to call concurrently with -+ ``rte_malloc_heap_create()`` or ``rte_malloc_heap_destroy()`` function calls. -+ - * pdump: The ``rte_pdump_set_socket_dir()``, the parameter ``path`` of - ``rte_pdump_init()`` and enum ``rte_pdump_socktype`` were deprecated diff --git a/lib/librte_eal/common/include/rte_malloc.h b/lib/librte_eal/common/include/rte_malloc.h index a5290b074..54a12467a 100644 --- a/lib/librte_eal/common/include/rte_malloc.h @@ -82,7 +69,7 @@ * @param f * A pointer to a file for output diff --git a/lib/librte_eal/common/rte_malloc.c b/lib/librte_eal/common/rte_malloc.c -index 09051c236..b39de3c99 100644 +index 06cf1e666..47c2bec72 100644 --- a/lib/librte_eal/common/rte_malloc.c +++ b/lib/librte_eal/common/rte_malloc.c @@ -157,18 +157,12 @@ rte_malloc_get_socket_stats(int socket,