[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20240306213453.72f9a925@gandalf.local.home>
Date: Wed, 6 Mar 2024 21:34:53 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Josh Poimboeuf <jpoimboe@...nel.org>
Cc: Jason Baron <jbaron@...mai.com>, Sam Sun <samsun1006219@...il.com>,
linux-kernel@...r.kernel.org, syzkaller@...glegroups.com,
xrivendell7@...il.com, ardb@...nel.org, peterz@...radead.org,
linux-mm@...ck.org, akpm@...ux-foundation.org, Paolo Bonzini
<pbonzini@...hat.com>
Subject: Re: [Bug] WARNING in static_key_disable_cpuslocked
On Wed, 6 Mar 2024 17:30:09 -0800
Josh Poimboeuf <jpoimboe@...nel.org> 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);
Powered by blists - more mailing lists