Re: [PATCH 3/6] sched_ext: idle: Introduce the concept of allowed CPUs

[Tejun Heo <tj@kernel.org>]

Hello,

On Fri, Mar 07, 2025 at 09:01:05PM +0100, Andrea Righi wrote:
> Many scx schedulers define their own concept of scheduling domains to
> represent topology characteristics, such as heterogeneous architectures

I'm not sure "domain" is a good choice given that sched_domain is already an
established construct in kernel and means something specific.

> (e.g., big.LITTLE, P-cores/E-cores), or to categorize tasks based on
> specific properties (e.g., setting the soft-affinity of certain tasks to
> a subset of CPUs).
> 
> Currently, there is no mechanism to share these domains with the
> built-in idle CPU selection policy. As a result, schedulers often
> implement their own idle CPU selection policies, which are typically
> similar to one another, leading to a lot of code duplication.
> 
> To address this, introduce the concept of allowed domain (represented as
> a cpumask) that can be used by the BPF schedulers to apply the built-in
> idle CPU selection policy to a subset of preferred CPUs.

We don't need a new term here, do we? All that's being added is an extra
mask when picking CPUs.

> With this concept the idle CPU selection policy becomes the following:
>  - always prioritize CPUs from fully idle SMT cores (if SMT is enabled),
>  - select the same CPU if it's idle and in the allowed domain,
>  - select an idle CPU within the same LLC domain, if the LLC domain is a
>    subset of the allowed domain,

Why not select from the intersection of the same LLC domain and the cpumask?

>  - select an idle CPU within the same node, if the node domain is a
>    subset of the allowed domain,

Ditto.

>  - select an idle CPU within the allowed domain.
> 
> If the allowed domain is empty or NULL, the behavior of the built-in
> idle CPU selection policy remains unchanged.
> 
> This only introduces the core concept of allowed domain. This
> functionality will be exposed through a dedicated kfunc in a separate
> patch.
...
> -s32 scx_select_cpu_dfl(struct task_struct *p, s32 prev_cpu, u64 wake_flags, u64 flags)
> +s32 scx_select_cpu_dfl(struct task_struct *p, const struct cpumask *cpus_allowed,
> +		       s32 prev_cpu, u64 wake_flags, u64 flags)

Maybe rearrange them (p, prev_cpu, wake_flags, and_cpumask, pick_idle_flags)
so that the first three args align with select_task_rq() and we don't have
three consecutive integer arguments? Two back-to-back flag args increase the
chance of subtle bugs.

Thanks.

-- 
tejun