Re: [PATCH 2/6] sched_ext: idle: Explicitly pass allowed cpumask to scx_select_cpu_dfl()

[Andrea Righi <arighi@nvidia.com>]

On Mon, Mar 31, 2025 at 11:50:27AM -1000, Tejun Heo wrote:
> Hello,
> 
> On Fri, Mar 21, 2025 at 11:10:48PM +0100, Andrea Righi wrote:
> ...
> > +s32 scx_select_cpu_dfl(struct task_struct *p, s32 prev_cpu, u64 wake_flags,
> > +		       const struct cpumask *cpus_allowed, u64 flags)
> >  {
> >  	const struct cpumask *llc_cpus = NULL, *numa_cpus = NULL;
> >  	int node = scx_cpu_node_if_enabled(prev_cpu);
> > @@ -457,9 +458,9 @@ s32 scx_select_cpu_dfl(struct task_struct *p, s32 prev_cpu, u64 wake_flags, u64
> >  		struct cpumask *local_cpus = this_cpu_cpumask_var_ptr(local_numa_idle_cpumask);
> >  		const struct cpumask *cpus = numa_span(prev_cpu);
> >  
> > -		if (task_affinity_all(p))
> > +		if (cpus_allowed == p->cpus_ptr && task_affinity_all(p))
> >  			numa_cpus = cpus;
> 
> Note that this test isn't quite correct. While the error isn't introduced by
> this patchset, this becomes a lot more prominent with the series.
> p->nr_cpus_allowed tracks the number of CPUs in p->cpus_mask. p->cpus_ptr
> can point away from p->cpus_mask without updating p->nr_cpus_allowed, so the
> condition that should be checked is p->cpus_ptr == &p->cpus_mask &&
> p->nr_cpus_allowed == num_possible_cpus().

Thanks for pointing this out. Considering that, it's more clear (and less
bug prone) to just use NULL when the caller doesn't want to specify an
additional cpumask. Will change it in the next version.

Thanks,
-Andrea