Re: [PATCH] selftests/sched_ext: Fix the incorrect logic validation

[Andrea Righi <arighi@nvidia.com>]

Hi Feng,

On Mon, Mar 02, 2026 at 04:41:30PM +0800, Feng Yang wrote:
> On Mon, 2 Mar 2026 08:47:17 +0100, Andrea Righi wrote:
> > On Mon, Mar 02, 2026 at 02:48:51PM +0800, Feng Yang wrote:
> > > From: Feng Yang <yangfeng@kylinos.cn>
> > > 
> > > When __COMPAT_scx_bpf_pick_idle_cpu_node selects an idle CPU,
> > > it reports that the CPU should be marked as busy.
> > > 
> > > Fixes: 5ae5161820e5 ("selftests/sched_ext: Add NUMA-aware scheduler test")
> > > Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
> > > ---
> > >  tools/testing/selftests/sched_ext/numa.bpf.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/tools/testing/selftests/sched_ext/numa.bpf.c b/tools/testing/selftests/sched_ext/numa.bpf.c
> > > index a79d86ed54a1..98423628b05c 100644
> > > --- a/tools/testing/selftests/sched_ext/numa.bpf.c
> > > +++ b/tools/testing/selftests/sched_ext/numa.bpf.c
> > > @@ -44,12 +44,12 @@ s32 BPF_STRUCT_OPS(numa_select_cpu,
> > >  	 */
> > >  	cpu = __COMPAT_scx_bpf_pick_idle_cpu_node(p->cpus_ptr, node,
> > >  					__COMPAT_SCX_PICK_IDLE_IN_NODE);
> > > -	if (cpu < 0)
> > > +	if (cpu < 0) {
> > >  		cpu = __COMPAT_scx_bpf_pick_any_cpu_node(p->cpus_ptr, node,
> > >  						__COMPAT_SCX_PICK_IDLE_IN_NODE);
> > > -
> > > -	if (is_cpu_idle(cpu, node))
> > > -		scx_bpf_error("CPU %d should be marked as busy", cpu);
> > > +		if (is_cpu_idle(cpu, node))
> > > +			scx_bpf_error("CPU %d should be marked as busy", cpu);
> > > +	}
> > 
> > No, this is not correct. The CPU returned by scx_bpf_pick_idle_cpu_node()
> > should be marked as busy at this point (bit is set in the idle cpumask),
> > essentially it has been reserved/allocated by the caller.
> 
> Okay, I misunderstood.
> The CI test of this patch (https://patchwork.kernel.org/project/netdevbpf/list/?series=1058866&state=*) 
> resulted in the error "CPU 0 should be marked as busy"(https://github.com/kernel-patches/bpf/actions/runs/22548310848/job/65314767856),
> but I don't see the connection between them that would cause this error. Could you explain why this happens?

The is_cpu_idle() test is going to be always a bit flaky in some cases,
for what I see we may have two races:

1) if the first call to scx_bpf_pick_idle_cpu_node() returns a negative
   value, we fallback to "pick any CPU". Between selecting the CPU and
   checking if it's idle, the task running on the CPU may have released it,
   allowing the idle task to run and set the idle bit again.

2) Even if the first scx_bpf_pick_idle_cpu_node() returns a cpu >= 0, that
   CPU might still execute a short-lived task, which then releases it and
   sets the idle bit again.

Given this, I think the most reliable approach is to remove the
is_cpu_idle() check here to avoid flakiness and instead implement something
along the lines of the following. What do you think?

Thanks,
-Andrea

 tools/testing/selftests/sched_ext/numa.bpf.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/sched_ext/numa.bpf.c b/tools/testing/selftests/sched_ext/numa.bpf.c
index a79d86ed54a1b..209dacdbd117e 100644
--- a/tools/testing/selftests/sched_ext/numa.bpf.c
+++ b/tools/testing/selftests/sched_ext/numa.bpf.c
@@ -19,18 +19,6 @@ UEI_DEFINE(uei);
 
 const volatile unsigned int __COMPAT_SCX_PICK_IDLE_IN_NODE;
 
-static bool is_cpu_idle(s32 cpu, int node)
-{
-	const struct cpumask *idle_cpumask;
-	bool idle;
-
-	idle_cpumask = __COMPAT_scx_bpf_get_idle_cpumask_node(node);
-	idle = bpf_cpumask_test_cpu(cpu, idle_cpumask);
-	scx_bpf_put_cpumask(idle_cpumask);
-
-	return idle;
-}
-
s32 BPF_STRUCT_OPS(numa_select_cpu,
 		   struct task_struct *p, s32 prev_cpu, u64 wake_flags)
 {
@@ -44,12 +32,12 @@ s32 BPF_STRUCT_OPS(numa_select_cpu,
 	 */
 	cpu = __COMPAT_scx_bpf_pick_idle_cpu_node(p->cpus_ptr, node,
 					__COMPAT_SCX_PICK_IDLE_IN_NODE);
-	if (cpu < 0)
+	if (cpu < 0) {
 		cpu = __COMPAT_scx_bpf_pick_any_cpu_node(p->cpus_ptr, node,
 						__COMPAT_SCX_PICK_IDLE_IN_NODE);
-
-	if (is_cpu_idle(cpu, node))
-		scx_bpf_error("CPU %d should be marked as busy", cpu);
+		if (cpu < 0)
+			return prev_cpu;
+	}
 
 	if (__COMPAT_scx_bpf_cpu_node(cpu) != node)
 		scx_bpf_error("CPU %d should be in node %d", cpu, node);