Re: [PATCH v2] sched_ext: pass the initial cpu.idle state in scx_cgroup_init_args
Andrea Righi <[email protected]>
| Newsgroups | dev.linux.lists.sched-ext,org.kernel.vger.bpf,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <aoxeGI-RsNj3nDKU@gpd4> |
Hi Tao, On Mon, Aug 24, 2026 at 10:28:16PM +0800, Tao Cui wrote: > From: Tao Cui <[email protected]> > > scx_cgroup_init_args carries the initial weight and bandwidth control > parameters of a cgroup to ops.cgroup_init(), but not its cpu.idle > state. A cgroup that was already configured idle before the scheduler > was loaded (or before it was onlined under it) is presented as > non-idle, and the BPF scheduler only learns about it if cpu.idle is > written again later. > > Add the idle state to scx_cgroup_init_args and fill it in all four > places that build the args: scx_tg_online() for cgroups onlined under > the scheduler, scx_cgroup_init() for cgroups that already exist when > the scheduler is loaded, and the sub-scheduler handover paths > scx_cgroup_claim_subtree() and scx_cgroup_return_subtree(). > > Verified in a VM with a probe scheduler printing the init args: a > cgroup configured cpu.idle=1 before loading shows idle=1 in > ops.cgroup_init(), the default shows 0, and later cpu.idle writes > still come through ops.cgroup_set_idle(). The sub-scheduler paths > are compile-tested only. > > Signed-off-by: Tao Cui <[email protected]> We should probably add: Fixes: 347ed2d566da ("sched/ext: Implement cgroup_set_idle() callback" With that: Reviewed-by: Andrea Righi <[email protected]> Thanks, -Andrea > --- > v1 -> v2: Also fill .idle in the sub-scheduler handover paths in > sub.c, scx_cgroup_claim_subtree() and scx_cgroup_return_subtree(), > missed in v1 and pointed out by Andrea Righi and the sashiko AI > review bot. > > v1: https://lore.kernel.org/r/[email protected] > > kernel/sched/ext/ext.c | 4 +++- > kernel/sched/ext/internal.h | 3 +++ > kernel/sched/ext/sub.c | 2 ++ > 3 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c > index b646711a45fe..a7218314dcfb 100644 > --- a/kernel/sched/ext/ext.c > +++ b/kernel/sched/ext/ext.c > @@ -4764,7 +4764,8 @@ int scx_tg_online(struct task_group *tg) > { .weight = tg->scx.weight, > .bw_period_us = tg->scx.bw_period_us, > .bw_quota_us = tg->scx.bw_quota_us, > - .bw_burst_us = tg->scx.bw_burst_us }; > + .bw_burst_us = tg->scx.bw_burst_us, > + .idle = tg->scx.idle }; > > ret = SCX_CALL_OP_RET(sch, cgroup_init, > NULL, tg->css.cgroup, &args); > @@ -5185,6 +5186,7 @@ static int scx_cgroup_init(struct scx_sched *sch) > .bw_period_us = tg->scx.bw_period_us, > .bw_quota_us = tg->scx.bw_quota_us, > .bw_burst_us = tg->scx.bw_burst_us, > + .idle = tg->scx.idle, > }; > > ret = SCX_CALL_OP_RET(sch, cgroup_init, NULL, css->cgroup, &args); > diff --git a/kernel/sched/ext/internal.h b/kernel/sched/ext/internal.h > index 53e136a47924..aa149a9c29f7 100644 > --- a/kernel/sched/ext/internal.h > +++ b/kernel/sched/ext/internal.h > @@ -259,6 +259,9 @@ struct scx_cgroup_init_args { > u64 bw_period_us; > u64 bw_quota_us; > u64 bw_burst_us; > + > + /* whether the cgroup is configured idle via cpu.idle */ > + bool idle; > }; > > enum scx_cpu_preempt_reason { > diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c > index 0554448835bd..9701bd1d8ea0 100644 > --- a/kernel/sched/ext/sub.c > +++ b/kernel/sched/ext/sub.c > @@ -1361,6 +1361,7 @@ static s32 scx_cgroup_claim_subtree(struct scx_sched *sch) > .bw_period_us = tg->scx.bw_period_us, > .bw_quota_us = tg->scx.bw_quota_us, > .bw_burst_us = tg->scx.bw_burst_us, > + .idle = tg->scx.idle, > }; > > if (tg->scx.sched != parent || > @@ -1464,6 +1465,7 @@ static void scx_cgroup_return_subtree(struct scx_sched *sch) > .bw_period_us = tg->scx.bw_period_us, > .bw_quota_us = tg->scx.bw_quota_us, > .bw_burst_us = tg->scx.bw_burst_us, > + .idle = tg->scx.idle, > }; > > /* the first pass must have transferred everything */ > -- > 2.43.0 >