All of
 help / color / mirror / Atom feed
From: Steven Rostedt <>
To: Tzvetomir Stoyanov <>
Cc: Linux Trace Devel <>
Subject: Re: [PATCH v2 2/3] trace-cmd library: Add check before applying tsc2nsec offset
Date: Mon, 19 Apr 2021 09:45:43 -0400	[thread overview]
Message-ID: <20210419094543.15c131c2@gandalf.local.home> (raw)
In-Reply-To: <>

On Mon, 19 Apr 2021 11:08:04 +0300
Tzvetomir Stoyanov <> wrote:

> > We don't want all timestamps zero. We want to disable the starting event.
> >
> > By having this instead:
> >
> >                 } else {
> >                         tracecmd_warning("Timestamp %llu is before the initial offset %llu\n"
> >                                          "\tSetting offset to 0",
> >                                          ts, handle->tsc_calc.offset);
> >                         handle->tsc_calc.offset = 0;
> >                         ts = mul_u64_u32_shr(ts, handle->tsc_calc.mult, handle->tsc_calc.shift);
> >                 }
> >  
> This will set the guest offset to 0, will not change the offsets of
> the other files. The result is that the files will not be displayed in
> sync, because of these few broken events. As this is only a sanity
> check, and should not happen if the patch "trace-cmd: Get the
> timestamp of the first recorded event as TSC offset" is applied, I
> would suggest to zero the broken timestamps and to advise the user to
> use "--raw-ts" option instead:
>  "Timestamp %llu is before the initial offset %llu, set it to 0.
> Display the files with --raw-ts option to see the original
> timestamps.\n"

I still don't like it, because a thousand events all with the same
timestamp will still screw up everything (including the output of
kernelshark, as you'll see a thousand events at time zero, and then the
rest of the events at some way off in the future time).

And as this is in the library, having a warning is only applicable to
trace-cmd, and not any other user case.

Actually, I think we are looking at the wrong place to do the check. All
event streams (CPUs) need to be in monotonic order, or it's just corrupted
to begin with.

The library should supply a check to see if each stream starts after the
offset, and if not, give a report back to the user (trace-cmd in this case,
or kernelsharks ftrace plugin). Then give the user a way to give another
offset to tell all the other streams to convert to.

That is:

	max = 0;
	for each stream:
		ret = read first event, compared to offset
		if (!ret) {
			/* ret will be how much off by offset */
			if (ret > max)
				max = ret;
	if (max) {
		for each stream:
			Update offset by subtracting max

Look at each stream, and have some callback give you how much ahead the
first event is from its given offset. Take the biggest value from reading
all the streams, then tell all the streams to subtract its offset by that
max value. The end result is that all streams now start at a positive value.

-- Steve

  reply	other threads:[~2021-04-19 13:45 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-16 10:34 [PATCH v2 0/3] Fix overflow when applying tsc2nsec calculations Tzvetomir Stoyanov (VMware)
2021-04-16 10:34 ` [PATCH v2 1/3] trace-cmd library: Add new trace-cmd library APIs for guest ts corrections Tzvetomir Stoyanov (VMware)
2021-04-16 10:34 ` [PATCH v2 2/3] trace-cmd library: Add check before applying tsc2nsec offset Tzvetomir Stoyanov (VMware)
2021-04-16 20:12   ` Steven Rostedt
2021-04-19  8:08     ` Tzvetomir Stoyanov
2021-04-19 13:45       ` Steven Rostedt [this message]
2021-04-19 15:14         ` Steven Rostedt
2021-04-28 12:31           ` Tzvetomir Stoyanov
2021-04-16 10:34 ` [PATCH v2 3/3] trace-cmd: Get the timestamp of the first recorded event as TSC offset Tzvetomir Stoyanov (VMware)
2021-04-16 19:28   ` Steven Rostedt

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210419094543.15c131c2@gandalf.local.home \ \ \ \

* 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.