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

[Andrea Righi <arighi@nvidia.com>]

Hi Tao,

On Mon, Aug 24, 2026 at 10:28:16PM +0800, Tao Cui wrote:
> From: Tao Cui <cuitao@kylinos.cn>
> 
> 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@kylinos.cn>

We should probably add:

Fixes: 347ed2d566da ("sched/ext: Implement cgroup_set_idle() callback"

With that:

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

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@linux.dev
> 
>  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
>