patch 'mem: fix deadlock with multiprocess' has been queued to stable release 22.11.4

Xueming Li xuemingl at nvidia.com
Sun Oct 22 16:20:51 CEST 2023


Hi,

FYI, your patch has been queued to stable release 22.11.4

Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
It will be pushed if I get no objections before 11/15/23. 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.

Queued patches are on a temporary branch at:
https://git.dpdk.org/dpdk-stable/log/?h=22.11-staging

This queued commit can be viewed at:
https://git.dpdk.org/dpdk-stable/commit/?h=22.11-staging&id=421c47495c84a17da80e90777da557315f946c2d

Thanks.

Xueming Li <xuemingl at nvidia.com>

---
>From 421c47495c84a17da80e90777da557315f946c2d Mon Sep 17 00:00:00 2001
From: Artemy Kovalyov <artemyko at nvidia.com>
Date: Fri, 8 Sep 2023 16:17:35 +0300
Subject: [PATCH] mem: fix deadlock with multiprocess
Cc: Xueming Li <xuemingl at nvidia.com>

[ upstream commit f82c02d3791ca1cd4ea1baa549a7d25dc2aef767 ]

The issue arose due to changes in the DPDK read-write lock
implementation. Following these changes, the RW-lock no longer supports
recursion, implying that a single thread shouldn't obtain a read lock if
it already possesses one. The problem arises during initialization: the
rte_eal_init() function acquires the memory_hotplug_lock, and later on,
there are sequences of calls that acquire it again without releasing it.
* rte_eal_memory_init() -> eal_memalloc_init() -> rte_memseg_list_walk()
* rte_eal_memory_init() -> rte_eal_hugepage_init() ->
  eal_dynmem_hugepage_init() -> rte_memseg_list_walk()
This scenario introduces the risk of a potential deadlock when concurrent
write locks are applied to the same memory_hotplug_lock. To address this
locally, we resolved the issue by replacing rte_memseg_list_walk() with
rte_memseg_list_walk_thread_unsafe().

Bugzilla ID: 1277
Fixes: 832cecc03d77 ("rwlock: prevent readers from starving writers")

Signed-off-by: Artemy Kovalyov <artemyko at nvidia.com>
Acked-by: Dmitry Kozlyuk <dmitry.kozliuk at gmail.com>
Tested-by: Jonathan Erb <jonathan.erb at threatblockr.com>
---
 .mailmap                             | 3 ++-
 lib/eal/common/eal_common_dynmem.c   | 5 ++++-
 lib/eal/include/generic/rte_rwlock.h | 4 ++++
 lib/eal/linux/eal_memalloc.c         | 7 +++++--
 4 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/.mailmap b/.mailmap
index e2855454c2..361e2dac4f 100644
--- a/.mailmap
+++ b/.mailmap
@@ -120,6 +120,7 @@ Arkadiusz Kubalewski <arkadiusz.kubalewski at intel.com>
 Arkadiusz Kusztal <arkadiuszx.kusztal at intel.com>
 Arnon Warshavsky <arnon at qwilt.com>
 Arshdeep Kaur <arshdeep.kaur at intel.com>
+Artemy Kovalyov <artemyko at nvidia.com>
 Artem V. Andreev <artem.andreev at oktetlabs.ru>
 Artur Rojek <ar at semihalf.com>
 Artur Trybula <arturx.trybula at intel.com>
@@ -649,7 +650,7 @@ John Ousterhout <ouster at cs.stanford.edu>
 John Romein <romein at astron.nl>
 John W. Linville <linville at tuxdriver.com>
 Jonas Pfefferle <jpf at zurich.ibm.com> <pepperjo at japf.ch>
-Jonathan Erb <jonathan.erb at banduracyber.com>
+Jonathan Erb <jonathan.erb at threatblockr.com> <jonathan.erb at banduracyber.com>
 Jon DeVree <nuxi at vault24.org>
 Jon Loeliger <jdl at netgate.com>
 Joongi Kim <joongi at an.kaist.ac.kr>
diff --git a/lib/eal/common/eal_common_dynmem.c b/lib/eal/common/eal_common_dynmem.c
index bdbbe233a0..95da55d9b0 100644
--- a/lib/eal/common/eal_common_dynmem.c
+++ b/lib/eal/common/eal_common_dynmem.c
@@ -251,7 +251,10 @@ eal_dynmem_hugepage_init(void)
 		 */
 		memset(&dummy, 0, sizeof(dummy));
 		dummy.hugepage_sz = hpi->hugepage_sz;
-		if (rte_memseg_list_walk(hugepage_count_walk, &dummy) < 0)
+		/*  memory_hotplug_lock is held during initialization, so it's
+		 *  safe to call thread-unsafe version.
+		 */
+		if (rte_memseg_list_walk_thread_unsafe(hugepage_count_walk, &dummy) < 0)
 			return -1;

 		for (i = 0; i < RTE_DIM(dummy.num_pages); i++) {
diff --git a/lib/eal/include/generic/rte_rwlock.h b/lib/eal/include/generic/rte_rwlock.h
index 233d4262be..e479daa867 100644
--- a/lib/eal/include/generic/rte_rwlock.h
+++ b/lib/eal/include/generic/rte_rwlock.h
@@ -79,6 +79,10 @@ rte_rwlock_init(rte_rwlock_t *rwl)
 /**
  * Take a read lock. Loop until the lock is held.
  *
+ * @note The RW lock isn't recursive, so calling this function on the same
+ * lock twice without releasing it could potentially result in a deadlock
+ * scenario when a write lock is involved.
+ *
  * @param rwl
  *   A pointer to a rwlock structure.
  */
diff --git a/lib/eal/linux/eal_memalloc.c b/lib/eal/linux/eal_memalloc.c
index f8b1588cae..9853ec78a2 100644
--- a/lib/eal/linux/eal_memalloc.c
+++ b/lib/eal/linux/eal_memalloc.c
@@ -1740,7 +1740,10 @@ eal_memalloc_init(void)
 		eal_get_internal_configuration();

 	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
-		if (rte_memseg_list_walk(secondary_msl_create_walk, NULL) < 0)
+		/*  memory_hotplug_lock is held during initialization, so it's
+		 *  safe to call thread-unsafe version.
+		 */
+		if (rte_memseg_list_walk_thread_unsafe(secondary_msl_create_walk, NULL) < 0)
 			return -1;
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY &&
 			internal_conf->in_memory) {
@@ -1778,7 +1781,7 @@ eal_memalloc_init(void)
 	}

 	/* initialize all of the fd lists */
-	if (rte_memseg_list_walk(fd_list_create_walk, NULL))
+	if (rte_memseg_list_walk_thread_unsafe(fd_list_create_walk, NULL))
 		return -1;
 	return 0;
 }
--
2.25.1

---
  Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- -	2023-10-22 22:17:35.254253700 +0800
+++ 0022-mem-fix-deadlock-with-multiprocess.patch	2023-10-22 22:17:34.156723700 +0800
@@ -1 +1 @@
-From f82c02d3791ca1cd4ea1baa549a7d25dc2aef767 Mon Sep 17 00:00:00 2001
+From 421c47495c84a17da80e90777da557315f946c2d Mon Sep 17 00:00:00 2001
@@ -4,0 +5,3 @@
+Cc: Xueming Li <xuemingl at nvidia.com>
+
+[ upstream commit f82c02d3791ca1cd4ea1baa549a7d25dc2aef767 ]
@@ -22 +24,0 @@
-Cc: stable at dpdk.org
@@ -35 +37 @@
-index 755a4da2cd..bcb36bb5a5 100644
+index e2855454c2..361e2dac4f 100644
@@ -38 +40,2 @@
-@@ -126,6 +126,7 @@ Arnaud Fiorini <arnaud.fiorini at polymtl.ca>
+@@ -120,6 +120,7 @@ Arkadiusz Kubalewski <arkadiusz.kubalewski at intel.com>
+ Arkadiusz Kusztal <arkadiuszx.kusztal at intel.com>
@@ -41 +43,0 @@
- Artemii Morozov <artemii.morozov at arknetworks.am>
@@ -46 +48 @@
-@@ -667,7 +668,7 @@ John Ousterhout <ouster at cs.stanford.edu>
+@@ -649,7 +650,7 @@ John Ousterhout <ouster at cs.stanford.edu>
@@ -72 +74 @@
-index 75f2b75782..5f939be98c 100644
+index 233d4262be..e479daa867 100644
@@ -75 +77 @@
-@@ -81,6 +81,10 @@ rte_rwlock_init(rte_rwlock_t *rwl)
+@@ -79,6 +79,10 @@ rte_rwlock_init(rte_rwlock_t *rwl)


More information about the stable mailing list