mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] sched_ext: pass the initial cpu.idle state in scx_cgroup_init_args
@ 2026-08-24 14:28 Tao Cui
  2026-08-24 15:07 ` Andrea Righi
  2026-08-24 16:54 ` Tejun Heo
  0 siblings, 2 replies; 5+ messages in thread
From: Tao Cui @ 2026-08-24 14:28 UTC (permalink / raw)
  To: tj, void, arighi
  Cc: changwoo, suzhidao, sched-ext, linux-kernel, bpf, cui.tao, Tao Cui

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>
---
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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] sched_ext: pass the initial cpu.idle state in scx_cgroup_init_args
  2026-08-24 14:28 [PATCH v2] sched_ext: pass the initial cpu.idle state in scx_cgroup_init_args Tao Cui
@ 2026-08-24 15:07 ` Andrea Righi
  2026-08-25  2:20   ` Tao Cui
  2026-08-24 16:54 ` Tejun Heo
  1 sibling, 1 reply; 5+ messages in thread
From: Andrea Righi @ 2026-08-24 15:07 UTC (permalink / raw)
  To: Tao Cui
  Cc: tj, void, changwoo, suzhidao, sched-ext, linux-kernel, bpf, Tao Cui

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
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] sched_ext: pass the initial cpu.idle state in scx_cgroup_init_args
  2026-08-24 14:28 [PATCH v2] sched_ext: pass the initial cpu.idle state in scx_cgroup_init_args Tao Cui
  2026-08-24 15:07 ` Andrea Righi
@ 2026-08-24 16:54 ` Tejun Heo
  2026-08-25  2:26   ` Tao Cui
  1 sibling, 1 reply; 5+ messages in thread
From: Tejun Heo @ 2026-08-24 16:54 UTC (permalink / raw)
  To: Tao Cui
  Cc: void, arighi, changwoo, suzhidao, emil, sched-ext, linux-kernel,
	bpf, cuitao

Hello, Tao.

On Mon, Aug 24, 2026 at 10:28:16PM +0800, Tao Cui wrote:
> +
> +	/* whether the cgroup is configured idle via cpu.idle */
> +	bool			idle;

In sched_ext, a bare "idle" reads as CPU idle state (ops.update_idle(),
idle cpumasks, scx_bpf_pick_idle_cpu()). cpu.idle is the cgroup analog
of the SCHED_IDLE policy, so please name the field sched_idle instead.
Renaming tg->scx.idle to match would be nice too.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] sched_ext: pass the initial cpu.idle state in scx_cgroup_init_args
  2026-08-24 15:07 ` Andrea Righi
@ 2026-08-25  2:20   ` Tao Cui
  0 siblings, 0 replies; 5+ messages in thread
From: Tao Cui @ 2026-08-25  2:20 UTC (permalink / raw)
  To: Andrea Righi
  Cc: cui.tao, tj, void, changwoo, suzhidao, sched-ext, linux-kernel,
	bpf, 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@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>
> 

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@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
>>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] sched_ext: pass the initial cpu.idle state in scx_cgroup_init_args
  2026-08-24 16:54 ` Tejun Heo
@ 2026-08-25  2:26   ` Tao Cui
  0 siblings, 0 replies; 5+ messages in thread
From: Tao Cui @ 2026-08-25  2:26 UTC (permalink / raw)
  To: Tejun Heo
  Cc: cui.tao, void, arighi, changwoo, suzhidao, emil, sched-ext,
	linux-kernel, bpf, cuitao

Hello, Tejun,

在 2026/8/25 00:54, Tejun Heo 写道:
> Hello, Tao.
> 
> On Mon, Aug 24, 2026 at 10:28:16PM +0800, Tao Cui wrote:
>> +
>> +	/* whether the cgroup is configured idle via cpu.idle */
>> +	bool			idle;
> 
> In sched_ext, a bare "idle" reads as CPU idle state (ops.update_idle(),
> idle cpumasks, scx_bpf_pick_idle_cpu()). cpu.idle is the cgroup analog
> of the SCHED_IDLE policy, so please name the field sched_idle instead.
> Renaming tg->scx.idle to match would be nice too.
> 

Thanks for the review. Both are renamed in v3: the new
scx_cgroup_init_args field, and tg->scx.idle -> tg->scx.sched_idle
as a separate second patch in the series, so 1/2 stays minimal for
stable backport.

Thanks,
Tao

> Thanks.
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-08-25  2:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-24 14:28 [PATCH v2] sched_ext: pass the initial cpu.idle state in scx_cgroup_init_args Tao Cui
2026-08-24 15:07 ` Andrea Righi
2026-08-25  2:20   ` Tao Cui
2026-08-24 16:54 ` Tejun Heo
2026-08-25  2:26   ` Tao Cui

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®