* [PATCH v6 0/2] sched: update the rq->avg_idle when a task is moved to an idle CPU
@ 2025-12-09 9:45 Huang Shijie
2025-12-09 9:45 ` [PATCH v6 1/2] sched/fair: set rq->idle_stamp at the end of the sched_balance_newidle Huang Shijie
2025-12-09 9:45 ` [PATCH v6 2/2] sched: update the rq->avg_idle when a task is moved to an idle CPU Huang Shijie
0 siblings, 2 replies; 16+ messages in thread
From: Huang Shijie @ 2025-12-09 9:45 UTC (permalink / raw)
To: mingo, peterz, juri.lelli, vincent.guittot
Cc: patches, cl, Shubhang, dietmar.eggemann, rostedt, bsegall,
mgorman, linux-kernel, vschneid, vineethr, kprateek.nayak,
Huang Shijie
In the newidle balance, the rq->idle_stamp may set to a non-zero value
if it cannot pull any task.
In the wakeup, it will detect the rq->idle_stamp, and updates
the rq->avg_idle, then ends the CPU idle status by setting rq->idle_stamp
to zero.
Besides the wakeup, current code does not end the CPU idle status
when a task is moved to the idle CPU, such as fork/clone, execve,
or other cases.
This patch set tries to resolve it.
Tested this patch set with Specjbb:
There is no regression for this patch set
v5--> v6:
Remove "this_rq->idle_stamp = 0;" in patch 1.
Update the test result with Specjbb.
--v5:https://lkml.org/lkml/2025/12/3/179
v4--> v5:
Modify the changelog.
--v4:https://lkml.org/lkml/2025/11/28/300
v3--> v4:
Remove the code for delayed task.
--v3: https://lkml.org/lkml/2025/11/27/456
v2--> v3:
-- merge patch 3 into patch 2:
move update_rq_avg_idle() to enqueue_task().
--v2: https://lkml.org/lkml/2025/11/27/214
v1--> v2:
-- Put update_rq_avg_idle() to activate_task()
-- Add Delay-dequeue task check.
--v1: https://lkml.org/lkml/2025/11/24/97
Huang Shijie (2):
sched/fair: set rq->idle_stamp at the end of the sched_balance_newidle
sched: update the rq->avg_idle when a task is moved to an idle CPU
kernel/sched/core.c | 29 +++++++++++++++++------------
kernel/sched/fair.c | 10 ++++++----
2 files changed, 23 insertions(+), 16 deletions(-)
--
2.40.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v6 1/2] sched/fair: set rq->idle_stamp at the end of the sched_balance_newidle
2025-12-09 9:45 [PATCH v6 0/2] sched: update the rq->avg_idle when a task is moved to an idle CPU Huang Shijie
@ 2025-12-09 9:45 ` Huang Shijie
2025-12-09 9:45 ` [PATCH v6 2/2] sched: update the rq->avg_idle when a task is moved to an idle CPU Huang Shijie
1 sibling, 0 replies; 16+ messages in thread
From: Huang Shijie @ 2025-12-09 9:45 UTC (permalink / raw)
To: mingo, peterz, juri.lelli, vincent.guittot
Cc: patches, cl, Shubhang, dietmar.eggemann, rostedt, bsegall,
mgorman, linux-kernel, vschneid, vineethr, kprateek.nayak,
Huang Shijie
In current newidle balance, the rq->idle_stamp may set to a non-zero value
if it cannot pull any task.
In the wakeup, it will detect the rq->idle_stamp, and updates
the rq->avg_idle, then ends the CPU idle status by setting rq->idle_stamp
to zero.
Besides the wakeup, current code does not end the CPU idle status
when a task is moved to the idle CPU, such as fork/clone, execve,
or other cases.
In order to fix this issue, we want to add a hook(update_rq_avg_idle())
in the enqueue_task(). With this hook, if a task is moved to the idle CPU,
it will update the rq->avg_idle. Unfortunately, this hook is also called
in the newidle balance:
sched_balance_newidle() --> sched_balance_rq() --> ... --> enqueue_task()
If we still set rq->idle_stamp at the beginning of sched_balance_newidle(),
the rq->avg_idle will not be updated correctly.
In order to make it work correctly, save the idle_stamp at the beginning
of sched_balance_newidle(). If newidle balance cannot pull any task,
set the saved value for rq->idle_stamp. With this method,
the newidle balance still work correctly, and the hook in enqueue_task()
also works correctly.
Signed-off-by: Huang Shijie <shijie@os.amperecomputing.com>
---
kernel/sched/fair.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 1855975b8248..c3b4895f8e50 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -12865,6 +12865,7 @@ static int sched_balance_newidle(struct rq *this_rq, struct rq_flags *rf)
u64 t0, t1, curr_cost = 0;
struct sched_domain *sd;
int pulled_task = 0;
+ u64 idle_stamp;
update_misfit_status(NULL, this_rq);
@@ -12880,7 +12881,7 @@ static int sched_balance_newidle(struct rq *this_rq, struct rq_flags *rf)
* for CPU_NEWLY_IDLE, such that we measure the this duration
* as idle time.
*/
- this_rq->idle_stamp = rq_clock(this_rq);
+ idle_stamp = rq_clock(this_rq);
/*
* Do not pull tasks towards !active CPUs...
@@ -12992,10 +12993,13 @@ static int sched_balance_newidle(struct rq *this_rq, struct rq_flags *rf)
if (time_after(this_rq->next_balance, next_balance))
this_rq->next_balance = next_balance;
- if (pulled_task)
+ if (pulled_task) {
this_rq->idle_stamp = 0;
- else
+ } else {
+ /* Set it here on purpose. */
+ this_rq->idle_stamp = idle_stamp;
nohz_newidle_balance(this_rq);
+ }
rq_repin_lock(this_rq, rf);
--
2.40.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v6 2/2] sched: update the rq->avg_idle when a task is moved to an idle CPU
2025-12-09 9:45 [PATCH v6 0/2] sched: update the rq->avg_idle when a task is moved to an idle CPU Huang Shijie
2025-12-09 9:45 ` [PATCH v6 1/2] sched/fair: set rq->idle_stamp at the end of the sched_balance_newidle Huang Shijie
@ 2025-12-09 9:45 ` Huang Shijie
2025-12-11 16:15 ` Dietmar Eggemann
2025-12-13 1:36 ` Vincent Guittot
1 sibling, 2 replies; 16+ messages in thread
From: Huang Shijie @ 2025-12-09 9:45 UTC (permalink / raw)
To: mingo, peterz, juri.lelli, vincent.guittot
Cc: patches, cl, Shubhang, dietmar.eggemann, rostedt, bsegall,
mgorman, linux-kernel, vschneid, vineethr, kprateek.nayak,
Huang Shijie
In the newidle balance, the rq->idle_stamp may set to a non-zero value
if it cannot pull any task.
In the wakeup, it will detect the rq->idle_stamp, and updates
the rq->avg_idle, then ends the CPU idle status by setting rq->idle_stamp
to zero.
Besides the wakeup, current code does not end the CPU idle status
when a task is moved to the idle CPU, such as fork/clone, execve,
or other cases. In order to get more accurate rq->avg_idle,
we need to update it at more places(not only the wakeup).
This patch introduces a helper: update_rq_avg_idle().
And uses it in enqueue_task(), so it will update the rq->avg_idle
when a task is moved to an idle CPU at:
-- wakeup
-- fork/clone
-- execve
-- idle balance
-- other cases
Signed-off-by: Huang Shijie <shijie@os.amperecomputing.com>
---
kernel/sched/core.c | 29 +++++++++++++++++------------
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 9f10cfbdc228..2e3c4043de51 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2078,6 +2078,21 @@ unsigned long get_wchan(struct task_struct *p)
return ip;
}
+static void update_rq_avg_idle(struct rq *rq)
+{
+ if (rq->idle_stamp) {
+ u64 delta = rq_clock(rq) - rq->idle_stamp;
+ u64 max = 2*rq->max_idle_balance_cost;
+
+ update_avg(&rq->avg_idle, delta);
+
+ if (rq->avg_idle > max)
+ rq->avg_idle = max;
+
+ rq->idle_stamp = 0;
+ }
+}
+
void enqueue_task(struct rq *rq, struct task_struct *p, int flags)
{
if (!(flags & ENQUEUE_NOCLOCK))
@@ -2100,6 +2115,8 @@ void enqueue_task(struct rq *rq, struct task_struct *p, int flags)
if (sched_core_enabled(rq))
sched_core_enqueue(rq, p);
+
+ update_rq_avg_idle(rq);
}
/*
@@ -3645,18 +3662,6 @@ ttwu_do_activate(struct rq *rq, struct task_struct *p, int wake_flags,
p->sched_class->task_woken(rq, p);
rq_repin_lock(rq, rf);
}
-
- if (rq->idle_stamp) {
- u64 delta = rq_clock(rq) - rq->idle_stamp;
- u64 max = 2*rq->max_idle_balance_cost;
-
- update_avg(&rq->avg_idle, delta);
-
- if (rq->avg_idle > max)
- rq->avg_idle = max;
-
- rq->idle_stamp = 0;
- }
}
/*
--
2.40.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 2/2] sched: update the rq->avg_idle when a task is moved to an idle CPU
2025-12-09 9:45 ` [PATCH v6 2/2] sched: update the rq->avg_idle when a task is moved to an idle CPU Huang Shijie
@ 2025-12-11 16:15 ` Dietmar Eggemann
2025-12-12 3:16 ` Shijie Huang
2025-12-13 1:36 ` Vincent Guittot
1 sibling, 1 reply; 16+ messages in thread
From: Dietmar Eggemann @ 2025-12-11 16:15 UTC (permalink / raw)
To: Huang Shijie, mingo, peterz, juri.lelli, vincent.guittot
Cc: patches, cl, Shubhang, rostedt, bsegall, mgorman, linux-kernel,
vschneid, vineethr, kprateek.nayak
On 09.12.25 10:45, Huang Shijie wrote:
> In the newidle balance, the rq->idle_stamp may set to a non-zero value
> if it cannot pull any task.
>
> In the wakeup, it will detect the rq->idle_stamp, and updates
> the rq->avg_idle, then ends the CPU idle status by setting rq->idle_stamp
> to zero.
>
> Besides the wakeup, current code does not end the CPU idle status
> when a task is moved to the idle CPU, such as fork/clone, execve,
> or other cases. In order to get more accurate rq->avg_idle,
> we need to update it at more places(not only the wakeup).
>
> This patch introduces a helper: update_rq_avg_idle().
> And uses it in enqueue_task(), so it will update the rq->avg_idle
> when a task is moved to an idle CPU at:
> -- wakeup
> -- fork/clone
> -- execve
> -- idle balance
> -- other cases
[...]
In v2 you moved update_rq_avg_idle() (1) from activate_task() (2) to
enqueue_task() to possibly handle delayed tasks.
In v3 you figured there can't be any delayed task on a CPU when it sets
rq->idle_stamp in sched_balance_newidle()
So you could move (1) back to (2) avoiding the 'if rq->idle_stamp' for
the sched_change pattern for instance?
I tried to understand whether this patch could help with one issue we
currently have with 'DCPerf Mediawiki' benchmark where on 2 comparable
servers, one has a 10% lower CPU utilization and I traced it down to
significantly different behaviour in sched_balance_newidle() and further
down to:
if (!get_rd_overloaded(this_rq->rd) || this_rq->avg_idle <
sd->max_newidle_lb_cost)
where the one with the lower CPU utilization bails out way less often
(because this_rq->avg_idle is very high, system is overloaded). But I
failed so far. Anyway, we'll use this patch for another test run right now.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 2/2] sched: update the rq->avg_idle when a task is moved to an idle CPU
2025-12-11 16:15 ` Dietmar Eggemann
@ 2025-12-12 3:16 ` Shijie Huang
2025-12-12 14:22 ` Dietmar Eggemann
0 siblings, 1 reply; 16+ messages in thread
From: Shijie Huang @ 2025-12-12 3:16 UTC (permalink / raw)
To: Dietmar Eggemann, Huang Shijie, mingo, peterz, juri.lelli,
vincent.guittot
Cc: patches, cl, Shubhang, rostedt, bsegall, mgorman, linux-kernel,
vschneid, vineethr, kprateek.nayak
On 12/12/2025 00:15, Dietmar Eggemann wrote:
> In v2 you moved update_rq_avg_idle() (1) from activate_task() (2) to
> enqueue_task() to possibly handle delayed tasks.
Not for the delayed tasks, just for a more common place to handle more
cases.
> In v3 you figured there can't be any delayed task on a CPU when it sets
> rq->idle_stamp in sched_balance_newidle()
Yes.
> So you could move (1) back to (2) avoiding the 'if rq->idle_stamp' for
> the sched_change pattern for instance?
Could you please tell me what is "avoiding the 'if rq->idle_stamp' for
the sched_change pattern" ?
Sorry, I do not understand your meaning.
IMHO, there is no need to move (1) back to (2).
Thanks
Huang Shijie
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 2/2] sched: update the rq->avg_idle when a task is moved to an idle CPU
2025-12-12 3:16 ` Shijie Huang
@ 2025-12-12 14:22 ` Dietmar Eggemann
2025-12-15 9:35 ` Shijie Huang
0 siblings, 1 reply; 16+ messages in thread
From: Dietmar Eggemann @ 2025-12-12 14:22 UTC (permalink / raw)
To: Shijie Huang, Huang Shijie, mingo, peterz, juri.lelli, vincent.guittot
Cc: patches, cl, Shubhang, rostedt, bsegall, mgorman, linux-kernel,
vschneid, vineethr, kprateek.nayak
On 12.12.25 04:16, Shijie Huang wrote:
>
> On 12/12/2025 00:15, Dietmar Eggemann wrote:
>> In v2 you moved update_rq_avg_idle() (1) from activate_task() (2) to
>> enqueue_task() to possibly handle delayed tasks.
> Not for the delayed tasks, just for a more common place to handle more
> cases.
>> In v3 you figured there can't be any delayed task on a CPU when it sets
>> rq->idle_stamp in sched_balance_newidle()
> Yes.
>> So you could move (1) back to (2) avoiding the 'if rq->idle_stamp' for
>> the sched_change pattern for instance?
> Could you please tell me what is "avoiding the 'if rq->idle_stamp' for
> the sched_change pattern" ?
>
> Sorry, I do not understand your meaning.
sched_change uses dequeue_task()/enqueue_task() for a queued task to
change prio, policy, sched params, taskgroups, etc.
kernel/sched/sched.h:
DEFINE_CLASS(sched_change, struct sched_change_ctx *,
sched_change_end(_T),
sched_change_begin(p, flags),
struct task_struct *p, unsigned int flags)
kernel/sched/core.c:
struct sched_change_ctx *sched_change_begin(struct task_struct *p,
unsigned int flags)
void sched_change_end(struct sched_change_ctx *ctx)
> IMHO, there is no need to move (1) back to (2).
Btw, I'm using a BUG_ON(this_rq->idle_stamp) in sched_balance_newidle()
with update_rq_avg_idle() in activate_task() for testing.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 2/2] sched: update the rq->avg_idle when a task is moved to an idle CPU
2025-12-09 9:45 ` [PATCH v6 2/2] sched: update the rq->avg_idle when a task is moved to an idle CPU Huang Shijie
2025-12-11 16:15 ` Dietmar Eggemann
@ 2025-12-13 1:36 ` Vincent Guittot
2025-12-16 6:22 ` Shijie Huang
1 sibling, 1 reply; 16+ messages in thread
From: Vincent Guittot @ 2025-12-13 1:36 UTC (permalink / raw)
To: Huang Shijie
Cc: mingo, peterz, juri.lelli, patches, cl, Shubhang,
dietmar.eggemann, rostedt, bsegall, mgorman, linux-kernel,
vschneid, vineethr, kprateek.nayak
On Tue, 9 Dec 2025 at 10:46, Huang Shijie <shijie@os.amperecomputing.com> wrote:
>
> In the newidle balance, the rq->idle_stamp may set to a non-zero value
> if it cannot pull any task.
>
> In the wakeup, it will detect the rq->idle_stamp, and updates
> the rq->avg_idle, then ends the CPU idle status by setting rq->idle_stamp
> to zero.
>
> Besides the wakeup, current code does not end the CPU idle status
> when a task is moved to the idle CPU, such as fork/clone, execve,
> or other cases. In order to get more accurate rq->avg_idle,
> we need to update it at more places(not only the wakeup).
>
> This patch introduces a helper: update_rq_avg_idle().
> And uses it in enqueue_task(), so it will update the rq->avg_idle
> when a task is moved to an idle CPU at:
> -- wakeup
> -- fork/clone
> -- execve
> -- idle balance
> -- other cases
>
> Signed-off-by: Huang Shijie <shijie@os.amperecomputing.com>
> ---
> kernel/sched/core.c | 29 +++++++++++++++++------------
> 1 file changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 9f10cfbdc228..2e3c4043de51 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -2078,6 +2078,21 @@ unsigned long get_wchan(struct task_struct *p)
> return ip;
> }
>
> +static void update_rq_avg_idle(struct rq *rq)
> +{
> + if (rq->idle_stamp) {
> + u64 delta = rq_clock(rq) - rq->idle_stamp;
> + u64 max = 2*rq->max_idle_balance_cost;
> +
> + update_avg(&rq->avg_idle, delta);
> +
> + if (rq->avg_idle > max)
> + rq->avg_idle = max;
> +
> + rq->idle_stamp = 0;
> + }
> +}
> +
> void enqueue_task(struct rq *rq, struct task_struct *p, int flags)
> {
> if (!(flags & ENQUEUE_NOCLOCK))
> @@ -2100,6 +2115,8 @@ void enqueue_task(struct rq *rq, struct task_struct *p, int flags)
>
> if (sched_core_enabled(rq))
> sched_core_enqueue(rq, p);
> +
> + update_rq_avg_idle(rq);
put_prev_task_idle() would be a better place to call
update_rq_avg_idle() because this is when we leave idle.
> }
>
> /*
> @@ -3645,18 +3662,6 @@ ttwu_do_activate(struct rq *rq, struct task_struct *p, int wake_flags,
> p->sched_class->task_woken(rq, p);
> rq_repin_lock(rq, rf);
> }
> -
> - if (rq->idle_stamp) {
> - u64 delta = rq_clock(rq) - rq->idle_stamp;
> - u64 max = 2*rq->max_idle_balance_cost;
> -
> - update_avg(&rq->avg_idle, delta);
> -
> - if (rq->avg_idle > max)
> - rq->avg_idle = max;
> -
> - rq->idle_stamp = 0;
> - }
> }
>
> /*
> --
> 2.40.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 2/2] sched: update the rq->avg_idle when a task is moved to an idle CPU
2025-12-12 14:22 ` Dietmar Eggemann
@ 2025-12-15 9:35 ` Shijie Huang
2025-12-17 16:15 ` Dietmar Eggemann
0 siblings, 1 reply; 16+ messages in thread
From: Shijie Huang @ 2025-12-15 9:35 UTC (permalink / raw)
To: Dietmar Eggemann, Huang Shijie, mingo, peterz, juri.lelli,
vincent.guittot
Cc: patches, cl, Shubhang, rostedt, bsegall, mgorman, linux-kernel,
vschneid, vineethr, kprateek.nayak
On 12/12/2025 22:22, Dietmar Eggemann wrote:
>>> So you could move (1) back to (2) avoiding the 'if rq->idle_stamp' for
>>> the sched_change pattern for instance?
>> Could you please tell me what is "avoiding the 'if rq->idle_stamp' for
>> the sched_change pattern" ?
>>
>> Sorry, I do not understand your meaning.
> sched_change uses dequeue_task()/enqueue_task() for a queued task to
> change prio, policy, sched params, taskgroups, etc.
For sched_change, the dequeue_task()/enqueue_task() only work when
the queued task has TASK_ON_RQ_QUEUED flags. The TASK_ON_RQ_QUEUED
is set in activate_task().
1.) For this active task, if the sched_change makes it
dequeue_task()/enqueue_task() on
current CPU, it's okay. Since current CPU is not in the newidle,
the "rq->idle_stamp" is 0 at this case.
This patch works fine.
2.) For this active task, if the sched_change makes it
dequeue_task()/enqueue_task() on an another CPU,
it's okay too.
2.1) If the another CPU's idle_stamp is 0, the another CPU is
busy now.
The sched_change works fine with this patch.
2.2) If the another CPU's idle_stamp is not 0, the
sched_change also works fine with this patch.
Since the sched_change is breaking the idle state of
the another CPU by moving an active
task to an idle CPU. It makes sense.
Thanks
Huang Shijie
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 2/2] sched: update the rq->avg_idle when a task is moved to an idle CPU
2025-12-13 1:36 ` Vincent Guittot
@ 2025-12-16 6:22 ` Shijie Huang
2025-12-16 7:17 ` Vincent Guittot
0 siblings, 1 reply; 16+ messages in thread
From: Shijie Huang @ 2025-12-16 6:22 UTC (permalink / raw)
To: Vincent Guittot, Huang Shijie
Cc: mingo, peterz, juri.lelli, patches, cl, Shubhang,
dietmar.eggemann, rostedt, bsegall, mgorman, linux-kernel,
vschneid, vineethr, kprateek.nayak
On 13/12/2025 09:36, Vincent Guittot wrote:
> put_prev_task_idle() would be a better place to call
> update_rq_avg_idle() because this is when we leave idle.
The update_rq_avg_idle() is not only called by current CPU, but also
called by
other CPUs. For example, the try_to_wake_up(), update_rq_avg_idle() is
called by
the other CPUs. So enqueue_task() is a good place.
Thanks
Huang Shijie
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 2/2] sched: update the rq->avg_idle when a task is moved to an idle CPU
2025-12-16 6:22 ` Shijie Huang
@ 2025-12-16 7:17 ` Vincent Guittot
2025-12-16 7:38 ` Shijie Huang
0 siblings, 1 reply; 16+ messages in thread
From: Vincent Guittot @ 2025-12-16 7:17 UTC (permalink / raw)
To: Shijie Huang
Cc: Huang Shijie, mingo, peterz, juri.lelli, patches, cl, Shubhang,
dietmar.eggemann, rostedt, bsegall, mgorman, linux-kernel,
vschneid, vineethr, kprateek.nayak
On Tue, 16 Dec 2025 at 07:22, Shijie Huang
<shijie@amperemail.onmicrosoft.com> wrote:
>
>
> On 13/12/2025 09:36, Vincent Guittot wrote:
> > put_prev_task_idle() would be a better place to call
> > update_rq_avg_idle() because this is when we leave idle.
>
> The update_rq_avg_idle() is not only called by current CPU, but also
> called by
>
> other CPUs. For example, the try_to_wake_up(), update_rq_avg_idle() is
> called by
>
> the other CPUs. So enqueue_task() is a good place.
But put_prev_task_idle() is called by local CPU whenever it leaves
idle so instead of trying to catch all places that could make the CPU
leave idle it's better to use this single place.
And as you mentioned, put_prev_task_idle is only called by local CPU
whereas enqueue_task can be called by all CPUs creating useless
pressure in the variable.
So I disagree when you say enqueue_task() is a "good place"
Thanks
>
>
> Thanks
>
> Huang Shijie
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 2/2] sched: update the rq->avg_idle when a task is moved to an idle CPU
2025-12-16 7:17 ` Vincent Guittot
@ 2025-12-16 7:38 ` Shijie Huang
2025-12-16 8:47 ` Vincent Guittot
0 siblings, 1 reply; 16+ messages in thread
From: Shijie Huang @ 2025-12-16 7:38 UTC (permalink / raw)
To: Vincent Guittot
Cc: Huang Shijie, mingo, peterz, juri.lelli, patches, cl, Shubhang,
dietmar.eggemann, rostedt, bsegall, mgorman, linux-kernel,
vschneid, vineethr, kprateek.nayak
On 16/12/2025 15:17, Vincent Guittot wrote:
> On Tue, 16 Dec 2025 at 07:22, Shijie Huang
> <shijie@amperemail.onmicrosoft.com> wrote:
>>
>> On 13/12/2025 09:36, Vincent Guittot wrote:
>>> put_prev_task_idle() would be a better place to call
>>> update_rq_avg_idle() because this is when we leave idle.
>> The update_rq_avg_idle() is not only called by current CPU, but also
>> called by
>>
>> other CPUs. For example, the try_to_wake_up(), update_rq_avg_idle() is
>> called by
>>
>> the other CPUs. So enqueue_task() is a good place.
> But put_prev_task_idle() is called by local CPU whenever it leaves
> idle so instead of trying to catch all places that could make the CPU
> leave idle it's better to use this single place.
> And as you mentioned, put_prev_task_idle is only called by local CPU
> whereas enqueue_task can be called by all CPUs creating useless
> pressure in the variable.
The rq->idle_stamp is set at sched_balance_newidle(). then we call
update_rq_avg_idle()
in put_prev_task_idle() right now. How can we update the rq->avg_idle?
Thanks
Huang Shijie
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 2/2] sched: update the rq->avg_idle when a task is moved to an idle CPU
2025-12-16 7:38 ` Shijie Huang
@ 2025-12-16 8:47 ` Vincent Guittot
2025-12-16 9:49 ` Shijie Huang
0 siblings, 1 reply; 16+ messages in thread
From: Vincent Guittot @ 2025-12-16 8:47 UTC (permalink / raw)
To: Shijie Huang
Cc: Huang Shijie, mingo, peterz, juri.lelli, patches, cl, Shubhang,
dietmar.eggemann, rostedt, bsegall, mgorman, linux-kernel,
vschneid, vineethr, kprateek.nayak
On Tue, 16 Dec 2025 at 08:39, Shijie Huang
<shijie@amperemail.onmicrosoft.com> wrote:
>
>
> On 16/12/2025 15:17, Vincent Guittot wrote:
> > On Tue, 16 Dec 2025 at 07:22, Shijie Huang
> > <shijie@amperemail.onmicrosoft.com> wrote:
> >>
> >> On 13/12/2025 09:36, Vincent Guittot wrote:
> >>> put_prev_task_idle() would be a better place to call
> >>> update_rq_avg_idle() because this is when we leave idle.
> >> The update_rq_avg_idle() is not only called by current CPU, but also
> >> called by
> >>
> >> other CPUs. For example, the try_to_wake_up(), update_rq_avg_idle() is
> >> called by
> >>
> >> the other CPUs. So enqueue_task() is a good place.
> > But put_prev_task_idle() is called by local CPU whenever it leaves
> > idle so instead of trying to catch all places that could make the CPU
> > leave idle it's better to use this single place.
> > And as you mentioned, put_prev_task_idle is only called by local CPU
> > whereas enqueue_task can be called by all CPUs creating useless
> > pressure in the variable.
>
> The rq->idle_stamp is set at sched_balance_newidle(). then we call
> update_rq_avg_idle()
>
> in put_prev_task_idle() right now. How can we update the rq->avg_idle?
I'm not sure I understand your point.
rq->avg_idle tracks idle time. The easiest way would be to use
- set_next_task_idle() when we enter idle
- put_prev_task_idle() when we exit idle
Except that sched_balance_newidle() can be long and the time should be
accounted as idle time too. So instead of using set_next_task_idle(),
we use sched_balance_newidle() to set . Which is okay because
sched_balance_newidle() is always called before going to idle.
>
> Thanks
>
> Huang Shijie
>
> >>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 2/2] sched: update the rq->avg_idle when a task is moved to an idle CPU
2025-12-16 8:47 ` Vincent Guittot
@ 2025-12-16 9:49 ` Shijie Huang
0 siblings, 0 replies; 16+ messages in thread
From: Shijie Huang @ 2025-12-16 9:49 UTC (permalink / raw)
To: Vincent Guittot
Cc: Huang Shijie, mingo, peterz, juri.lelli, patches, cl, Shubhang,
dietmar.eggemann, rostedt, bsegall, mgorman, linux-kernel,
vschneid, vineethr, kprateek.nayak
On 16/12/2025 16:47, Vincent Guittot wrote:
> On Tue, 16 Dec 2025 at 08:39, Shijie Huang
> <shijie@amperemail.onmicrosoft.com> wrote:
>>
>> On 16/12/2025 15:17, Vincent Guittot wrote:
>>> On Tue, 16 Dec 2025 at 07:22, Shijie Huang
>>> <shijie@amperemail.onmicrosoft.com> wrote:
>>>> On 13/12/2025 09:36, Vincent Guittot wrote:
>>>>> put_prev_task_idle() would be a better place to call
>>>>> update_rq_avg_idle() because this is when we leave idle.
>>>> The update_rq_avg_idle() is not only called by current CPU, but also
>>>> called by
>>>>
>>>> other CPUs. For example, the try_to_wake_up(), update_rq_avg_idle() is
>>>> called by
>>>>
>>>> the other CPUs. So enqueue_task() is a good place.
>>> But put_prev_task_idle() is called by local CPU whenever it leaves
>>> idle so instead of trying to catch all places that could make the CPU
>>> leave idle it's better to use this single place.
>>> And as you mentioned, put_prev_task_idle is only called by local CPU
>>> whereas enqueue_task can be called by all CPUs creating useless
>>> pressure in the variable.
>> The rq->idle_stamp is set at sched_balance_newidle(). then we call
>> update_rq_avg_idle()
>>
>> in put_prev_task_idle() right now. How can we update the rq->avg_idle?
> I'm not sure I understand your point.
>
> rq->avg_idle tracks idle time. The easiest way would be to use
> - set_next_task_idle() when we enter idle
> - put_prev_task_idle() when we exit idle
>
> Except that sched_balance_newidle() can be long and the time should be
> accounted as idle time too. So instead of using set_next_task_idle(),
> we use sched_balance_newidle() to set . Which is okay because
> sched_balance_newidle() is always called before going to idle.
Thanks for the explanations.
It seems that put_prev_task_idle() is really a better place to call
update_rq_avg_idle(). Let me think it for a while :)
Thanks
Huang Shijie
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 2/2] sched: update the rq->avg_idle when a task is moved to an idle CPU
2025-12-15 9:35 ` Shijie Huang
@ 2025-12-17 16:15 ` Dietmar Eggemann
2026-01-07 15:48 ` Dietmar Eggemann
0 siblings, 1 reply; 16+ messages in thread
From: Dietmar Eggemann @ 2025-12-17 16:15 UTC (permalink / raw)
To: Shijie Huang, Huang Shijie, mingo, peterz, juri.lelli, vincent.guittot
Cc: patches, cl, Shubhang, rostedt, bsegall, mgorman, linux-kernel,
vschneid, vineethr, kprateek.nayak
On 15.12.25 10:35, Shijie Huang wrote:
>
> On 12/12/2025 22:22, Dietmar Eggemann wrote:
>>>> So you could move (1) back to (2) avoiding the 'if rq->idle_stamp' for
>>>> the sched_change pattern for instance?
>>> Could you please tell me what is "avoiding the 'if rq->idle_stamp' for
>>> the sched_change pattern" ?
>>>
>>> Sorry, I do not understand your meaning.
>> sched_change uses dequeue_task()/enqueue_task() for a queued task to
>> change prio, policy, sched params, taskgroups, etc.
>
> For sched_change, the dequeue_task()/enqueue_task() only work when
>
> the queued task has TASK_ON_RQ_QUEUED flags. The TASK_ON_RQ_QUEUED
>
> is set in activate_task().
I guess this was a misunderstanding. It works because of 'if
(rq->idle_stamp)' and setting 'rq->idle_stamp = 0' within the condition
but this condition isn't worth checking in certain places where we
actually call enqueue_task(). >
> 1.) For this active task, if the sched_change makes it dequeue_task()/
> enqueue_task() on
>
> current CPU, it's okay. Since current CPU is not in the newidle,
> the "rq->idle_stamp" is 0 at this case.
>
> This patch works fine.
>
>
> 2.) For this active task, if the sched_change makes it dequeue_task()/
> enqueue_task() on an another CPU,
>
> it's okay too.
>
> 2.1) If the another CPU's idle_stamp is 0, the another CPU is
> busy now.
>
> The sched_change works fine with this patch.
>
> 2.2) If the another CPU's idle_stamp is not 0, the sched_change
> also works fine with this patch.
>
> Since the sched_change is breaking the idle state of the
> another CPU by moving an active
>
> task to an idle CPU. It makes sense.
Not sure about this. I thought so far that the sched_change pattern is
doing a task dequeue + enqueue on the same CPU (this CPU or other)? So
you can't come out of idle here. We lock the rq before we call
scoped_guard (sched_change, ...)
I think Vincent is right by saying the update_rq_avg_idle() should be
put into put_prev_task_idle() instead.
Still waiting for the DCPerf Mediawiki test results to see if this
change fixes my 'rq->avg_idle being too big' issue.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 2/2] sched: update the rq->avg_idle when a task is moved to an idle CPU
2025-12-17 16:15 ` Dietmar Eggemann
@ 2026-01-07 15:48 ` Dietmar Eggemann
2026-01-09 13:33 ` Vincent Guittot
0 siblings, 1 reply; 16+ messages in thread
From: Dietmar Eggemann @ 2026-01-07 15:48 UTC (permalink / raw)
To: Shijie Huang, Huang Shijie, mingo, peterz, juri.lelli, vincent.guittot
Cc: patches, cl, Shubhang, rostedt, bsegall, mgorman, linux-kernel,
vschneid, vineethr, kprateek.nayak
On 17.12.25 17:15, Dietmar Eggemann wrote:
> On 15.12.25 10:35, Shijie Huang wrote:
>>
>> On 12/12/2025 22:22, Dietmar Eggemann wrote:
[...]
> I think Vincent is right by saying the update_rq_avg_idle() should be
> put into put_prev_task_idle() instead.
>
> Still waiting for the DCPerf Mediawiki test results to see if this
> change fixes my 'rq->avg_idle being too big' issue.
Turns out the patch didn't fix this issue. Still seeing a huge number of
sched_balance_newidle() calls in which the system is (1) overloaded and
(2) this_rq->avg_idle >= sd->max_newidle_lb_cost so that there is no
early bailout and no task is pulled at the end. Must be something else ...
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v6 2/2] sched: update the rq->avg_idle when a task is moved to an idle CPU
2026-01-07 15:48 ` Dietmar Eggemann
@ 2026-01-09 13:33 ` Vincent Guittot
0 siblings, 0 replies; 16+ messages in thread
From: Vincent Guittot @ 2026-01-09 13:33 UTC (permalink / raw)
To: Dietmar Eggemann
Cc: Shijie Huang, Huang Shijie, mingo, peterz, juri.lelli, patches,
cl, Shubhang, rostedt, bsegall, mgorman, linux-kernel, vschneid,
vineethr, kprateek.nayak
On Wed, 7 Jan 2026 at 16:48, Dietmar Eggemann <dietmar.eggemann@arm.com> wrote:
>
> On 17.12.25 17:15, Dietmar Eggemann wrote:
> > On 15.12.25 10:35, Shijie Huang wrote:
> >>
> >> On 12/12/2025 22:22, Dietmar Eggemann wrote:
>
> [...]
>
> > I think Vincent is right by saying the update_rq_avg_idle() should be
> > put into put_prev_task_idle() instead.
> >
> > Still waiting for the DCPerf Mediawiki test results to see if this
> > change fixes my 'rq->avg_idle being too big' issue.
>
> Turns out the patch didn't fix this issue. Still seeing a huge number of
> sched_balance_newidle() calls in which the system is (1) overloaded and
> (2) this_rq->avg_idle >= sd->max_newidle_lb_cost so that there is no
> early bailout and no task is pulled at the end. Must be something else ...
Do you mean the v6 or v7 version ?
sched_balance_newidle() will be called every time there is no cfs or
higher priority tasks to pick next but we will not loop the sched
domains and bail out early. So having a huge number of call to
sched_balance_newidle() is normal but if you don't bail out early and
loop the sched domain then it's a problem
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2026-01-09 13:34 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-09 9:45 [PATCH v6 0/2] sched: update the rq->avg_idle when a task is moved to an idle CPU Huang Shijie
2025-12-09 9:45 ` [PATCH v6 1/2] sched/fair: set rq->idle_stamp at the end of the sched_balance_newidle Huang Shijie
2025-12-09 9:45 ` [PATCH v6 2/2] sched: update the rq->avg_idle when a task is moved to an idle CPU Huang Shijie
2025-12-11 16:15 ` Dietmar Eggemann
2025-12-12 3:16 ` Shijie Huang
2025-12-12 14:22 ` Dietmar Eggemann
2025-12-15 9:35 ` Shijie Huang
2025-12-17 16:15 ` Dietmar Eggemann
2026-01-07 15:48 ` Dietmar Eggemann
2026-01-09 13:33 ` Vincent Guittot
2025-12-13 1:36 ` Vincent Guittot
2025-12-16 6:22 ` Shijie Huang
2025-12-16 7:17 ` Vincent Guittot
2025-12-16 7:38 ` Shijie Huang
2025-12-16 8:47 ` Vincent Guittot
2025-12-16 9:49 ` Shijie Huang
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®