On Tue, Jul 30, 2024 at 10:33:41PM -0700, Darrick J. Wong wrote: > Sooooo... it turns out that somehow your patch got mismerged on the > first go-round, and that worked. The second time, there was no > mismerge, which mean that the wrong atomic_cmpxchg() callsite was > tested. > > Looking back at the mismerge, it actually changed > __static_key_slow_dec_cpuslocked, which had in 6.10: > > if (atomic_dec_and_test(&key->enabled)) > jump_label_update(key); > > Decrement, then return true if the value was set to zero. With the 6.11 > code, it looks like we want to exchange a 1 with a 0, and act only if > the previous value had been 1. > > So perhaps we really want this change? I'll send it out to the fleet > and we'll see what it reports tomorrow morning. Bah yes, I missed we had it twice. Definitely both sites want this. I'll tentatively merge the below patch in tip/locking/urgent. I can rebase if there is need. --- Subject: jump_label: Fix the fix, brown paper bags galore From: Peter Zijlstra <peterz@xxxxxxxxxxxxx> Date: Wed Jul 31 12:43:21 CEST 2024 Per the example of: !atomic_cmpxchg(&key->enabled, 0, 1) the inverse was written as: atomic_cmpxchg(&key->enabled, 1, 0) except of course, that while !old is only true for old == 0, old is true for everything except old == 0. Fix it to read: atomic_cmpxchg(&key->enabled, 1, 0) == 1 such that only the 1->0 transition returns true and goes on to disable the keys. Fixes: 83ab38ef0a0b ("jump_label: Fix concurrency issues in static_key_slow_dec()") Reported-by: Darrick J. Wong <djwong@xxxxxxxxxx> Tested-by: Darrick J. Wong <djwong@xxxxxxxxxx> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx> --- kernel/jump_label.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) --- a/kernel/jump_label.c +++ b/kernel/jump_label.c @@ -236,7 +236,7 @@ void static_key_disable_cpuslocked(struc } jump_label_lock(); - if (atomic_cmpxchg(&key->enabled, 1, 0)) + if (atomic_cmpxchg(&key->enabled, 1, 0) == 1) jump_label_update(key); jump_label_unlock(); } @@ -289,7 +289,7 @@ static void __static_key_slow_dec_cpuslo return; guard(mutex)(&jump_label_mutex); - if (atomic_cmpxchg(&key->enabled, 1, 0)) + if (atomic_cmpxchg(&key->enabled, 1, 0) == 1) jump_label_update(key); else WARN_ON_ONCE(!static_key_slow_try_dec(key));