[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ZrZ4LX20hN5w7MsJ@google.com>
Date: Fri, 9 Aug 2024 13:12:29 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Yang Jihong <yangjihong@...edance.com>
Cc: peterz@...radead.org, mingo@...hat.com, acme@...nel.org,
mark.rutland@....com, alexander.shishkin@...ux.intel.com,
jolsa@...nel.org, irogers@...gle.com, adrian.hunter@...el.com,
kan.liang@...ux.intel.com, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf sched timehist: Skip print non-idle task samples
when only show idle events
Hello,
On Tue, Aug 06, 2024 at 03:51:31PM +0800, Yang Jihong wrote:
> when only show idle events, runtime stats of non-idle tasks is not updated,
> and the value is 0, there is no need to print non-idle samples.
>
> Before:
>
> # perf sched timehist -I
> Samples of sched_switch event do not have callchains.
> time cpu task name wait time sch delay run time
> [tid/pid] (msec) (msec) (msec)
> --------------- ------ ------------------------------ --------- --------- ---------
> 2090450.763235 [0000] migration/0[15] 0.000 0.000 0.000
> 2090450.763268 [0001] migration/1[21] 0.000 0.000 0.000
> 2090450.763309 [0002] migration/2[27] 0.000 0.000 0.000
> 2090450.763343 [0003] migration/3[33] 0.000 0.000 0.000
> 2090450.763469 [0004] migration/4[39] 0.000 0.000 0.000
> 2090450.763501 [0005] migration/5[45] 0.000 0.000 0.000
> 2090450.763622 [0006] migration/6[51] 0.000 0.000 0.000
> 2090450.763660 [0007] migration/7[57] 0.000 0.000 0.000
> 2090450.763741 [0009] migration/9[69] 0.000 0.000 0.000
> 2090450.763862 [0010] migration/10[75] 0.000 0.000 0.000
> 2090450.763894 [0011] migration/11[81] 0.000 0.000 0.000
> 2090450.764021 [0012] migration/12[87] 0.000 0.000 0.000
> 2090450.764056 [0013] migration/13[93] 0.000 0.000 0.000
> 2090450.764135 [0014] migration/14[99] 0.000 0.000 0.000
> 2090450.764163 [0015] migration/15[105] 0.000 0.000 0.000
> 2090450.764292 [0016] migration/16[111] 0.000 0.000 0.000
> 2090450.764371 [0017] migration/17[117] 0.000 0.000 0.000
> 2090450.764422 [0018] migration/18[123] 0.000 0.000 0.000
> 2090450.764490 [0000] <idle> 0.000 0.000 1.255
> 2090450.764505 [0000] s1-perf[8235/7168] 0.000 0.000 0.000
> 2090450.764571 [0016] <idle> 0.000 0.000 0.278
> 2090450.764588 [0010] <idle> 0.000 0.000 0.725
> 2090450.764590 [0016] s1-agent[7179/7162] 0.000 0.000 0.000
> 2090450.764635 [0000] <idle> 0.015 0.015 0.129
> 2090450.764637 [0017] <idle> 0.000 0.000 0.266
> 2090450.764639 [0000] s1-perf[8235/7168] 0.000 0.000 0.000
> 2090450.764668 [0017] s1-agent[7180/7162] 0.000 0.000 0.000
> 2090450.764669 [0000] <idle> 0.003 0.003 0.029
> 2090450.764672 [0000] s1-perf[8235/7168] 0.000 0.000 0.000
> 2090450.764683 [0000] <idle> 0.003 0.003 0.010
>
> After:
>
> # perf sched timehist -I
> Samples of sched_switch event do not have callchains.
> time cpu task name wait time sch delay run time
> [tid/pid] (msec) (msec) (msec)
> --------------- ------ ------------------------------ --------- --------- ---------
> 2090450.764490 [0000] <idle> 0.000 0.000 1.255
> 2090450.764571 [0016] <idle> 0.000 0.000 0.278
> 2090450.764588 [0010] <idle> 0.000 0.000 0.725
> 2090450.764635 [0000] <idle> 0.015 0.015 0.129
> 2090450.764637 [0017] <idle> 0.000 0.000 0.266
> 2090450.764669 [0000] <idle> 0.003 0.003 0.029
> 2090450.764683 [0000] <idle> 0.003 0.003 0.010
> 2090450.764688 [0016] <idle> 0.019 0.019 0.097
> 2090450.764694 [0000] <idle> 0.001 0.001 0.009
> 2090450.764706 [0000] <idle> 0.001 0.001 0.010
> 2090450.764725 [0002] <idle> 0.000 0.000 1.415
> 2090450.764728 [0000] <idle> 0.002 0.002 0.019
> 2090450.764823 [0000] <idle> 0.003 0.003 0.091
> 2090450.764838 [0019] <idle> 0.000 0.000 0.154
> 2090450.764865 [0002] <idle> 0.109 0.109 0.029
> 2090450.764866 [0000] <idle> 0.012 0.012 0.030
> 2090450.764880 [0002] <idle> 0.013 0.013 0.001
> 2090450.764880 [0000] <idle> 0.002 0.002 0.011
> 2090450.764896 [0000] <idle> 0.001 0.001 0.013
> 2090450.764903 [0019] <idle> 0.063 0.063 0.002
> 2090450.764908 [0019] <idle> 0.003 0.003 0.001
>
> Fixes: 07235f84ece6 ("perf sched timehist: Add -I/--idle-hist option")
> Signed-off-by: Yang Jihong <yangjihong@...edance.com>
> ---
> tools/perf/builtin-sched.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 8750b5f2d49b..04770c2ae008 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -2729,8 +2729,14 @@ static int timehist_sched_change_event(struct perf_tool *tool,
> }
> }
>
> - if (!sched->summary_only)
> + /*
> + * when only show idle events, only runtime stats of idle tasks
> + * need to update, and can skip non-idle tasks sample.
> + */
> + if (!sched->summary_only &&
> + !(sched->idle_hist && thread__tid(thread) != 0)) {
> timehist_print_sample(sched, evsel, sample, &al, thread, t, state);
> + }
I think we already have the same condition in the previous block, maybe
better to move this code into the block. Or, we can change it to goto
out and reduce the indentation level of the block.
Thanks,
Namhyung
>
> out:
> if (sched->hist_time.start == 0 && t >= ptime->start)
> --
> 2.25.1
>
Powered by blists - more mailing lists