Re: [RFC][PATCH 4/4] time: Clean up CLOCK_MONOTONIC_RAW time handling

From: John Stultz <john.stultz@linaro.org>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: lkml <linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@kernel.org>,
	Miroslav Lichvar <mlichvar@redhat.com>,
	Richard Cochran <richardcochran@gmail.com>,
	Prarit Bhargava <prarit@redhat.com>,
	Stephen Boyd <stephen.boyd@linaro.org>,
	Kevin Brodsky <kevin.brodsky@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Daniel Mentz <danielmentz@google.com>
Subject: Re: [RFC][PATCH 4/4] time: Clean up CLOCK_MONOTONIC_RAW time handling
Date: Fri, 25 Aug 2017 11:55:18 -0700	[thread overview]
Message-ID: <CALAqxLW4CUaz_DSdygkQvpHmEecadNASc353RSr2LO0J0a+xUw@mail.gmail.com> (raw)
In-Reply-To: <150366841319.27971.6041120504203143444@mail.alporthouse.com>

On Fri, Aug 25, 2017 at 6:40 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting John Stultz (2017-05-27 04:33:55)
>> Now that we fixed the sub-ns handling for CLOCK_MONOTONIC_RAW,
>> remove the duplicitive tk->raw_time.tv_nsec, which can be
>> stored in tk->tkr_raw.xtime_nsec (similarly to how its handled
>> for monotonic time).
>
> This patch breaks tkr_raw pretty badly. It is now accumulating 2x faster
> than tkr_mono, with the occasional wacky jump between two reads.
>
> e.g:
>
> @@ -838,6 +840,11 @@ ktime_t ktime_get_raw(void)
>
>         } while (read_seqcount_retry(&tk_core.seq, seq));
>
> +       pr_err("%s: raw.base=%llu, mono.base=%llu: nsecs=%llu, mono.nsecs=%llu\n",
> +                       __func__,
> +                       ktime_to_ns(base), ktime_to_ns(tk->tkr_mono.base),
> +                       nsecs, timekeeping_get_ns(&tk->tkr_mono));
> +
>
> gave
>         ktime_get_raw: raw.base=40255680121, mono.base=39940532364: nsecs=261321742, mono.nsecs=318630514
>         ktime_get_raw: raw.base=40339680124, mono.base=39940532364: nsecs=345836800, mono.nsecs=403109209
>
> https://bugs.freedesktop.org/show_bug.cgi?id=102336
>
> The patch still reverts cleanly and aiui was not part of the bugfixes,
> but a cleanup afterwards; so can be reapplied at leisure.

Thanks for the bug report!

Its odd, as I'm not seeing such behavior in my testing (using the
raw_skew or change_skew tests in kselftest/timers).  Can you try
running those tests to see if they fail on your hardware?

>From the bug report, it says it BYT specific, but I'm not sure what
that means. Are there any hardware specific details you can share?
What clocksource is being used? Can you send a dmesg log?

I'll look over the code again to see if I can catch anything by
review. Worse case if we can't get any traction on this in a day or so
I'll submit a revert.

thanks
-john