Re: [PATCH v5 04/24] cpumask: Introduce cpu_preferred_mask

[Shrikanth Hegde]

On 6/26/26 6:10 PM, Yury Norov wrote:
> On Fri, Jun 26, 2026 at 11:39:01AM +0200, Peter Zijlstra wrote:
> > On Thu, Jun 25, 2026 at 06:16:28PM +0530, Shrikanth Hegde wrote:
> >
> > > diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
> > > index 80211900f373..5a643d608ea6 100644
> > > --- a/include/linux/cpumask.h
> > > +++ b/include/linux/cpumask.h
> > > @@ -120,12 +120,20 @@ extern struct cpumask __cpu_enabled_mask;
> > >   extern struct cpumask __cpu_present_mask;
> > >   extern struct cpumask __cpu_active_mask;
> > >   extern struct cpumask __cpu_dying_mask;
> > > +
> > > +#ifdef CONFIG_PREFERRED_CPU
> > > +extern struct cpumask __cpu_preferred_mask;
> > > +#else
> > > +#define __cpu_preferred_mask __cpu_active_mask
> > > +#endif
> >
> > This is cure, but does it not result in set_cpu_preferred() changing
> > active mask, and it that not somewhat unexpected behaviour?
>
> I agree, and I think I already commented on it on previous round.
> set_cpu_preferred() should be protected the same way as the
> corresponding mask, and should be a NOP when CONFIG_PREFERRED_CPU
> is disabled.
>
> > >   #define cpu_possible_mask ((const struct cpumask *)&__cpu_possible_mask)
> > >   #define cpu_online_mask   ((const struct cpumask *)&__cpu_online_mask)
> > >   #define cpu_enabled_mask   ((const struct cpumask *)&__cpu_enabled_mask)
> > >   #define cpu_present_mask  ((const struct cpumask *)&__cpu_present_mask)
> > >   #define cpu_active_mask   ((const struct cpumask *)&__cpu_active_mask)
> > >   #define cpu_dying_mask    ((const struct cpumask *)&__cpu_dying_mask)
> > > +#define cpu_preferred_mask ((const struct cpumask *)&__cpu_preferred_mask)
> > >   
> > >   extern atomic_t __num_online_cpus;
> > >   extern unsigned int __num_possible_cpus;
> >
> > > diff --git a/kernel/cpu.c b/kernel/cpu.c
> > > index bc4f7a9ba64e..d623a9c5554a 100644
> > > --- a/kernel/cpu.c
> > > +++ b/kernel/cpu.c
> > > @@ -3107,6 +3107,11 @@ EXPORT_SYMBOL(__cpu_dying_mask);
> > >   atomic_t __num_online_cpus __read_mostly;
> > >   EXPORT_SYMBOL(__num_online_cpus);
> > >   
> > > +#ifdef CONFIG_PREFERRED_CPU
> > > +struct cpumask __cpu_preferred_mask __read_mostly;
> > > +EXPORT_SYMBOL(__cpu_preferred_mask);
> > > +#endif
> >
> > Precedent is definitely towards !GPL exports for this, but could we get
> > away with making this one GPL?
> >
> >
> > > @@ -3164,6 +3169,7 @@ void __init boot_cpu_init(void)
> > >   	/* Mark the boot cpu "present", "online" etc for SMP and UP case */
> > >   	set_cpu_online(cpu, true);
> > >   	set_cpu_active(cpu, true);
> > > +	set_cpu_preferred(cpu, true);
> >
> > This sets active twice, which is harmless, but wasteful...
>
> I think, the good criteria for correctness of this series would be the
> identical binaries before the series, and when CONFIG_PREFERRED_CPU is
> off. At least, as a mental model. This double-set chunk breaks that
> model.

Sorry, i didn't get how comparison could be,
You mean bloat-o-meter or kernel/cpu.o size or vmlinux size file?

That would mean everything should be under ifdef CONFIG_PREFERRED_CPU.
No? That was the case in few versions earlier, and it was not looking 
good since due to many ifdefs.

If we fix set_cpu_preferred to be a NOP when CONFIG_PREFERRED_CPU=n and 
driver depends on it, i think we should be good.

What do you think?