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
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.