[PATCH v2] perf: Timehist account sch delay for scheduled out running

[PATCH v2] perf: Timehist account sch delay for scheduled out running

[Fernand Sieber]

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@amazon.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Madadi Vineeth Reddy <vineethr@linux.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:
     
                 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);
-- 
2.40.1

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

[Madadi Vineeth Reddy]

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@amazon.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Madadi Vineeth Reddy <vineethr@linux.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@linux.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);

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

[Namhyung Kim]

On Tue, 18 Jun 2024 11:03:39 +0200, 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.
> 
> [...]

Applied to perf-tools-next, thanks!

Best regards,
Namhyung