From: Namhyung Kim <namhyung@kernel.org>
To: Yang Jihong <yangjihong@bytedance.com>
Cc: peterz@infradead.org, mingo@redhat.com, acme@kernel.org,
mark.rutland@arm.com, alexander.shishkin@linux.intel.com,
jolsa@kernel.org, irogers@google.com, adrian.hunter@intel.com,
kan.liang@linux.intel.com, linux-perf-users@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] perf sched timehist: Skip print non-idle task samples when only show idle events
Date: Fri, 9 Aug 2024 13:12:29 -0700 [thread overview]
Message-ID: <ZrZ4LX20hN5w7MsJ@google.com> (raw)
In-Reply-To: <20240806075131.1382728-1-yangjihong@bytedance.com>
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@bytedance.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
>
prev parent reply other threads:[~2024-08-09 20:12 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-06 7:51 [PATCH] perf sched timehist: Skip print non-idle task samples when only show idle events Yang Jihong
2024-08-09 20:12 ` Namhyung Kim [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZrZ4LX20hN5w7MsJ@google.com \
--to=namhyung@kernel.org \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=irogers@google.com \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=yangjihong@bytedance.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox