Re: [PATCH v2 2/2] test-ww_mutex: Fix deadlock in test_cycle_work
Bradley Morgan <[email protected]>
| Newsgroups | org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On 10 August 2026 17:34:31 BST, "Håkon Bugge" <[email protected]> wrote: >When running with N online CPUs, where N is fairly large, let's say >512, a deadlock may happen in test_cycle_work() when running with >N + 1 kernel threads. The reason is that the min_active is too small >in order to let N + 1 worker threads run concurrently. The following >is my analyzes. The test sets up a circular dependency with ww_mutexes >and completions: > > work[1] completes signal[0] > work[2] completes signal[1] > ... > work[511] completes signal[510] > work[512] would complete signal[511] > work[0] completes signal[512] > >Therefore: > > work[0..510] -> blocked in ww_mutex_lock(b_mutex) > work[511] -> blocked in wait_for_completion(signal[511]) > work[512] -> inactive; test_cycle_work() has not started > >Because the last worker thread has not started, worker 511 hangs >forever in wait_for_completion(). > >This bug produces the following splats (slightly edited for better >brevity). This for the worker threads hung in ww_mutex_lock(): > > INFO: task kworker/u2066:1:13658 blocked for more than 123 seconds. > Workqueue: test-ww_mutex test_cycle_work [test_ww_mutex] > Call Trace: > __schedule+0x28e/0x670 > schedule+0x27/0xa0 > schedule_preempt_disabled+0x15/0x30 > __ww_mutex_lock.constprop.0+0x841/0xe00 > test_cycle_work+0x9d/0x150 [test_ww_mutex] > process_one_work+0x196/0x370 > worker_thread+0x1af/0x320 > kthread+0xe3/0x120 > ret_from_fork+0x19e/0x260 > ret_from_fork_asm+0x1a/0x30 > >And this for the single worker thread 511, hung in >wait_for_completion(): > > Workqueue: test-ww_mutex test_cycle_work [test_ww_mutex] > Call Trace: > __schedule+0x28e/0x670 > schedule+0x27/0xa0 > schedule_timeout+0xac/0xf0 > __wait_for_common+0x97/0x1b0 > test_cycle_work+0x80/0x160 [test_ww_mutex] > process_one_work+0x196/0x370 > worker_thread+0x1af/0x320 > kthread+0xe3/0x120 > ret_from_fork+0x19e/0x260 > ret_from_fork_asm+0x1a/0x30 > >We fix this by adjusting the wq's min_active parameter. When >num_online_cpus() has been sampled in run_tests(), we adjust the >{min,max}_active values of the wq, to make sure both are set to ncpus >+ 1 in order to avoid the above deadlock. > >Note that this fix is invariant to num_online_cpus() changing after it >has been sampled, because the RC here is the number of runnable worker >threads vs. threads created, not per se the number of online CPUs. > >Also, in order to avoid exceeding WQ_MAX_ACTIVE, we create cycle_ncpus >and clamp it. We do not want to change ncpus for the other tests, >not affected by this bug. > >Fixes: d1b42b800e5d ("locking/ww_mutex: Add kselftests for resolving ww_mutex cyclic deadlocks") >Signed-off-by: Håkon Bugge <[email protected]> >Reviewed-by: Bradley Morgan <[email protected]> Shouldn't a rb be on top of a sob? > >--- > >v1 -> v2: > * Added Bradley's r-b > * Reworded comment in run_tests() >--- > kernel/locking/test-ww_mutex.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > >diff --git a/kernel/locking/test-ww_mutex.c b/kernel/locking/test-ww_mutex.c >index 47e016a4f4fea..e11f6b869b4d4 100644 >--- a/kernel/locking/test-ww_mutex.c >+++ b/kernel/locking/test-ww_mutex.c >@@ -676,6 +676,7 @@ static int stress(struct ww_class *class, int nlocks, int nthreads, unsigned int > static int run_tests(struct ww_class *class) > { > int ncpus = num_online_cpus(); >+ int cycle_ncpus = min_t(int, ncpus, WQ_MAX_ACTIVE - 1); > int ret, i; > > ret = test_mutex(class); >@@ -696,7 +697,17 @@ static int run_tests(struct ww_class *class) > return ret; > } > >- ret = test_cycle(class, ncpus); >+ /* >+ * test_cycle_work() has a linear dependency which requires >+ * all kernel threads to be run-able at once. With N CPUs and >+ * N + 1 worker threads, deadlock may happen. Hence, adjust >+ * min_active. Raise max first, min_active is clamped to it. >+ * Cap N so that N + 1 doesn't exceed WQ_MAX_ACTIVE. >+ */ >+ workqueue_set_max_active(wq, cycle_ncpus + 1); >+ workqueue_set_min_active(wq, cycle_ncpus + 1); >+ >+ ret = test_cycle(class, cycle_ncpus); > if (ret) > return ret; > > Thanks!