linux-kernel - Re: [PATCH 2/6] sched_ext: idle: Explicitly pass allowed cpumask to scx_select_cpu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z-uF4v0-MZ4iQFDZ@gpd3>
Date: Tue, 1 Apr 2025 08:21:22 +0200
From: Andrea Righi <arighi@...dia.com>
To: Tejun Heo <tj@...nel.org>
Cc: David Vernet <void@...ifault.com>, Changwoo Min <changwoo@...lia.com>,
	Joel Fernandes <joelagnelf@...dia.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/6] sched_ext: idle: Explicitly pass allowed cpumask to
 scx_select_cpu_dfl()

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