Re: [PATCHSET v3 sched_ext/for-7.2] sched_ext: Topological CPU IDs and cid-form struct_ops

[Andrea Righi <arighi@nvidia.com>]

On Wed, Apr 29, 2026 at 03:29:43PM +0200, Andrea Righi wrote:
> Hi Tejun,
> 
> On Tue, Apr 28, 2026 at 10:35:28AM -1000, Tejun Heo wrote:
> > Hello,
> > 
> > v3 (all from the Sashiko AI review at
> > https://sashiko.dev/#/patchset/20260424172721.3458520-1-tj%40kernel.org):
> > 
> > - cid: drop leaked cpus_read_lock() on scx_cid_init() failure;
> >   BUILD_BUG_ON tightened to NR_CPUS<=8192 to match the BPF cmask
> >   helpers' CMASK_MAX_WORDS coverage.
> > - bpf-struct-size: use offsetof() in struct_size() to match the
> >   kernel <linux/overflow.h> macro semantics (no inflation from
> >   trailing struct padding).
> > - cmask: cmask_copy_from_kernel() validates src->base==0 via
> >   probe-read; nr_bits check is bit-level rather than rounded-up
> >   word-count.
> > - cid-qmap-idle: qmap_init() refuses to load when scx_bpf_nr_cids()
> >   exceeds SCX_QMAP_MAX_CPUS; the task_ctx flex array would otherwise
> >   overflow into the next slab entry.
> > 
> > v2: https://lore.kernel.org/r/20260424172721.3458520-1-tj@kernel.org
> > v1: https://lore.kernel.org/r/20260421071945.3110084-1-tj@kernel.org
> > 
> > This patchset introduces topological CPU IDs (cids) - dense,
> > topology-ordered cpu identifiers - and an alternative cid-form struct_ops
> > type that lets BPF schedulers operate in cid space directly.
> > 
> > Key pieces:
> > 
> > - cid space: scx_cid_init() walks nodes * LLCs * cores * threads and packs
> >   a dense cid mapping. The mapping can be overridden via
> >   scx_bpf_cid_override(). See "Topological CPU IDs" in ext_cid.h for the
> >   model.
> > 
> > - cmask: a base-windowed bitmap over cid space. Kernel and BPF helpers with
> >   identical semantics. Used by scx_qmap for per-task affinity and idle-cid
> >   tracking; meant to be the substrate for sub-sched cid allocation.
> > 
> > - bpf_sched_ext_ops_cid: a parallel struct_ops type whose callbacks take
> >   cids/cmasks instead of cpus/cpumasks. Kernel translates at the boundary
> >   via scx_cpu_arg() / scx_cpu_ret(); the two struct types share offsets up
> >   through @priv (verified by BUILD_BUG_ON) so the union view in scx_sched
> >   works without function-pointer casts. Sub-sched support is tied to
> >   cid-form: validate_ops() rejects cpu-form sub-scheds and cpu-form roots
> >   that expose sub_attach / sub_detach.
> > 
> > - cid-form kfuncs: scx_bpf_kick_cid, scx_bpf_cidperf_{cap,cur,set},
> >   scx_bpf_cid_curr, scx_bpf_task_cid, scx_bpf_this_cid,
> >   scx_bpf_nr_{cids,online_cids}, scx_bpf_cid_to_cpu, scx_bpf_cpu_to_cid.
> >   A cid-form program may not call cpu-only kfuncs (enforced at verifier
> >   load via scx_kfunc_context_filter); the reverse is intentionally
> >   permissive to ease migration.
> 
> So, IIUC scx schedulers attached to bpf_sched_ext_ops_cid can't use the built-in
> idle CPU selection kfuncs (ext_idle.c), right?
> 
> And that also means sub-sched support => no built-in idle CPU selection. That's
> a bit unfortunate... I guess we could implement a similar logic in cid/cmask
> space, maybe in BPF.

And apart from this and the other comments in PATCH 8/17 and PATCH 16/17,
everything else looks good to me.

Reviewed-by: Andrea Righi <arighi@nvidia.com>

Thanks,
-Andrea