Re: [PATCH] sched_ext: Use atomic cpumask_clear_cpu in scx_idle_test_and_clear_cpu()

[Michał Błaszczyk]

Hi Andrea,

On Tue, Sep 1, 2026 at 6:20 PM Andrea Righi <arighi@xxxxxxxxxx> wrote:
> I think the atomic clear makes sense here, but, as sashiko also pointed out, it
> does not fully address the race, because idle_smts is also modified by the
> non-atomic cpumask_andnot() below and cpumask_or() in update_builtin_idle().
>
> > Fixes: 48849271e661 ("sched_ext: idle: Per-node idle cpumasks")
>
> And the race existed way before this commit, the idle SMT tracking has been
> always documented as racy and self-correcting.
>
> This change may still be a best-effort improvement, but the commit message
> should describe it in this way. Did you notice any improvements/benefits with
> some workloads with this patch applied?

I was investigating an automated static analysis report from Sashiko
which flagged the use of __cpumask_clear_cpu() as a concurrency bug
that could lead to lost updates. I mistakenly assumed this specific
call was an isolated oversight and that the other surrounding cpumask_*
operations were properly atomic.

I hadn't initially realized that cpumask_andnot() and cpumask_or() are
also non-atomic bulk operations. After reading your explanation and
looking closer at the code, it's clear to me now that this non-atomic
handling was intentional to avoid lock overhead on the fast path.

Since my original intent was to fix what I incorrectly thought was a
strict logical bug, and not a profiled optimization, I think it makes
the most sense to just drop this patch. Making just this one operation
atomic while the rest of the mask is manipulated non-atomically would
just be inconsistent and add unnecessary overhead.

Thank you for taking the time to look at this and explain the
context to me.

Best,
Michal