From ba731ea1dda3f1a1b7eb4248d5988de746d3ef0a 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 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 --- 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(-) diff --git a/doc/guides/rel_notes/release_19_02.rst b/doc/guides/rel_notes/release_19_02.rst index 47768288a3..0b248d55d3 100644 --- a/doc/guides/rel_notes/release_19_02.rst +++ b/doc/guides/rel_notes/release_19_02.rst @@ -126,6 +126,10 @@ API Changes - In cases where memfd support would have been required to provide segment 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 since 18.05 and are removed in this release. diff --git a/lib/librte_eal/common/include/rte_malloc.h b/lib/librte_eal/common/include/rte_malloc.h index a5290b0740..54a12467a4 100644 --- a/lib/librte_eal/common/include/rte_malloc.h +++ b/lib/librte_eal/common/include/rte_malloc.h @@ -251,6 +251,9 @@ 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 * @param socket_stats @@ -461,6 +464,9 @@ rte_malloc_heap_socket_is_external(int socket_id); * Dump for the specified type to a file. If the type argument is * 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 * @param type @@ -473,6 +479,9 @@ 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 09051c2362..b39de3c99e 100644 --- a/lib/librte_eal/common/rte_malloc.c +++ b/lib/librte_eal/common/rte_malloc.c @@ -156,20 +156,14 @@ rte_malloc_get_socket_stats(int socket, struct rte_malloc_socket_stats *socket_stats) { 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; } /* @@ -181,14 +175,10 @@ rte_malloc_dump_heaps(FILE *f) struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config; 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); } int @@ -262,8 +252,6 @@ rte_malloc_dump_stats(FILE *f, __rte_unused const char *type) unsigned int heap_id; 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++) { struct malloc_heap *heap = &mcfg->malloc_heaps[heap_id]; @@ -280,7 +268,6 @@ rte_malloc_dump_stats(FILE *f, __rte_unused const char *type) fprintf(f, "\tAlloc_count:%u,\n",sock_stats.alloc_count); fprintf(f, "\tFree_count:%u,\n", sock_stats.free_count); } - rte_rwlock_read_unlock(&mcfg->memory_hotplug_lock); return; } -- 2.20.1