Received-SPF: pass (google.com: domain of linux-kernel+bounces-94865-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Wed, 6 Mar 2024 21:34:53 -0500
From: Steven Rostedt <rostedt@goodmis.org>
To: Josh Poimboeuf <jpoimboe@kernel.org>
Cc: Jason Baron <jbaron@akamai.com>, Sam Sun <samsun1006219@gmail.com>,
 linux-kernel@vger.kernel.org, syzkaller@googlegroups.com,
 xrivendell7@gmail.com, ardb@kernel.org, peterz@infradead.org,
 linux-mm@kvack.org, akpm@linux-foundation.org, Paolo Bonzini
 <pbonzini@redhat.com>
Subject: Re: [Bug] WARNING in static_key_disable_cpuslocked
Message-ID: <20240306213453.72f9a925@gandalf.local.home>
In-Reply-To: <20240307013009.erbnug2mopx5yrnf@treble>
References: <CAEkJfYNNZftjpYBpnH4tEnm82orKtQ6SQn9i3sg7YNO-Df3tSQ@mail.gmail.com>
	<20240306105420.6a6bea2c@gandalf.local.home>
	<20240306193101.s2g33o4viqi2azf3@treble>
	<854e523c-c467-47f6-b977-933cbaadeb62@akamai.com>
	<20240306221650.sw3lha7kca2quv63@treble>
	<8f586bd2-c436-4334-92af-762a284e1101@akamai.com>
	<20240306234253.zporv6cypoc7yihs@treble>
	<20240307013009.erbnug2mopx5yrnf@treble>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Wed, 6 Mar 2024 17:30:09 -0800
Josh Poimboeuf <jpoimboe@kernel.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);