Re: [PATCH] sched_ext: idle: Respect task affinity in restricted CPU selection

[Hui Su <sh_def@163.com>]

Hi Andrea,

> The fix makes sense, but I'm wondering if we should derive the candidate CPU
> from the task's state instead of relying on prev_cpu. In practice prev_cpu
> should be always considered a hint rather than a guarantee that the task can
> still run there.
>
> Per-CPU tasks could use cpumask_first(p->cpus_ptr) and migration-disabled tasks
> could use task_cpu(p) directly.

Agreed. prev_cpu is only a hint, so deriving the candidate from the task's
state is the better behavior after an affinity change. The restricted path
should use cpumask_first(p->cpus_ptr) for a single-CPU task and task_cpu(p)
for a migration-disabled task, while validating the candidate against the
task's effective affinity and the caller-provided allowed mask as appropriate.

> And could you also clarify in the patch description that selecting the CPU
> consumes its idle bit? Without the affinity check, we can claim an idle CPU that
> the task cannot use and later dispatch validation doesn't undo that side effect.

Yes. scx_idle_test_and_clear_cpu() consumes the idle bit, so the affinity
checks need to happen before the claim. I will make that side effect explicit
in the patch description.

> It would also help to mention that per-CPU and migration-disabled tasks skip
> ops.select_cpu(), so this affects calls from other contexts, such as
> ops.enqueue(), leaving the normal ops.select_cpu() path unaffected.

That distinction is useful. These tasks skip ops.select_cpu(), so the
restricted path can be reached from other contexts such as ops.enqueue(),
while the normal ops.select_cpu() path continues through
scx_select_cpu_dfl() and is unaffected.

Thanks for the review. I will incorporate these points in the follow-up
revision.

--
Hui