Re: [PATCH 3/4] test-ww_mutex: Handle transient -EDEADLK in test_cycle_work

[Haakon Bugge]

> On 8 Jul 2026, at 21:55, John Stultz <jstultz@xxxxxxxxxx> wrote:
> 
> On Mon, Jun 22, 2026 at 6:22 AM Haakon Bugge <haakon.bugge@xxxxxxxxxx> wrote:

[snip]

>> 
>> This is true for deadlocks. But my main point is that this is *not* a
>> deadlock which requires one of the WW resolution methods. Put it
>> another way, if it *is* a deadlock, my do-while loop would loop
>> forever. It doesn't, and therefore, it isn't a deadlock.
>> 
>> Another question is of course why ww_mutex_lock() returns -EDEADLK
>> when it isn't the case.
> 
> That seems like a solid quetion.

I think the word is _allegedly_ deadlocked.

>>>> Fixes: d1b42b800e5d ("locking/ww_mutex: Add kselftests for resolving ww_mutex cyclic deadlocks")
>>>> Fixes: e4a02ed2aaf4 ("locking/ww_mutex: Fix runtime warning in the WW mutex selftest")
>>>> Signed-off-by: Håkon Bugge <haakon.bugge@xxxxxxxxxx>
>>>> ---
>>>> kernel/locking/test-ww_mutex.c | 4 +++-
>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>> 
>>>> diff --git a/kernel/locking/test-ww_mutex.c b/kernel/locking/test-ww_mutex.c
>>>> index 5a4c92801bdfb..6b29a7a8f5fba 100644
>>>> --- a/kernel/locking/test-ww_mutex.c
>>>> +++ b/kernel/locking/test-ww_mutex.c
>>>> @@ -306,7 +306,9 @@ static void test_cycle_work(struct work_struct *work)
>>>>               err = 0;
>>>>               ww_mutex_unlock(&cycle->a_mutex);
>>>>               ww_mutex_lock_slow(cycle->b_mutex, &ctx);
>>>> -               erra = ww_mutex_lock(&cycle->a_mutex, &ctx);
>>>> +               do {
>>>> +                       erra = ww_mutex_lock(&cycle->a_mutex, &ctx);
>>>> +               } while (erra == -EDEADLK);
>>>>       }
>>>> 
>>> I don't have a clear example in mind, but just trying to grab the same
>>> lock again (especially in a loop with no timeout) seems like it could
>>> open up other problems here.
>> 
>> The do { ... } while (erra == -EDEADLK) loop in test_cycle_work() is
>> terminating under the ww-mutex progress guarantee. The test builds a
>> finite circular dependency where worker N waits on worker N+1’s
>> a_mutex. On -EDEADLK, the worker releases its own a_mutex, acquires
>> b_mutex through the slow path, and retries. Thus each deadlock
>> encounter removes one hold from the cycle before retrying. Since the
>> cycle is finite and every participant follows the same backoff rule,
>> the deadlock chain must eventually collapse; once the conflicting
>> owner releases its lock, the retry of a_mutex returns success and the
>> loop exits.
>> 
> 
> Apologies for leaving you hanging on this one for so long! I've
> unfortuantely not managed to find the time/focus to really sit and
> understand your argument. :(
> 
> That said, I have been running testing with your patches applied, and
> while they do avoid the "cyclic deadlock not resolved, ret[77/93] =
> -35" warnings, I have unfortunately also seen:
> 
> [   75.360933] ------------[ cut here ]------------
> [   75.361860] DEBUG_LOCKS_WARN_ON(ww_ctx->contending_lock)
> [   75.361860] WARNING: CPU: 21 PID: 1045 at
> kernel/locking/ww_mutex.h:444
> __ww_mutex_lock.constprop.0+0x14ca/0x14d0
> [   75.372591] CPU: 21 UID: 0 PID: 1045 Comm: kworker/u256:11 Not
> tainted 6.18.0-00097-g5d455e26497a-dirty #21 PREEMPT(full)
> [   75.379208] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS 1.17.0-debian-1.17.0-1 04/01/2014
> [   75.383205] Workqueue: test-ww_mutex test_cycle_work
> [   75.386624] RIP: 0010:__ww_mutex_lock.constprop.0+0x14ca/0x14d0
> [   75.390165] Code: e8 5b 05 41 ff 85 c0 74 09 83 3d 10 ec 1a 01 00
> 74 03 90 eb d3 90 48 c7 c6 30 b2 c9 82 48 c7 c7 0b 36 c9 82 e8 47 e7
> d8 fe 0
> [   75.401223] RSP: 0000:ffffc90002c57c60 EFLAGS: 00010092
> [   75.404873] RAX: 0000000000000000 RBX: ffff888106c6c250 RCX: 0000000000000004
> [   75.409870] RDX: 0000000000000001 RSI: ffffc90002c57b08 RDI: 00000000ffffffff
> [   75.413929] RBP: ffffc90002c57d30 R08: 00000000ffffbfff R09: ffff8881b8dfffa8
> [   75.417866] R10: 40000000ffffc2a2 R11: 0000000000000001 R12: ffff8881032a8000
> [   75.423377] R13: ffff888106c6c250 R14: ffffc90002a27cd0 R15: ffffc90002c57d70
> [   75.429016] FS:  0000000000000000(0000) GS:ffff888235225000(0000)
> knlGS:0000000000000000
> [   75.433877] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   75.433877] CR2: 0000000000000000 CR3: 000000000344c001 CR4: 0000000000370ef0
> [   75.433877] Call Trace:
> [   75.442881]  <TASK>
> [   75.442881]  ? test_cycle_work+0x274/0x330
> [   75.442881]  ? match_held_lock+0xa5/0xe0
> [   75.447874]  ? ww_mutex_lock+0x3c/0xb0
> [   75.447874]  ww_mutex_lock+0x3c/0xb0
> [   75.447874]  test_cycle_work+0x274/0x330
> [   75.454875]  process_one_work+0x201/0x590
> [   75.457872]  ? lock_is_held_type+0xcd/0x130
> [   75.457872]  worker_thread+0x192/0x350
> [   75.457872]  ? __pfx_worker_thread+0x10/0x10
> [   75.464893]  kthread+0x11c/0x240
> [   75.464929]  ? __pfx_kthread+0x10/0x10
> [   75.466859]  ret_from_fork+0x191/0x1e0
> [   75.466859]  ? __pfx_kthread+0x10/0x10
> [   75.466859]  ret_from_fork_asm+0x1a/0x30
> [   75.473863]  </TASK>
> 
> 
> I've isolated this down to where this patch adds the  "} while (erra
> == -EDEADLK);" loop
> 
> [  667.892200] kworker/-961       4d..2. 600168128us :
> __ww_mutex_lock.constprop.0: JDB: ww_mutex_lock_acquired ww_ctx:
> 0000000032d33208 clearing contending_lock 00000000f00df359 to NULL
> [  667.898234] kworker/-961       4d..2. 600168129us :
> __ww_mutex_lock.constprop.0: JDB: __ww_mutex_kill ww_ctx:
> 0000000032d33208 setting contending_lock to 000000007c555268
> [  667.903876] kworker/-961       4..... 600168130us :
> test_cycle_work: JDB: test_cycle_work ww_mutex_lock hit EDEADLK in
> loop! ctx: 0000000032d33208
> [  667.908993] kworker/-961       4d..2. 600168130us :
> __ww_mutex_lock.constprop.0: JDB: __ww_mutex_kill ERRRR ww_ctx:
> 0000000032d33208 is not NULL! 000000007c555268  trying to set it to
> 000000007c555268
> 
> So while I've not fully gotten my head around the correctness of your
> argument (you indeed may be right), it seems your change runs afoul of
> the DEBUG_WW_MUTEXES checks, so something extra is needed.

Yes, I also see that when running with CONFIG_DEBUG_MUTEXES (using v7.2-rc2). I think this is an allegedly, but not true deadlock. As such, the DEBUG_WW_MUTEXES check does what it is supposed to do.

Instead of retrying the lock acquisition endlessly, I have a recursive helper function in mind, which will be a good citizen and obey the WW rules. Aka, if mutex X is deadlocked, release it, acquire Y using the slow path, retry the acquisition of X, and if deadlocked, call the helper function with X and Y swapped.

This may take a couple of days.


Thxs, Håkon