Re: [PATCH v2] sched_ext: pass the initial cpu.idle state in scx_cgroup_init_args

[Tao Cui]

Hello, Andrea,

在 2026/8/24 23:07, Andrea Righi 写道:
> Hi Tao,
> 
> On Mon, Aug 24, 2026 at 10:28:16PM +0800, Tao Cui wrote:
>> From: Tao Cui <cuitao@xxxxxxxxxx>
>>
>> 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 <cuitao@xxxxxxxxxx>
> 
> We should probably add:
> 
> Fixes: 347ed2d566da ("sched/ext: Implement cgroup_set_idle() callback"
> 
> With that:
> 
> Reviewed-by: Andrea Righi <arighi@xxxxxxxxxx>
> 

Thank you for the review, and for catching the sub.c handover paths
earlier. v3 adds the Fixes: tag you suggested and carries your
Reviewed-by. The field is also renamed to sched_idle per Tejun's
review; the rename is mechanical, so I kept the tag, but I'm happy
to wait for a re-review if you prefer.

Thanks,
Tao

> 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/20260824133954.561956-1-cui.tao@xxxxxxxxx
>>
>>  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
>>