Re: [Y2038] [PATCH] crypto: use timespec64 for jent_get_nstime

[John Stultz]

On Tue, Jun 21, 2016 at 9:34 AM, Stephan Mueller <smueller@xxxxxxxxxx> wrote:
> Am Dienstag, 21. Juni 2016, 09:22:31 schrieb John Stultz:
>
> Hi John,
>
>> On Tue, Jun 21, 2016 at 1:32 AM, Arnd Bergmann <arnd@xxxxxxxx> wrote:
>> > On Tuesday, June 21, 2016 8:20:10 AM CEST Stephan Mueller wrote:
>> >> Am Freitag, 17. Juni 2016, 17:59:41 schrieb Arnd Bergmann:
>> > Compared to the previous __getnstimeofday(), the difference is
>> >
>> > - using "monotonic" timebase instead of "real", so the zero time
>> >
>> >   is when the system booted rather than Jan 1 1970
>>
>> I haven't looked at the details of the calling code, but I'd worry for
>> crypto uses, especially if its being used for entropy collection,
>> using the monotonic clock instead of the realtime clock might be
>> problematic.
>
> Funnily it does not seem like that. All tests that I have conducted show that
> monotonic clocks behave equally as realtime clocks, because the uncertainty
> lies in the execution time of a set of instructions. All we need to do is to
> measure it with a timer that has a resolution that allows detecting these
> variations.

Ok. If you're only using it for interval measurements, then either way
shouldn't matter. I just wanted to make sure the entropy wasn't coming
from the actual time.


>> > - "raw" means we don't honor updates for the rate based on ntp,
>> >
>> >   which is probably better as the ntp state might be observable
>> >   over the net (it probably doesn't matter, but it can't hurt)
>>
>> So... this feels like a very vague explanation, and the lack of
>> frequency correction here probably need a really good comment. Keeping
>> multiple time domains is usually asking for trouble, but we added the
>> MONOTONIC_RAW clock to address a few cases where people really wanted
>> an abstract hardware counter, which was unaffected by frequency
>> corrections. I'd really make sure its clear why this is what you want
>> vs the standard system time domain so we don't run into problems
>> understanding it later.
>
> Perfect, that is what I would be interested in.

But documenting *why* clearly is the thing I'd very strongly suggest.
If we need to make some slight semantic change for whatever reason, I
don't want folks worried "we can't do that because the crypto code is
using it for voodoo".


>> > - "fast" means that in very rare cases, the time might appear
>> >
>> >   to go backwards (it probably can't happen here because you are not
>> >   called in an NMI).
>>
>> "fast" really means "safe-for-nmi wrt to locking".  The tradeoff being
>> that when frequency adjustments occur, and if your code is delayed,
>> you might see time go backwards by a small amount. This allows
>
> My code would not see that as an issue.
>
>> tracing/sched code (or other code called from NMI)  to not have to
>> duplicate the timekeeping infrastructure.
>>
>> I think without a much better explanation, using the "fast" method
>> isn't really warranted here.
>
> Thanks a lot. With that, I would think that the proposed ktime_get_raw_fast_ns
> is good for use, which is supported with testing on my system.

So.. again, I'd avoid using the "fast" accessor unless there is a
clear need or obvious benefit. Which should be documented.

thanks
-john