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: Fixed timestamp error when unable to confirm event sched_in time
Date: Fri, 16 Aug 2024 10:05:28 -0700 [thread overview]
Message-ID: <Zr-G2IjXp_W2CkHi@google.com> (raw)
In-Reply-To: <20240806034357.1340216-1-yangjihong@bytedance.com>
On Tue, Aug 06, 2024 at 11:43:57AM +0800, Yang Jihong wrote:
> If sched_in event for current task is not recorded, sched_in timestamp
> will be set to end_time of time window interest, causing an error in
> timestamp show. In this case, we choose to ignore this event.
>
> Test scenario:
> perf[1229608] does not record the first sched_in event, run time and sch delay are both 0
>
> # perf sched timehist
> 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.763231 [0000] perf[1229608] 0.000 0.000 0.000
> 2090450.763235 [0000] migration/0[15] 0.000 0.001 0.003
> 2090450.763263 [0001] perf[1229608] 0.000 0.000 0.000
> 2090450.763268 [0001] migration/1[21] 0.000 0.001 0.004
> 2090450.763302 [0002] perf[1229608] 0.000 0.000 0.000
> 2090450.763309 [0002] migration/2[27] 0.000 0.001 0.007
> 2090450.763338 [0003] perf[1229608] 0.000 0.000 0.000
> 2090450.763343 [0003] migration/3[33] 0.000 0.001 0.004
>
> Before:
> arbitrarily specify a time window of interest, timestamp will be set to an incorrect value
>
> # perf sched timehist --time 100,200
> Samples of sched_switch event do not have callchains.
> time cpu task name wait time sch delay run time
> [tid/pid] (msec) (msec) (msec)
> --------------- ------ ------------------------------ --------- --------- ---------
> 200.000000 [0000] perf[1229608] 0.000 0.000 0.000
> 200.000000 [0001] perf[1229608] 0.000 0.000 0.000
> 200.000000 [0002] perf[1229608] 0.000 0.000 0.000
> 200.000000 [0003] perf[1229608] 0.000 0.000 0.000
> 200.000000 [0004] perf[1229608] 0.000 0.000 0.000
> 200.000000 [0005] perf[1229608] 0.000 0.000 0.000
> 200.000000 [0006] perf[1229608] 0.000 0.000 0.000
> 200.000000 [0007] perf[1229608] 0.000 0.000 0.000
>
> After:
>
> # perf sched timehist --time 100,200
> Samples of sched_switch event do not have callchains.
> time cpu task name wait time sch delay run time
> [tid/pid] (msec) (msec) (msec)
> --------------- ------ ------------------------------ --------- --------- ---------
>
> Fixes: 853b74071110 ("perf sched timehist: Add option to specify time window of interest")
> Signed-off-by: Yang Jihong <yangjihong@bytedance.com>
> ---
> tools/perf/builtin-sched.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 8750b5f2d49b..92b1113b22ad 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -2683,9 +2683,12 @@ static int timehist_sched_change_event(struct perf_tool *tool,
> * - previous sched event is out of window - we are done
> * - sample time is beyond window user cares about - reset it
> * to close out stats for time window interest
> + * - If tprev is 0, that is, sched_in event for current task is
> + * not recorded, cannot determine whether sched_in event is
> + * within time window interest - ignore it
> */
> if (ptime->end) {
> - if (tprev > ptime->end)
> + if (!tprev || (tprev && tprev > ptime->end))
This can be 'if (!tprev || tprev > ptime->end)'
Thanks,
Namhyung
> goto out;
>
> if (t > ptime->end)
> --
> 2.25.1
>
prev parent reply other threads:[~2024-08-16 17:05 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-06 3:43 [PATCH] perf sched timehist: Fixed timestamp error when unable to confirm event sched_in time Yang Jihong
2024-08-16 17:05 ` 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=Zr-G2IjXp_W2CkHi@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