linux-kernel - Re: [PATCH v2] perf: Timehist account sch delay for scheduled out running

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <d1282b00-1f5d-43d1-8b4d-cb4ea5840330@linux.ibm.com>
Date: Tue, 18 Jun 2024 15:03:14 +0530
From: Madadi Vineeth Reddy <vineethr@...ux.ibm.com>
To: Fernand Sieber <sieberf@...zon.com>
Cc: linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
        Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Madadi Vineeth Reddy <vineethr@...ux.ibm.com>
Subject: Re: [PATCH v2] perf: Timehist account sch delay for scheduled out
 running

Hi Fernand,

On 18/06/24 14:33, Fernand Sieber wrote:
> When using perf timehist, sch delay is only computed for a waking task,
> not for a pre empted task. This patches changes sch delay to account for
> both. This makes sense as testing scheduling policy need to consider the
> effect of scheduling delay globally, not only for waking tasks.
> 
> Example of `perf timehist` report before the patch for `stress` task
> competing with each other.
> 
> First column is wait time, second column sch delay, third column
> runtime.
> 
> 1.492060 [0000]  s    stress[81]                          1.999      0.000      2.000      R  next: stress[83]
> 1.494060 [0000]  s    stress[83]                          2.000      0.000      2.000      R  next: stress[81]
> 1.496060 [0000]  s    stress[81]                          2.000      0.000      2.000      R  next: stress[83]
> 1.498060 [0000]  s    stress[83]                          2.000      0.000      1.999      R  next: stress[81]
> 
> After the patch, it looks like this (note that all wait time is not zero
> anymore):
> 
> 1.492060 [0000]  s    stress[81]                          1.999      1.999      2.000      R  next: stress[83]
> 1.494060 [0000]  s    stress[83]                          2.000      2.000      2.000      R  next: stress[81]
> 1.496060 [0000]  s    stress[81]                          2.000      2.000      2.000      R  next: stress[83]
> 1.498060 [0000]  s    stress[83]                          2.000      2.000      1.999      R  next: stress[81]
> 
> Signed-off-by: Fernand Sieber <sieberf@...zon.com>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: Arnaldo Carvalho de Melo <acme@...nel.org>
> Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>
> Cc: Madadi Vineeth Reddy <vineethr@...ux.ibm.com>
> ---
>  tools/perf/Documentation/perf-sched.txt | 4 ++--
>  tools/perf/builtin-sched.c              | 5 ++++-
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-sched.txt b/tools/perf/Documentation/perf-sched.txt
> index a216d2991b19..74c812f7a4a4 100644
> --- a/tools/perf/Documentation/perf-sched.txt
> +++ b/tools/perf/Documentation/perf-sched.txt
> @@ -64,8 +64,8 @@ There are several variants of 'perf sched':
>      
>     By default it shows the individual schedule events, including the wait
>     time (time between sched-out and next sched-in events for the task), the
> -   task scheduling delay (time between wakeup and actually running) and run
> -   time for the task:
> +   task scheduling delay (time between runnable and actually running) and
> +   run time for the task:

LGTM.

Reviewed-by: Madadi Vineeth Reddy <vineethr@...ux.ibm.com>

Thanks and Regards
Madadi Vineeth Reddy

>      
>                  time    cpu  task name             wait time  sch delay   run time
>                               [tid/pid]                (msec)     (msec)     (msec)
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 5977c49ae2c7..7422c930abaf 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -2659,7 +2659,10 @@ static int timehist_sched_change_event(struct perf_tool *tool,
>  		tr->last_state = state;
>  
>  		/* sched out event for task so reset ready to run time */
> -		tr->ready_to_run = 0;
> +		if (state == 'R')
> +			tr->ready_to_run = t;
> +		else
> +			tr->ready_to_run = 0;
>  	}
>  
>  	evsel__save_time(evsel, sample->time, sample->cpu);