On Mon, Aug 05, 2024 at 07:35:22AM -0700, Darrick J. Wong wrote:
> On Wed, Jul 31, 2024 at 12:55:57PM +0200, Peter Zijlstra wrote:
> > 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.
>
> Hi Peter,
>
> This morning, I noticed the splat below with -rc2.
>
> WARNING: CPU: 0 PID: 8578 at kernel/jump_label.c:295 __static_key_slow_dec_cpuslocked.part.0+0x50/0x60
>
> Line 295 is the else branch of this code:
>
> if (atomic_cmpxchg(&key->enabled, 1, 0) == 1)
> jump_label_update(key);
> else
> WARN_ON_ONCE(!static_key_slow_try_dec(key));
>
> Apparently static_key_slow_try_dec returned false? Looking at that
> function, I suppose the atomic_read of key->enabled returned 0, since it
> didn't trigger the "WARN_ON_ONCE(v < 0)" code. Does that mean the value
> must have dropped from positive N to 0 without anyone ever taking the
> jump_label_mutex?
One possible scenario I see:
slow_dec
if (try_dec) // dec_not_one-ish, false
// enabled == 1
slow_inc
if (inc_not_disabled) // inc_not_zero-ish
// enabled == 2
return
guard((mutex)(&jump_label_mutex);
if (atomic_cmpxchg(1,0)==1) // false, we're 2
slow_dec
if (try-dec) // dec_not_one, true
// enabled == 1
return
else
try_dec() // dec_not_one, false
WARN
Let me go play to see how best to cure this.
> Unfortunately I'm a little too covfid-brained to figure this out today.
> :(
Urgh, brain-fog is the worst :/
