Re: [PATCH] pstore: use ktime_get_real_fast_ns() instead of __getnstimeofday()

[Thomas Gleixner <tglx@linutronix.de>]

On Thu, 9 Nov 2017, Arnd Bergmann wrote:
> On Thu, Nov 9, 2017 at 1:55 AM, Kees Cook <keescook@chromium.org> wrote:
> > On Wed, Nov 8, 2017 at 8:00 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> I noticed that __getnstimeofday() is a rather odd interface, with
> >> a number of quirks:
> >>
> >> - The caller may come from NMI context, but the implementation is not NMI safe

Hmm, no. None of the regular accessor functions can be called from NMI
context safely.

> >> - The calling conventions are different from any other timekeeping functions

Yes, that was done back then to actually return the value (which is
possible with TSC) and indicate at the same time that timekeeping is
suspended.

> >> - The naming doesn't fit into the 'ktime_get_*()' scheme

Right, because it's not in the ktime_get_() family of functions.

> > As long as this is sane to call when timekeeping is not running, I'm
> > happy to take the patch.
> 
> Maybe John or Thomas can comment on this, I'm not totally sure how
> reliable it is. My best guess is that it will still produce correct time stamps
> a lot of the time, but it depends a bit on the clocksource hardware and
> how long the timekeeping has been suspended. As far as I can tell,
> 
> - if the clocksource register contents wrap around, the reported time
>   might appear to go back to the time of the last timer interrupt. The
>   shortest time I could find for an overflow is a 16-bit timer running at
>   32khz on i.MX1, overflowing every two seconds.
> - if reading a suspended clocksource returns zero (or another incorrect
>   value) the time might be in the far future
> - if reading a suspended clocksource causes a hang or crash, you
>   lose.

None of those problems exist for the fast NMI safe accessors. In
timekeeping_suspend() we store the current time and any access to the fast
timekeeper returns exactly the time at suspend up to the point where
timekeeping is resumed. See halt_fast_timekeeper().

So all you get between timekeeping_suspend() and timekeeping_resume() is
a stale timestamp. No backwards, no crash and burn.

The normal timekeeping accessor functions cannot be called between
timekeeping_suspend() and timekeeping_resume() at all. They will emit a
warning and can indeed crash and burn in one of the ways you described
above. This does not happen on x86 because the TSC will just work on
systems with pstore.

Thanks,

	tglx