Re: [PATCH 3/4] sched_ext: idle: Introduce scx_bpf_select_cpu_pref()

[Andrea Righi]

Hi Changwoo,

On Fri, Mar 07, 2025 at 12:15:04PM +0900, Changwoo Min wrote:
...
> > +__bpf_kfunc s32 scx_bpf_select_cpu_pref(struct task_struct *p,
> > +					const struct cpumask *preferred_cpus,
> > +					s32 prev_cpu, u64 wake_flags, u64 flags)
> > +{
> > +#ifdef CONFIG_SMP
> > +	struct cpumask *preferred = NULL;
> > +	bool is_idle = false;
> > +#endif
> > +
> > +	if (!ops_cpu_valid(prev_cpu, NULL))
> > +		return -EINVAL;
> > +
> > +	if (!check_builtin_idle_enabled())
> > +		return -EBUSY;
> > +
> > +	if (!scx_kf_allowed(SCX_KF_SELECT_CPU))
> > +		return -EPERM;
> > +
> > +#ifdef CONFIG_SMP
> > +	preempt_disable();
> > +
> > +	/*
> > +	 * As an optimization, do not update the local idle mask when
> > +	 * p->cpus_ptr is passed directly in @preferred_cpus.
> > +	 */
> > +	if (preferred_cpus != p->cpus_ptr) {
> > +		preferred = this_cpu_cpumask_var_ptr(local_idle_cpumask);
> > +		if (!cpumask_and(preferred, p->cpus_ptr, preferred_cpus))
> > +			preferred = NULL;
> 
> I think it would be better to move cpumask_and() inside
> scx_select_cpu_dfl() because scx_select_cpu_dfl() assumes that
> anyway. That will make the code easier to read and avoid
> potential mistakes when extending scx_select_cpu_dfl() in the
> future.

I agree, will do this in the next version.

Thanks!
-Andrea