On Thu, Jun 25, 2020 at 7:38 AM Steven Rostedt <rost...@goodmis.org> wrote: > > On Thu, 25 Jun 2020 09:53:15 -0400 (EDT) > Mathieu Desnoyers <mathieu.desnoy...@efficios.com> wrote: > > > ----- On Jun 25, 2020, at 9:44 AM, rostedt rost...@goodmis.org wrote: > > > > > From: "Steven Rostedt (VMware)" <rost...@goodmis.org> > > > > > > [ SEVEN YEAR PROBLEM SOLVED! ] > > > > > > Up until now, if an event is interrupted while it is recorded by an > > > interrupt, and that interrupt records events, the time of those events > > > will > > > all be the same. This is because events only record the delta of the time > > > since the previous event (or beginning of a page), and to handle updating > > > the time keeping for that of nested events is extremely racy. After years > > > of > > > thinking about this and several failed attempts, I finally have a solution > > > to solve this puzzle. > > > > Out of curiosity, considering that LTTng has solved this problem 10+ years > > ago > > with a simpler concurrency-friendly time-stamping model, why not simply use > > it > > rather than add complexity to the current ftrace timestamp scheme ? > > Because it requires updating all the tools that read this from user > space. > > I found a solution that works, so why change it and cause the backward > compatibility pain now? > > -- Steve
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 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: > 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