Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1425273 > unrolled thread

[PATCH] crypto: use timespec64 for jent_get_nstime

Back to article view | Back to linux.kernel

Contents

  [PATCH] crypto: use timespec64 for jent_get_nstime Arnd Bergmann <arnd@arndb.de> - 2016-06-17 18:00 +0200
    Re: [PATCH] crypto: use timespec64 for jent_get_nstime Stephan Mueller <smueller@chronox.de> - 2016-06-18 10:20 +0200
    Re: [PATCH] crypto: use timespec64 for jent_get_nstime Stephan Mueller <smueller@chronox.de> - 2016-06-21 08:50 +0200
      Re: [PATCH] crypto: use timespec64 for jent_get_nstime Stephan Mueller <smueller@chronox.de> - 2016-06-21 10:40 +0200
      Re: [PATCH] crypto: use timespec64 for jent_get_nstime Arnd Bergmann <arnd@arndb.de> - 2016-06-21 10:40 +0200
        Re: [Y2038] [PATCH] crypto: use timespec64 for jent_get_nstime John Stultz <john.stultz@linaro.org> - 2016-06-21 18:30 +0200
          Re: [Y2038] [PATCH] crypto: use timespec64 for jent_get_nstime Stephan Mueller <smueller@chronox.de> - 2016-06-21 18:40 +0200
            Re: [Y2038] [PATCH] crypto: use timespec64 for jent_get_nstime John Stultz <john.stultz@linaro.org> - 2016-06-21 18:50 +0200
              Re: [Y2038] [PATCH] crypto: use timespec64 for jent_get_nstime Stephan Mueller <smueller@chronox.de> - 2016-06-21 19:00 +0200
                Re: [Y2038] [PATCH] crypto: use timespec64 for jent_get_nstime Stephan Mueller <smueller@chronox.de> - 2016-06-21 19:20 +0200
                Re: [Y2038] [PATCH] crypto: use timespec64 for jent_get_nstime John Stultz <john.stultz@linaro.org> - 2016-06-21 19:20 +0200
                Re: [Y2038] [PATCH] crypto: use timespec64 for jent_get_nstime Stephan Mueller <smueller@chronox.de> - 2016-06-21 19:40 +0200

#1425273 — [PATCH] crypto: use timespec64 for jent_get_nstime

The jent_get_nstime() function uses __getnstimeofday() to get
something similar to a 64-bit nanosecond counter. As we want
to get rid of struct timespec to fix the y2038 overflow,
this patch changes the code to use __getnstimeofday64()
instead, which returns a timespec64 structure.

Nothing changes about the algorithm, but it looks like it
might be better to use

 *out = ts.tv_sec * NSEC_PER_SEC + ts.tv_nsec;

or even

 *out = ktime_get_raw_fast_ns();

to get an actual nanosecond value and avoid the predictable
jitter that happens at the end of a second. Checking whether
or not this would be good needs investigation by someone who
understands the code better than me.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 crypto/jitterentropy-kcapi.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c
index 597cedd3531c..82ac44eff20d 100644
--- a/crypto/jitterentropy-kcapi.c
+++ b/crypto/jitterentropy-kcapi.c
@@ -89,7 +89,7 @@ void jent_memcpy(void *dest, const void *src, unsigned int n)
 
 void jent_get_nstime(__u64 *out)
 {
-	struct timespec ts;
+	struct timespec64 ts;
 	__u64 tmp = 0;
 
 	tmp = random_get_entropy();
@@ -98,9 +98,11 @@ void jent_get_nstime(__u64 *out)
 	 * If random_get_entropy does not return a value (which is possible on,
 	 * for example, MIPS), invoke __getnstimeofday
 	 * hoping that there are timers we can work with.
+	 *
+	 * should we have a __ktime_get_ns() instead?
 	 */
 	if ((0 == tmp) &&
-	   (0 == __getnstimeofday(&ts))) {
+	   (0 == __getnstimeofday64(&ts))) {
 		tmp = ts.tv_sec;
 		tmp = tmp << 32;
 		tmp = tmp | ts.tv_nsec;
-- 
2.9.0

[toc] | [next] | [standalone]

#1425664

Am Freitag, 17. Juni 2016, 17:59:41 schrieb Arnd Bergmann:

Hi Arnd,

> The jent_get_nstime() function uses __getnstimeofday() to get
> something similar to a 64-bit nanosecond counter. As we want
> to get rid of struct timespec to fix the y2038 overflow,
> this patch changes the code to use __getnstimeofday64()
> instead, which returns a timespec64 structure.
> 
> Nothing changes about the algorithm, but it looks like it
> might be better to use
> 
>  *out = ts.tv_sec * NSEC_PER_SEC + ts.tv_nsec;
> 
> or even
> 
>  *out = ktime_get_raw_fast_ns();
> 
> to get an actual nanosecond value and avoid the predictable
> jitter that happens at the end of a second. Checking whether
> or not this would be good needs investigation by someone who
> understands the code better than me.

I will test it.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  crypto/jitterentropy-kcapi.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c
> index 597cedd3531c..82ac44eff20d 100644
> --- a/crypto/jitterentropy-kcapi.c
> +++ b/crypto/jitterentropy-kcapi.c
> @@ -89,7 +89,7 @@ void jent_memcpy(void *dest, const void *src, unsigned int
> n)
> 
>  void jent_get_nstime(__u64 *out)
>  {
> -	struct timespec ts;
> +	struct timespec64 ts;
>  	__u64 tmp = 0;
> 
>  	tmp = random_get_entropy();
> @@ -98,9 +98,11 @@ void jent_get_nstime(__u64 *out)
>  	 * If random_get_entropy does not return a value (which is possible 
on,
>  	 * for example, MIPS), invoke __getnstimeofday
>  	 * hoping that there are timers we can work with.
> +	 *
> +	 * should we have a __ktime_get_ns() instead?
>  	 */
>  	if ((0 == tmp) &&
> -	   (0 == __getnstimeofday(&ts))) {
> +	   (0 == __getnstimeofday64(&ts))) {
>  		tmp = ts.tv_sec;
>  		tmp = tmp << 32;
>  		tmp = tmp | ts.tv_nsec;


Ciao
Stephan

[toc] | [prev] | [next] | [standalone]

#1427351

Am Freitag, 17. Juni 2016, 17:59:41 schrieb Arnd Bergmann:

Hi Arnd,

> The jent_get_nstime() function uses __getnstimeofday() to get
> something similar to a 64-bit nanosecond counter. As we want
> to get rid of struct timespec to fix the y2038 overflow,
> this patch changes the code to use __getnstimeofday64()
> instead, which returns a timespec64 structure.
> 
> Nothing changes about the algorithm, but it looks like it
> might be better to use
> 
>  *out = ts.tv_sec * NSEC_PER_SEC + ts.tv_nsec;
> 
> or even
> 
>  *out = ktime_get_raw_fast_ns();

I tested ktime_get_raw_fast_ns and it works perfectly well for the use case, 
i.e. the RNG behavior is indistinguishable from RDTSC on x86.

Which time source is used for this function? I am wondering about 
architectures other than X86.

Note, this function is used as a fallback when random_get_entropy is not 
implemented. In addition the Jitter RNG has an online health test which will 
catch the failure of the time stamp operation. Hence, even if this function 
may not be suitable on one or the other arch, it should not hurt though.

PS: If someone is interested in the test code or the test results, let me 
know.

Ciao
Stephan

[toc] | [prev] | [next] | [standalone]

#1427455

Am Dienstag, 21. Juni 2016, 10:32:23 schrieb Arnd Bergmann:

Hi Arnd,

> On Tuesday, June 21, 2016 8:20:10 AM CEST Stephan Mueller wrote:
> > Am Freitag, 17. Juni 2016, 17:59:41 schrieb Arnd Bergmann:
> > 
> > Hi Arnd,
> > 
> > > The jent_get_nstime() function uses __getnstimeofday() to get
> > > something similar to a 64-bit nanosecond counter. As we want
> > > to get rid of struct timespec to fix the y2038 overflow,
> > > this patch changes the code to use __getnstimeofday64()
> > > instead, which returns a timespec64 structure.
> > > 
> > > Nothing changes about the algorithm, but it looks like it
> > > might be better to use
> > > 
> > >  *out = ts.tv_sec * NSEC_PER_SEC + ts.tv_nsec;
> > > 
> > > or even
> > > 
> > >  *out = ktime_get_raw_fast_ns();
> > 
> > I tested ktime_get_raw_fast_ns and it works perfectly well for the use
> > case, i.e. the RNG behavior is indistinguishable from RDTSC on x86.
> > 
> > Which time source is used for this function? I am wondering about
> > architectures other than X86.
> 
> (adding John Stultz to Cc, he can correct me if I say something wrong)
> 
> All ktime_get_* and *get*timeofday() functions use the same clocksource,
> which is configurable and picked from the available clocksources,
> using a combination of reliability, latency for reading and accuracy.
> 
> 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
> - "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)
> - "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).

Thank you for the explanation.
> 
> There may be other clocksources in the system that are more appropriate
> for the purpose of jent_get_nstime(), but I don't think we have a way
> of exposing them at the moment, because we have not needed it in the
> past.
> 
> I assume what you want is the highest-resolution clocksource, and
> you don't really care about

Correct, all I need is a high-res clocksource.
> 
> > Note, this function is used as a fallback when random_get_entropy is not
> > implemented. In addition the Jitter RNG has an online health test which
> > will catch the failure of the time stamp operation. Hence, even if this
> > function may not be suitable on one or the other arch, it should not hurt
> > though.
> Ok. For the ARM architecture, I think most (maybe all) of the modern ARMv7
> platforms have either an architected "global timer" or have their own
> replacement, while the majority of the older ARMv4/v5/v6 machines don't
> have one.
> 
> However, there are some machines that implement "sched_clock" but don't
> implement get_cycles (which is the default for random_get_entropy()).
> 
> It's possible that there are cases where it's better to call sched_clock()
> than ktime_get_raw_fast_ns() as a fallback, though I could not find
> specific examples so far.

Given your explanation above, considering that the RNG would scream in the 
kernel log when the timer is too coarse and the fact that we have not had bug 
reports on this issue, I would think that a replacement call that is equal to 
the old __getnstimeofday is fine.

Do you want to update your patch or shall I hand in the patch?

Ciao
Stephan

[toc] | [prev] | [next] | [standalone]

#1427456

On Tuesday, June 21, 2016 8:20:10 AM CEST Stephan Mueller wrote:
> Am Freitag, 17. Juni 2016, 17:59:41 schrieb Arnd Bergmann:
> 
> Hi Arnd,
> 
> > The jent_get_nstime() function uses __getnstimeofday() to get
> > something similar to a 64-bit nanosecond counter. As we want
> > to get rid of struct timespec to fix the y2038 overflow,
> > this patch changes the code to use __getnstimeofday64()
> > instead, which returns a timespec64 structure.
> > 
> > Nothing changes about the algorithm, but it looks like it
> > might be better to use
> > 
> >  *out = ts.tv_sec * NSEC_PER_SEC + ts.tv_nsec;
> > 
> > or even
> > 
> >  *out = ktime_get_raw_fast_ns();
> 
> I tested ktime_get_raw_fast_ns and it works perfectly well for the use case, 
> i.e. the RNG behavior is indistinguishable from RDTSC on x86.
> 
> Which time source is used for this function? I am wondering about 
> architectures other than X86.

(adding John Stultz to Cc, he can correct me if I say something wrong)

All ktime_get_* and *get*timeofday() functions use the same clocksource,
which is configurable and picked from the available clocksources,
using a combination of reliability, latency for reading and accuracy.

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
- "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)
- "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).

There may be other clocksources in the system that are more appropriate
for the purpose of jent_get_nstime(), but I don't think we have a way
of exposing them at the moment, because we have not needed it in the
past.

I assume what you want is the highest-resolution clocksource, and
you don't really care about 

> Note, this function is used as a fallback when random_get_entropy is not 
> implemented. In addition the Jitter RNG has an online health test which will 
> catch the failure of the time stamp operation. Hence, even if this function 
> may not be suitable on one or the other arch, it should not hurt though.

Ok. For the ARM architecture, I think most (maybe all) of the modern ARMv7
platforms have either an architected "global timer" or have their own
replacement, while the majority of the older ARMv4/v5/v6 machines don't
have one.

However, there are some machines that implement "sched_clock" but don't
implement get_cycles (which is the default for random_get_entropy()).

It's possible that there are cases where it's better to call sched_clock()
than ktime_get_raw_fast_ns() as a fallback, though I could not find
specific examples so far.

	Arnd

[toc] | [prev] | [next] | [standalone]

#1427935 — Re: [Y2038] [PATCH] crypto: use timespec64 for jent_get_nstime

On Tue, Jun 21, 2016 at 1:32 AM, Arnd Bergmann <arnd@arndb.de> 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.

> - "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.

> - "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
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
-john

[toc] | [prev] | [next] | [standalone]

#1427941 — Re: [Y2038] [PATCH] crypto: use timespec64 for jent_get_nstime

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@arndb.de> 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.
> 
> > - "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.
> 
> > - "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.
> 
> thanks
> -john


Ciao
Stephan

[toc] | [prev] | [next] | [standalone]

#1427950 — Re: [Y2038] [PATCH] crypto: use timespec64 for jent_get_nstime

On Tue, Jun 21, 2016 at 9:34 AM, Stephan Mueller <smueller@chronox.de> 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@arndb.de> 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

[toc] | [prev] | [next] | [standalone]

#1427954 — Re: [Y2038] [PATCH] crypto: use timespec64 for jent_get_nstime

Am Dienstag, 21. Juni 2016, 09:47:23 schrieb John Stultz:

Hi John,

> On Tue, Jun 21, 2016 at 9:34 AM, Stephan Mueller <smueller@chronox.de> 
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@arndb.de> 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".

I hope my explanation is sufficient to not count as voodoo: I only need an 
interval measurement capability which has a sufficient high resolution similar 
or better than RDTSC on x86.
> 
> >> > - "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.

So, you suggest ktime_get_raw_ns? If yes, let me test that for one use case. 

Thanks
Stephan

[toc] | [prev] | [next] | [standalone]

#1427977 — Re: [Y2038] [PATCH] crypto: use timespec64 for jent_get_nstime

Am Dienstag, 21. Juni 2016, 10:12:24 schrieb John Stultz:

Hi John,
> 
> So this is definitely more clear then what was described earlier, and
> worries me because on many x86 machines (though fewer I guess these
> days then in the past) the clocksource will often not be the TSC (and
> have lower resolution).

Please note, the use of the ktime_get_raw_ns() is *only* used as a fallback on 
architectures where random_get_entropy / get_cycles is not implemented.

So, on X86 and on many other arches, this function call will never be 
triggered.

...
> 
> But please just make sure the reason why you're using that specific
> interface is clearly documented in the code so we don't have to later
> reverse engineer the intent.

Will do.

@Arnd: I would prepare a patch then using ktime_get_raw_ns and providing a 
good documentation in the code.

Ciao
Stephan

[toc] | [prev] | [next] | [standalone]

#1427989 — Re: [Y2038] [PATCH] crypto: use timespec64 for jent_get_nstime

On Tue, Jun 21, 2016 at 9:51 AM, Stephan Mueller <smueller@chronox.de> wrote:
> Am Dienstag, 21. Juni 2016, 09:47:23 schrieb John Stultz:
>
> Hi John,
>
>> On Tue, Jun 21, 2016 at 9:34 AM, Stephan Mueller <smueller@chronox.de>
> 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@arndb.de> 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".
>
> I hope my explanation is sufficient to not count as voodoo: I only need an
> interval measurement capability which has a sufficient high resolution similar
> or better than RDTSC on x86.

So this is definitely more clear then what was described earlier, and
worries me because on many x86 machines (though fewer I guess these
days then in the past) the clocksource will often not be the TSC (and
have lower resolution).

So you might boot w/ "clocksource=acpi_pm" or "clocksource=hpet" to
test these other possible clocksources.

However, this was always the case, so if if __getnstimeofday64()
worked before (though, its not clear why you were using the __ version
prior, since that's supposed to be an internal function), it uses the
same clocksource, so I suspect there won't be any functional
difference the the pre-existing code.

But please just make sure the reason why you're using that specific
interface is clearly documented in the code so we don't have to later
reverse engineer the intent.

thanks
-john

[toc] | [prev] | [next] | [standalone]

#1427999 — Re: [Y2038] [PATCH] crypto: use timespec64 for jent_get_nstime

Am Dienstag, 21. Juni 2016, 18:51:32 schrieb Stephan Mueller:

Hi,

> > So.. again, I'd avoid using the "fast" accessor unless there is a
> > clear need or obvious benefit. Which should be documented.
> 
> So, you suggest ktime_get_raw_ns? If yes, let me test that for one use case.

I tested ktime_get_raw_ns and it exhibits the same results as 
ktime_get_raw_fast_ns and RDTSC. So, I see no objections in using 
ktime_get_raw_ns.

Ciao
Stephan

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web