* [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-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-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
* 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-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
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®