linux-kernel - Re: [PATCH] sched/rt: fix incorrect schedstats for rt thread

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: <20260114115533.2959537-1-dengjun.su@mediatek.com>
Date: Wed, 14 Jan 2026 19:55:31 +0800
From: Dengjun Su <dengjun.su@...iatek.com>
To: <peterz@...radead.org>
CC: <angelogioacchino.delregno@...labora.com>, <bsegall@...gle.com>,
	<dengjun.su@...iatek.com>, <dietmar.eggemann@....com>,
	<haiqiang.gong@...iatek.com>, <juri.lelli@...hat.com>,
	<linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
	<linux-mediatek@...ts.infradead.org>, <matthias.bgg@...il.com>,
	<mgorman@...e.de>, <mike.zhang@...iatek.com>, <mingo@...hat.com>,
	<peijun.huang@...iatek.com>, <rostedt@...dmis.org>,
	<vincent.guittot@...aro.org>, <vschneid@...hat.com>
Subject: Re: [PATCH] sched/rt: fix incorrect schedstats for rt thread

On Mon, 2026-01-12 at 17:38 +0100, Peter Zijlstra wrote:
> On Fri, Jan 09, 2026 at 03:24:47PM +0800, Dengjun Su wrote:
>
> > For __update_stats_wait_end(), task_on_rq_migrating(p) is needed to
> > distinguish between stage 2 and stage 4 because they involve
> > different
> > processing flows, but for __update_stats_wait_start(), it is not
> > necessary
> > to distinguish between stage 1 and stage 3.
> >
> > As for adding the condition wait_start > prev_wait_start, I think
> > it is
> > more like a mechanism to prevent statistical deviations caused by
> > time
> > inconsistencies.
>
> It looks like nonsense to me.. since you have a test-case, could you
> see
> what this does for you?
>
> Specifically:
>
>  - it ensures that when not in a migration, prev_wait_start must be 0
>
>  - it unconditionally subtracts; unsigned types are defined to wrap
>    nicely (2s complement) and it all should work just fine.
>
> ---
>
> diff --git a/kernel/sched/stats.c b/kernel/sched/stats.c
> index d1c9429a4ac5..144b23029327 100644
> --- a/kernel/sched/stats.c
> +++ b/kernel/sched/stats.c
> @@ -12,8 +12,10 @@ void __update_stats_wait_start(struct rq *rq,
> struct task_struct *p,
>  	wait_start = rq_clock(rq);
>  	prev_wait_start = schedstat_val(stats->wait_start);
>  
> -	if (p && likely(wait_start > prev_wait_start))
> +	if (p) {
> +		WARN_ON_ONCE(!task_on_rq_migrating(p) &&
> prev_wait_start);
>  		wait_start -= prev_wait_start;
> +	}
>  
>  	__schedstat_set(stats->wait_start, wait_start);
>  }

Hi Peter,

I have confirm in my current test case, adding this change or not
have no impact. I think this is reasonable, task_on_rq_migrating(p)
should be true in my test case.

Base on original patch that adds this, I think the logic of 
__update_stats_wait_start is:

  1. If migrating and the time is updated normally, it records the 
  waiting time on the previous queue.

  2. If time is abnormal, it updates wait_start to the current time. 
  For migrating, it will discards the previously counted waiting time 
  on the previous queue (perhaps this statistical value itself 
  is abnormal).

According to the current modification, if there is a time abnormal,
wait_start will not be updated to current time.

Partial fragments of the original patch are as follows.

---

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 9a5e60f..53ec4d4 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -738,27 +738,41 @@ static void update_curr_fair(struct rq *rq)
 }
 
 static inline void
-update_stats_wait_start(struct cfs_rq *cfs_rq, struct sched_entity *se)
+update_stats_wait_start(struct cfs_rq *cfs_rq, struct sched_entity *se,
+			bool migrating)
 {
-	schedstat_set(se->statistics.wait_start, rq_clock(rq_of(cfs_rq)));
+	schedstat_set(se->statistics.wait_start,
+		migrating &&
+		likely(rq_clock(rq_of(cfs_rq)) > se->statistics.wait_start) ?
+		rq_clock(rq_of(cfs_rq)) - se->statistics.wait_start :
+		rq_clock(rq_of(cfs_rq)));
 }