Re: [PATCH] sched_ext: Use WRITE_ONCE() for the write side of scx_enable helper pointer

[Andrea Righi <arighi@nvidia.com>]

Hi,

On Fri, Mar 06, 2026 at 06:59:01PM +0800, zhidao su wrote:
> scx_enable() uses double-checked locking to lazily initialize a static
> kthread_worker pointer:
> 
>     if (!READ_ONCE(helper)) {
>         mutex_lock(&helper_mutex);
>         if (!helper) {
>             helper = kthread_run_worker(0, "scx_enable_helper");
>                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>                      plain write -- KCSAN data race
> 
> The outer READ_ONCE() annotates the lockless fast-path read, but the
> write side uses a plain assignment without the matching WRITE_ONCE().
> The KCSAN documentation requires that if one accessor uses READ_ONCE()
> or WRITE_ONCE() on a variable to annotate lock-free access, all other
> accesses must also use the appropriate accessor. A plain write leaves
> the pair incomplete and will trigger KCSAN warnings.
> 
> The error path also has the same issue:
> 
>             helper = NULL;
>             ^^^^^^^^^^
>             plain write -- KCSAN data race
> 
> Fix both plain writes by using WRITE_ONCE() to complete the concurrent
> access annotation and make the code KCSAN-clean.
> 

There should be a:

Fixes: b06ccbabe2506 ("sched_ext: Fix starvation of scx_enable() under fair-class saturation")

Apart than that LGTM.

Acked-by: Andrea Righi <arighi@nvidia.com>

Thanks,
-Andrea

> Signed-off-by: zhidao su <suzhidao@xiaomi.com>
> ---
>  kernel/sched/ext.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index 9a1471ad5ae7..c4ccd685259f 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -5355,9 +5355,9 @@ static int scx_enable(struct sched_ext_ops *ops, struct bpf_link *link)
>  	if (!READ_ONCE(helper)) {
>  		mutex_lock(&helper_mutex);
>  		if (!helper) {
> -			helper = kthread_run_worker(0, "scx_enable_helper");
> +			WRITE_ONCE(helper, kthread_run_worker(0, "scx_enable_helper"));
>  			if (IS_ERR_OR_NULL(helper)) {
> -				helper = NULL;
> +				WRITE_ONCE(helper, NULL);
>  				mutex_unlock(&helper_mutex);
>  				return -ENOMEM;
>  			}
> -- 
> 2.43.0
>