linux-kernel - Re: [PATCH v6 2/2] sched: update the rq->avg_idle when a task is moved to an idle CPU

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <f898a0f0-8b77-4a71-be3b-c74267de0bde@arm.com>
Date: Wed, 17 Dec 2025 17:15:59 +0100
From: Dietmar Eggemann <dietmar.eggemann@....com>
To: Shijie Huang <shijie@...eremail.onmicrosoft.com>,
 Huang Shijie <shijie@...amperecomputing.com>, mingo@...hat.com,
 peterz@...radead.org, juri.lelli@...hat.com, vincent.guittot@...aro.org
Cc: patches@...erecomputing.com, cl@...ux.com,
 Shubhang@...amperecomputing.com, rostedt@...dmis.org, bsegall@...gle.com,
 mgorman@...e.de, linux-kernel@...r.kernel.org, vschneid@...hat.com,
 vineethr@...ux.ibm.com, kprateek.nayak@....com
Subject: Re: [PATCH v6 2/2] sched: update the rq->avg_idle when a task is
 moved to an idle CPU

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.