lkml.org 
[lkml]   [2019]   [Mar]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH] locking/static_key: Fix false positive warnings on concurrent dec/inc
Date
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
\
 
 \ /
  Last update: 2019-03-18 22:59    [W:0.288 / U:0.244 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site