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
