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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPo_bq3Bd4gRTeZHqD0p3KR+-339C=cmqgjgYMz1hvu1f+QAAQ@mail.gmail.com>
Date:   Thu, 25 Jun 2020 09:42:34 -0700
From:   Korben Rusek <korben@...gle.com>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Arnaldo Carvalho de Melo <acme@...hat.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Yordan Karadzhov <y.karadz@...il.com>,
        Tzvetomir Stoyanov <tz.stoyanov@...il.com>,
        Tom Zanussi <zanussi@...nel.org>,
        Jason Behmer <jbehmer@...gle.com>,
        Julia Lawall <julia.lawall@...ia.fr>,
        Clark Williams <williams@...hat.com>,
        bristot <bristot@...hat.com>, Daniel Wagner <wagi@...om.org>,
        Darren Hart <dvhart@...are.com>,
        Jonathan Corbet <corbet@....net>,
        "Suresh E. Warrier" <warrier@...ux.vnet.ibm.com>
Subject: Re: [RFC][PATCH] ring-buffer: Have nested events still record running
 time stamp

On Thu, Jun 25, 2020 at 7:38 AM Steven Rostedt <rostedt@...dmis.org> wrote:
>
> On Thu, 25 Jun 2020 09:53:15 -0400 (EDT)
> Mathieu Desnoyers <mathieu.desnoyers@...icios.com> wrote:
>
> > ----- On Jun 25, 2020, at 9:44 AM, rostedt rostedt@...dmis.org wrote:
> >
> > > From: "Steven Rostedt (VMware)" <rostedt@...dmis.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