[dpdk-stable] patch 'malloc: fix deadlock when reading stats' has been queued to LTS release 18.11.1

[Kevin Traynor]

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 <anatoly.burakov at intel.com>
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 <anatoly.burakov at intel.com>
---
 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 <anatoly.burakov at intel.com>
 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 at dpdk.org
 
 Signed-off-by: Anatoly Burakov <anatoly.burakov at intel.com>
 ---
- 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,