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

[Shrikanth Hegde]

Hi Peter,

On 6/26/26 3:09 PM, 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?
>
> >   #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?

I think it is better to put EXPORT_SYMBOL_GPL.
So that any module tries to alter it must be a GPL one.
Would be less headache for us i guess.

> > @@ -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...
>
> >   	set_cpu_present(cpu, true);
> >   	set_cpu_possible(cpu, true);
> >   
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index 2f4530eb543f..9e16946c9d62 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -8685,6 +8685,9 @@ int sched_cpu_activate(unsigned int cpu)
> >   	 */
> >   	sched_set_rq_online(rq, cpu);
> >   
> > +	/* preferred is subset of active and follows its state */
> > +	set_cpu_preferred(cpu, true);
> > +
> >   	return 0;
> >   }
> >   
> > @@ -8698,6 +8701,8 @@ int sched_cpu_deactivate(unsigned int cpu)
> >   	if (ret)
> >   		return ret;
> >   
> > +	set_cpu_preferred(cpu, false);
> > +
> >   	/*
> >   	 * Remove CPU from nohz.idle_cpus_mask to prevent participating in
> >   	 * load balancing when not active
>
> But this one clears active earlier, is that not a problem?
>
> Perhaps it is best if the modifier is a no-op when preferred mask does
> not exist?

Will make it as yury suggested in other thread with ifdefs.