[PATCH] locking/static_key: Fix false positive warnings on concurrent dec/inc

[Jakub Kicinski]

Even though the atomic_dec_and_mutex_lock() in
__static_key_slow_dec_cpuslocked() can never see a negative
value in key->enabled the subsequent sanity check is re-reading
key->enabled, which may have been set to -1 in the meantime by
static_key_slow_inc_cpuslocked().

Instead of using -1 as a "enable in progress" constant use
-0xffff, this way we can still treat smaller negative values
as errors.

Alternatively we could implement atomic_dec_and_mutex_lock_return().

Fixes: 4c5ea0a9cd02 ("locking/static_key: Fix concurrent static_key_slow_inc()")
Signed-off-by: Jakub Kicinski <jakub.kicin...@netronome.com>
---
 kernel/jump_label.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index bad96b476eb6..4a227e70a8f3 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -89,7 +89,7 @@ static void jump_label_update(struct static_key *key);
 int static_key_count(struct static_key *key)
 {
        /*
-        * -1 means the first static_key_slow_inc() is in progress.
+        * -0xffff means the first static_key_slow_inc() is in progress.
         *  static_key_enabled() must return true, so return 1 here.
         */
        int n = atomic_read(&key->enabled);
@@ -125,7 +125,10 @@ void static_key_slow_inc_cpuslocked(struct static_key *key)
 
        jump_label_lock();
        if (atomic_read(&key->enabled) == 0) {
-               atomic_set(&key->enabled, -1);
+               /* Use a large enough negative number so we can still
+                * catch underflow bugs in static_key_slow_dec().
+                */
+               atomic_set(&key->enabled, -0xffff);
                jump_label_update(key);
                /*
                 * Ensure that if the above cmpxchg loop observes our positive
@@ -158,7 +161,7 @@ void static_key_enable_cpuslocked(struct static_key *key)
 
        jump_label_lock();
        if (atomic_read(&key->enabled) == 0) {
-               atomic_set(&key->enabled, -1);
+               atomic_set(&key->enabled, -0xffff);
                jump_label_update(key);
                /*
                 * See static_key_slow_inc().
@@ -208,15 +211,11 @@ static void __static_key_slow_dec_cpuslocked(struct 
static_key *key,
 {
        lockdep_assert_cpus_held();
 
-       /*
-        * The negative count check is valid even when a negative
-        * key->enabled is in use by static_key_slow_inc(); a
-        * __static_key_slow_dec() before the first static_key_slow_inc()
-        * returns is unbalanced, because all other static_key_slow_inc()
-        * instances block while the update is in progress.
-        */
        if (!atomic_dec_and_mutex_lock(&key->enabled, &jump_label_mutex)) {
-               WARN(atomic_read(&key->enabled) < 0,
+               int v;
+
+               v = atomic_read(&key->enabled);
+               WARN(v < 0 && v != -0xffff,
                     "jump label: negative count!\n");
                return;
        }
-- 
2.19.2