Messages in this thread Patch in this message | | | From | Jakub Kicinski <> | Subject | [PATCH] locking/static_key: Fix false positive warnings on concurrent dec/inc | Date | Mon, 18 Mar 2019 14:58:14 -0700 |
| |
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.kicinski@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
| |