Re: [PATCH v2] perf: Timehist account sch delay for scheduled out running
From: Madadi Vineeth Reddy
Date: Tue Jun 18 2024 - 06:58:14 EST
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@xxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> Cc: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx>
> Cc: Madadi Vineeth Reddy <vineethr@xxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxx>
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);