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; }