2024-03-05 08:03:56

by Sam Sun

[permalink] [raw]
Subject: [Bug] WARNING in static_key_disable_cpuslocked

Dear developers and maintainers,

We encountered a kernel warning while using our modified Syzkaller. It
is tested against linux kernel 6.8-rc7. Kernel config and C repro are
attached to this email. The kernel log is listed below.

```
WARNING: CPU: 0 PID: 8286 at kernel/jump_label.c:229
static_key_disable_cpuslocked+0x15e/0x1c0 kernel/jump_label.c:229
Modules linked in:
CPU: 0 PID: 8286 Comm: syz-executor393 Not tainted 6.7.0-rc7 #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
RIP: 0010:static_key_disable_cpuslocked+0x15e/0x1c0 kernel/jump_label.c:229
Code: d3 ff 48 c7 c7 a0 cc 0a 8d e8 5e d3 b6 08 5b 5d e9 f7 bd d3 ff
e8 f2 bd d3 ff 48 89 ef e8 2a f8 ff ff eb d9 e8 e3 bd d3 ff 90 <0f> 0b
90 eb 8f e8 d8 bd d3 ff 90 48 c7 c6 00 74 b7 8a 48 89 ea 48
RSP: 0018:ffffc90002c4faf0 EFLAGS: 00010293
RAX: 0000000000000000 RBX: 00000000ffffffff RCX: ffffffff81b1568e
RDX: ffff888013b5d980 RSI: ffffffff81b156fd RDI: 0000000000000005
RBP: ffffffff8d0c75d0 R08: 0000000000000005 R09: 0000000000000000
R10: 00000000ffffffff R11: 0000000000000000 R12: ffffffff8d0c75d0
R13: ffff8880209fb980 R14: 0000000000000000 R15: ffffffff8211e160
FS: 0000555555e0e3c0(0000) GS:ffff888063a00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f49e210e090 CR3: 000000001a2d4000 CR4: 0000000000750ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
PKRU: 55555554
Call Trace:
<TASK>
static_key_disable+0x1a/0x20 kernel/jump_label.c:243
enabled_store+0x288/0x2040 mm/vmscan.c:5137
kobj_attr_store+0x54/0x80 lib/kobject.c:836
sysfs_kf_write+0x114/0x170 fs/sysfs/file.c:136
kernfs_fop_write_iter+0x337/0x500 fs/kernfs/file.c:334
call_write_iter include/linux/fs.h:2020 [inline]
new_sync_write fs/read_write.c:491 [inline]
vfs_write+0x96a/0xd80 fs/read_write.c:584
ksys_write+0x122/0x250 fs/read_write.c:637
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0x40/0x110 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x63/0x6b
RIP: 0033:0x7f49e208a7bd
Code: c3 e8 a7 1f 00 00 0f 1f 80 00 00 00 00 f3 0f 1e fa 48 89 f8 48
89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d
01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffe25660128 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 00007f49e20db078 RCX: 00007f49e208a7bd
RDX: 0000000000000002 RSI: 0000000020000080 RDI: 0000000000000004
RBP: 0000000000000000 R08: 00007f49e20dfb55 R09: 00007f49e20dfb55
R10: 00007f49e20dfb55 R11: 0000000000000246 R12: 00000000000207f3
R13: 00007ffe2566013c R14: 00007ffe25660150 R15: 00007ffe25660140
</TASK>
```

We analyzed the cause of this bug. It seems that in function
static_key_disable_cpuslocked(), there is a small racing window
between two atomic_read(&key->enabled) in line 228 & 229, triggering
the WARN_ON_ONCE macro. This might cause function returned without
actually disabling the static_key "key". I am not sure if there is any
other potential threat here.

I searched on web and found that there was a similar bug reported by
syzbot several years ago
(https://groups.google.com/g/syzkaller-bugs/c/_W3lgRCwlb4/m/TqzyQcPpAQAJ).
At that time the fix was in the net instead of kernel/jump_label.c. So
I checked the call stack and cc this email to memory management
maintainers. Hope there is no confusion.

If you have any questions, please contact us.
Reported by: Yue Sun <[email protected]>
Reported by: xingwei lee <[email protected]>

Best Regards,
Yue


Attachments:
config (240.82 kB)
static_key_disable_cpuslocked.c (3.96 kB)
Download all attachments

2024-03-06 15:52:43

by Steven Rostedt

[permalink] [raw]
Subject: Re: [Bug] WARNING in static_key_disable_cpuslocked

On Tue, 5 Mar 2024 15:54:24 +0800
Sam Sun <[email protected]> wrote:

> We analyzed the cause of this bug. It seems that in function
> static_key_disable_cpuslocked(), there is a small racing window
> between two atomic_read(&key->enabled) in line 228 & 229, triggering
> the WARN_ON_ONCE macro. This might cause function returned without
> actually disabling the static_key "key". I am not sure if there is any
> other potential threat here.
>
> I searched on web and found that there was a similar bug reported by
> syzbot several years ago
> (https://groups.google.com/g/syzkaller-bugs/c/_W3lgRCwlb4/m/TqzyQcPpAQAJ).
> At that time the fix was in the net instead of kernel/jump_label.c. So
> I checked the call stack and cc this email to memory management
> maintainers. Hope there is no confusion.
>
> If you have any questions, please contact us.
> Reported by: Yue Sun <[email protected]>
> Reported by: xingwei lee <[email protected]>

Thanks for the report. I wonder if it simply needs to add the tests in the
locking? Like the patch below.

Because I could see:

CPU 0 CPU 1
----- -----

key->enabled = 0

static_key_enable_cpus_locked()
jump_label_lock();
static_key_disable_cpus_locked()
if (key->enabled != 1) {

key->enabled = 1;
WARN_ON(key->enabled != 0)

Now I guess the question is, why is something trying to disable something
that is not enabled? Is the above scenario OK? Or should the users of
static_key also prevent this?

-- Steve


diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index d9c822bbffb8..f154caf2a62b 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -194,12 +194,12 @@ void static_key_enable_cpuslocked(struct static_key *key)
STATIC_KEY_CHECK_USE(key);
lockdep_assert_cpus_held();

+ jump_label_lock();
if (atomic_read(&key->enabled) > 0) {
WARN_ON_ONCE(atomic_read(&key->enabled) != 1);
- return;
+ goto unlock;
}

- jump_label_lock();
if (atomic_read(&key->enabled) == 0) {
atomic_set(&key->enabled, -1);
jump_label_update(key);
@@ -208,6 +208,7 @@ void static_key_enable_cpuslocked(struct static_key *key)
*/
atomic_set_release(&key->enabled, 1);
}
+unlock:
jump_label_unlock();
}
EXPORT_SYMBOL_GPL(static_key_enable_cpuslocked);
@@ -225,14 +226,15 @@ void static_key_disable_cpuslocked(struct static_key *key)
STATIC_KEY_CHECK_USE(key);
lockdep_assert_cpus_held();

+ jump_label_lock();
if (atomic_read(&key->enabled) != 1) {
WARN_ON_ONCE(atomic_read(&key->enabled) != 0);
- return;
+ goto unlock;
}

- jump_label_lock();
if (atomic_cmpxchg(&key->enabled, 1, 0))
jump_label_update(key);
+unlock:
jump_label_unlock();
}
EXPORT_SYMBOL_GPL(static_key_disable_cpuslocked);

2024-03-06 19:31:14

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [Bug] WARNING in static_key_disable_cpuslocked

On Wed, Mar 06, 2024 at 10:54:20AM -0500, Steven Rostedt wrote:
> Now I guess the question is, why is something trying to disable something
> that is not enabled? Is the above scenario OK? Or should the users of
> static_key also prevent this?

Apparently that's an allowed scenario, as the jump label code seems to
be actively trying to support it. Basically the last one "wins".

See for example:

1dbb6704de91 ("jump_label: Fix concurrent static_key_enable/disable()")

Also the purpose of the first atomic_read() is to do a quick test before
grabbing the jump lock. So instead of grabbing the jump lock earlier,
it should actually do the first test atomically:

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index d9c822bbffb8..f29c47930d46 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -191,11 +191,14 @@ EXPORT_SYMBOL_GPL(static_key_slow_inc);

void static_key_enable_cpuslocked(struct static_key *key)
{
+ int tmp;
+
STATIC_KEY_CHECK_USE(key);
lockdep_assert_cpus_held();

- if (atomic_read(&key->enabled) > 0) {
- WARN_ON_ONCE(atomic_read(&key->enabled) != 1);
+ tmp = atomic_read(&key->enabled);
+ if (tmp != 0) {
+ WARN_ON_ONCE(tmp != 1);
return;
}

@@ -222,11 +225,14 @@ EXPORT_SYMBOL_GPL(static_key_enable);

void static_key_disable_cpuslocked(struct static_key *key)
{
+ int tmp;
+
STATIC_KEY_CHECK_USE(key);
lockdep_assert_cpus_held();

- if (atomic_read(&key->enabled) != 1) {
- WARN_ON_ONCE(atomic_read(&key->enabled) != 0);
+ tmp = atomic_read(&key->enabled);
+ if (tmp != 1) {
+ WARN_ON_ONCE(tmp != 0);
return;
}


2024-03-06 21:14:41

by Jason Baron

[permalink] [raw]
Subject: Re: [Bug] WARNING in static_key_disable_cpuslocked



On 3/6/24 2:31 PM, Josh Poimboeuf wrote:
> On Wed, Mar 06, 2024 at 10:54:20AM -0500, Steven Rostedt wrote:
>> Now I guess the question is, why is something trying to disable something
>> that is not enabled? Is the above scenario OK? Or should the users of
>> static_key also prevent this?
>
> Apparently that's an allowed scenario, as the jump label code seems to
> be actively trying to support it. Basically the last one "wins".
>
> See for example:
>
> 1dbb6704de91 ("jump_label: Fix concurrent static_key_enable/disable()")
>
> Also the purpose of the first atomic_read() is to do a quick test before
> grabbing the jump lock. So instead of grabbing the jump lock earlier,
> it should actually do the first test atomically:

Makes sense but the enable path can also set key->enabled to -1. So I
think a concurrent disable could then see the -1 in tmp and still
trigger the WARN. So I think we could change the WARN to be:
WARN_ON_ONCE(tmp != 0 && tmp != -1). And also add a similar check
for enable if we have enable vs enable racing?

Although it seems like the set key->enabled to -1 while used in the
inc/dec API isn't really doing anything in the enable/disable part here?
But then the key->enabled I think has to move in front of the
jump_label_update() to make that part work right...

Thanks,

-Jason


>
> diff --git a/kernel/jump_label.c b/kernel/jump_label.c
> index d9c822bbffb8..f29c47930d46 100644
> --- a/kernel/jump_label.c
> +++ b/kernel/jump_label.c
> @@ -191,11 +191,14 @@ EXPORT_SYMBOL_GPL(static_key_slow_inc);
>
> void static_key_enable_cpuslocked(struct static_key *key)
> {
> + int tmp;
> +
> STATIC_KEY_CHECK_USE(key);
> lockdep_assert_cpus_held();
>
> - if (atomic_read(&key->enabled) > 0) {
> - WARN_ON_ONCE(atomic_read(&key->enabled) != 1);
> + tmp = atomic_read(&key->enabled);
> + if (tmp != 0) {
> + WARN_ON_ONCE(tmp != 1);
> return;
> }
>
> @@ -222,11 +225,14 @@ EXPORT_SYMBOL_GPL(static_key_enable);
>
> void static_key_disable_cpuslocked(struct static_key *key)
> {
> + int tmp;
> +
> STATIC_KEY_CHECK_USE(key);
> lockdep_assert_cpus_held();
>
> - if (atomic_read(&key->enabled) != 1) {
> - WARN_ON_ONCE(atomic_read(&key->enabled) != 0);
> + tmp = atomic_read(&key->enabled);
> + if (tmp != 1) {
> + WARN_ON_ONCE(tmp != 0);
> return;
> }
>

2024-03-06 22:16:59

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [Bug] WARNING in static_key_disable_cpuslocked

On Wed, Mar 06, 2024 at 03:12:07PM -0500, Jason Baron wrote:
>
>
> On 3/6/24 2:31 PM, Josh Poimboeuf wrote:
> > On Wed, Mar 06, 2024 at 10:54:20AM -0500, Steven Rostedt wrote:
> > > Now I guess the question is, why is something trying to disable something
> > > that is not enabled? Is the above scenario OK? Or should the users of
> > > static_key also prevent this?
> >
> > Apparently that's an allowed scenario, as the jump label code seems to
> > be actively trying to support it. Basically the last one "wins".
> >
> > See for example:
> >
> > 1dbb6704de91 ("jump_label: Fix concurrent static_key_enable/disable()")
> >
> > Also the purpose of the first atomic_read() is to do a quick test before
> > grabbing the jump lock. So instead of grabbing the jump lock earlier,
> > it should actually do the first test atomically:
>
> Makes sense but the enable path can also set key->enabled to -1.

Ah, this code is really subtle :-/

> So I think a concurrent disable could then see the -1 in tmp and still
> trigger the WARN.

I think this shouldn't be possible, for the same reason that
static_key_slow_try_dec() warns on -1: key->enabled can only be -1
during the first enable. And disable should never be called before
then.

> So I think we could change the WARN to be:
> WARN_ON_ONCE(tmp != 0 && tmp != -1). And also add a similar check
> for enable if we have enable vs enable racing?

My patch subtly changed the "key->enabled > 0" to "key->enabled != 0".
If I change that back then it should be fine.

> Although it seems like the set key->enabled to -1 while used in the inc/dec
> API isn't really doing anything in the enable/disable part here?
> But then the key->enabled I think has to move in front of the
> jump_label_update() to make that part work right...

Yeah, this code needs better comments. Let me turn it into a proper
patch.

--
Josh

2024-03-06 23:44:19

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [Bug] WARNING in static_key_disable_cpuslocked

On Wed, Mar 06, 2024 at 05:40:11PM -0500, Jason Baron wrote:
>
>
> On 3/6/24 5:16 PM, Josh Poimboeuf wrote:
> > On Wed, Mar 06, 2024 at 03:12:07PM -0500, Jason Baron wrote:
> > >
> > >
> > > On 3/6/24 2:31 PM, Josh Poimboeuf wrote:
> > > > On Wed, Mar 06, 2024 at 10:54:20AM -0500, Steven Rostedt wrote:
> > > > > Now I guess the question is, why is something trying to disable something
> > > > > that is not enabled? Is the above scenario OK? Or should the users of
> > > > > static_key also prevent this?
> > > >
> > > > Apparently that's an allowed scenario, as the jump label code seems to
> > > > be actively trying to support it. Basically the last one "wins".
> > > >
> > > > See for example:
> > > >
> > > > 1dbb6704de91 ("jump_label: Fix concurrent static_key_enable/disable()")
> > > >
> > > > Also the purpose of the first atomic_read() is to do a quick test before
> > > > grabbing the jump lock. So instead of grabbing the jump lock earlier,
> > > > it should actually do the first test atomically:
> > >
> > > Makes sense but the enable path can also set key->enabled to -1.
> >
> > Ah, this code is really subtle :-/
> >
> > > So I think a concurrent disable could then see the -1 in tmp and still
> > > trigger the WARN.
> >
> > I think this shouldn't be possible, for the same reason that
> > static_key_slow_try_dec() warns on -1: key->enabled can only be -1
> > during the first enable. And disable should never be called before
> > then.
>
> hmm, right but I think in this case the reproducer is writing to a sysfs
> file to enable/disable randomly so i'm not sure if there is anything that
> would enforce that ordering. I guess you could try the reproducer, I haven't
> really looked at it in any detail.
>
> The code in question here is in mm/vmscan.c which actually already takes the
> local 'state_mutex' for some cases. So that could be extended I think easily
> to avoid this warning.

Hm, right... For now I'll just continue to allow "disable before enable"
(or "double disable") since it may be harmless and I don't want to
introduce any unnecessary constraints, unless we manage to convince
ourselves that it's the right thing to do.

I'll work up a patch.

--
Josh

2024-03-07 00:06:20

by Jason Baron

[permalink] [raw]
Subject: Re: [Bug] WARNING in static_key_disable_cpuslocked



On 3/6/24 5:16 PM, Josh Poimboeuf wrote:
> On Wed, Mar 06, 2024 at 03:12:07PM -0500, Jason Baron wrote:
>>
>>
>> On 3/6/24 2:31 PM, Josh Poimboeuf wrote:
>>> On Wed, Mar 06, 2024 at 10:54:20AM -0500, Steven Rostedt wrote:
>>>> Now I guess the question is, why is something trying to disable something
>>>> that is not enabled? Is the above scenario OK? Or should the users of
>>>> static_key also prevent this?
>>>
>>> Apparently that's an allowed scenario, as the jump label code seems to
>>> be actively trying to support it. Basically the last one "wins".
>>>
>>> See for example:
>>>
>>> 1dbb6704de91 ("jump_label: Fix concurrent static_key_enable/disable()")
>>>
>>> Also the purpose of the first atomic_read() is to do a quick test before
>>> grabbing the jump lock. So instead of grabbing the jump lock earlier,
>>> it should actually do the first test atomically:
>>
>> Makes sense but the enable path can also set key->enabled to -1.
>
> Ah, this code is really subtle :-/
>
>> So I think a concurrent disable could then see the -1 in tmp and still
>> trigger the WARN.
>
> I think this shouldn't be possible, for the same reason that
> static_key_slow_try_dec() warns on -1: key->enabled can only be -1
> during the first enable. And disable should never be called before
> then.

hmm, right but I think in this case the reproducer is writing to a sysfs
file to enable/disable randomly so i'm not sure if there is anything
that would enforce that ordering. I guess you could try the reproducer,
I haven't really looked at it in any detail.

The code in question here is in mm/vmscan.c which actually already takes
the local 'state_mutex' for some cases. So that could be extended I
think easily to avoid this warning.

>
>> So I think we could change the WARN to be:
>> WARN_ON_ONCE(tmp != 0 && tmp != -1). And also add a similar check
>> for enable if we have enable vs enable racing?
>
> My patch subtly changed the "key->enabled > 0" to "key->enabled != 0".
> If I change that back then it should be fine.
>
>> Although it seems like the set key->enabled to -1 while used in the inc/dec
>> API isn't really doing anything in the enable/disable part here?
>> But then the key->enabled I think has to move in front of the
>> jump_label_update() to make that part work right...
>
> Yeah, this code needs better comments. Let me turn it into a proper
> patch.
>

2024-03-07 01:30:31

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [Bug] WARNING in static_key_disable_cpuslocked

On Wed, Mar 06, 2024 at 03:42:55PM -0800, Josh Poimboeuf wrote:
> On Wed, Mar 06, 2024 at 05:40:11PM -0500, Jason Baron wrote:
> > On 3/6/24 5:16 PM, Josh Poimboeuf wrote:
> > > On Wed, Mar 06, 2024 at 03:12:07PM -0500, Jason Baron wrote:
> > > > On 3/6/24 2:31 PM, Josh Poimboeuf wrote:
> > > > > On Wed, Mar 06, 2024 at 10:54:20AM -0500, Steven Rostedt wrote:
> > > > > > Now I guess the question is, why is something trying to disable something
> > > > > > that is not enabled? Is the above scenario OK? Or should the users of
> > > > > > static_key also prevent this?
> > > > >
> > > > > Apparently that's an allowed scenario, as the jump label code seems to
> > > > > be actively trying to support it. Basically the last one "wins".
> > > > >
> > > > > See for example:
> > > > >
> > > > > 1dbb6704de91 ("jump_label: Fix concurrent static_key_enable/disable()")
> > > > >
> > > > > Also the purpose of the first atomic_read() is to do a quick test before
> > > > > grabbing the jump lock. So instead of grabbing the jump lock earlier,
> > > > > it should actually do the first test atomically:
> > > >
> > > > Makes sense but the enable path can also set key->enabled to -1.
> > >
> > > Ah, this code is really subtle :-/
> > >
> > > > So I think a concurrent disable could then see the -1 in tmp and still
> > > > trigger the WARN.
> > >
> > > I think this shouldn't be possible, for the same reason that
> > > static_key_slow_try_dec() warns on -1: key->enabled can only be -1
> > > during the first enable. And disable should never be called before
> > > then.
> >
> > hmm, right but I think in this case the reproducer is writing to a sysfs
> > file to enable/disable randomly so i'm not sure if there is anything that
> > would enforce that ordering. I guess you could try the reproducer, I haven't
> > really looked at it in any detail.
> >
> > The code in question here is in mm/vmscan.c which actually already takes the
> > local 'state_mutex' for some cases. So that could be extended I think easily
> > to avoid this warning.
>
> Hm, right... For now I'll just continue to allow "disable before enable"
> (or "double disable") since it may be harmless and I don't want to
> introduce any unnecessary constraints, unless we manage to convince
> ourselves that it's the right thing to do.

So, I think we can simplify this nicely by getting rid of the whole -1
thing altogether:

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index d9c822bbffb8..ef7eda7685b2 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -194,20 +194,15 @@ void static_key_enable_cpuslocked(struct static_key *key)
STATIC_KEY_CHECK_USE(key);
lockdep_assert_cpus_held();

- if (atomic_read(&key->enabled) > 0) {
- WARN_ON_ONCE(atomic_read(&key->enabled) != 1);
+ if (atomic_read(&key->enabled) == 1)
return;
- }
-
jump_label_lock();
- if (atomic_read(&key->enabled) == 0) {
- atomic_set(&key->enabled, -1);
+
+ if (atomic_cmpxchg(&key->enabled, 0, 1) == 0)
jump_label_update(key);
- /*
- * See static_key_slow_inc().
- */
- atomic_set_release(&key->enabled, 1);
- }
+ else
+ WARN_ON_ONCE(atomic_read(&key->enabled) != 1);
+
jump_label_unlock();
}
EXPORT_SYMBOL_GPL(static_key_enable_cpuslocked);
@@ -225,14 +220,16 @@ void static_key_disable_cpuslocked(struct static_key *key)
STATIC_KEY_CHECK_USE(key);
lockdep_assert_cpus_held();

- if (atomic_read(&key->enabled) != 1) {
- WARN_ON_ONCE(atomic_read(&key->enabled) != 0);
+ if (atomic_read(&key->enabled) == 0)
return;
- }

jump_label_lock();
- if (atomic_cmpxchg(&key->enabled, 1, 0))
+
+ if (atomic_cmpxchg(&key->enabled, 1, 0) == 1)
jump_label_update(key);
+ else
+ WARN_ON_ONCE(atomic_read(&key->enabled) != 0);
+
jump_label_unlock();
}
EXPORT_SYMBOL_GPL(static_key_disable_cpuslocked);

2024-03-07 02:33:10

by Steven Rostedt

[permalink] [raw]
Subject: Re: [Bug] WARNING in static_key_disable_cpuslocked

On Wed, 6 Mar 2024 17:30:09 -0800
Josh Poimboeuf <[email protected]> wrote:

> So, I think we can simplify this nicely by getting rid of the whole -1
> thing altogether:
>
> diff --git a/kernel/jump_label.c b/kernel/jump_label.c
> index d9c822bbffb8..ef7eda7685b2 100644
> --- a/kernel/jump_label.c
> +++ b/kernel/jump_label.c
> @@ -194,20 +194,15 @@ void static_key_enable_cpuslocked(struct static_key *key)
> STATIC_KEY_CHECK_USE(key);
> lockdep_assert_cpus_held();
>
> - if (atomic_read(&key->enabled) > 0) {
> - WARN_ON_ONCE(atomic_read(&key->enabled) != 1);
> + if (atomic_read(&key->enabled) == 1)
> return;
> - }
> -
> jump_label_lock();
> - if (atomic_read(&key->enabled) == 0) {
> - atomic_set(&key->enabled, -1);
> +
> + if (atomic_cmpxchg(&key->enabled, 0, 1) == 0)
> jump_label_update(key);
> - /*
> - * See static_key_slow_inc().
> - */
> - atomic_set_release(&key->enabled, 1);
> - }
> + else
> + WARN_ON_ONCE(atomic_read(&key->enabled) != 1);
> +
> jump_label_unlock();

You may be able to clean it up more with:

int tmp;

tmp = atomic_read(&key->enabled);
if (tmp == 1)
return;

jump_label_lock();

if (!tmp && atomic_try_cmpxchg(&key->enabled, &tmp, 1))
jump_label_update(key);
else
WARN_ON_ONCE(tmp != 1);

jump_label_unlock();

;-)

-- Steve


> }
> EXPORT_SYMBOL_GPL(static_key_enable_cpuslocked);
> @@ -225,14 +220,16 @@ void static_key_disable_cpuslocked(struct static_key *key)
> STATIC_KEY_CHECK_USE(key);
> lockdep_assert_cpus_held();
>
> - if (atomic_read(&key->enabled) != 1) {
> - WARN_ON_ONCE(atomic_read(&key->enabled) != 0);
> + if (atomic_read(&key->enabled) == 0)
> return;
> - }
>
> jump_label_lock();
> - if (atomic_cmpxchg(&key->enabled, 1, 0))
> +
> + if (atomic_cmpxchg(&key->enabled, 1, 0) == 1)
> jump_label_update(key);
> + else
> + WARN_ON_ONCE(atomic_read(&key->enabled) != 0);
> +
> jump_label_unlock();
> }
> EXPORT_SYMBOL_GPL(static_key_disable_cpuslocked);