Re: [RFC][PATCH] ring-buffer: Have nested events still record running time stamp

[Steven Rostedt <rostedt@goodmis.org>]

On Thu, 25 Jun 2020 09:42:34 -0700
Korben Rusek <korben@google.com> wrote:

> Great work! I'm not exactly qualified to review the code, but the
> logic seems correct. I'm curious how unlikely a zero delta is now and
> how you quantify it. Also does it negate the patch that I emailed out

Actually, in all my stress testing (where I also add nested
trace_printk()s to read what is happening), I was never once able to
trigger the zero delta path! I only tested it by adding code to inject
the event to force the given race condition.

Note, zero deltas are still there between absolute time stamps and
start of page, but that's still different than a zero delta from the
previous event.

> last week that adds a `force_abs_timestamp` trace/option in an attempt
> to get around this particular issue?
> 
> In reading through, I did notice a couple simple typos in the comments
> that are probably worth pointing out:

Thanks.

-- Steve


> 
> > If preempting an event time update, we may need absolute timestamp.  
> 
> Not a big deal, but it should be "may need *an* absolute timestamp"
> 
> > * Preempted beween C and E:
> > * Lost the previous events time stamp. Just set the
> > * delta to zero, and this will be the same time as
> > * the veent this event preempted. And the events that
> > * came after this will still be correct (as they would
> > * have built their delta on the previous event.  
> 
> Should be "the *event* this event preempted." It also needs a
> parenthesis at the end of the comment to close the parenthetical
> statement.
> 
> Thanks, Korben