linux-kernel - Re: [PATCH 4/6] x86-64: vclock_gettime(CLOCK

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]

Date:	Tue, 29 Mar 2011 07:54:32 -0400
From:	Andrew Lutomirski <luto@....edu>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	x86@...nel.org, linux-kernel@...r.kernel.org,
	John Stultz <johnstul@...ibm.com>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 4/6] x86-64: vclock_gettime(CLOCK_MONOTONIC) can't ever
 see nsec < 0

On Tue, Mar 29, 2011 at 2:21 AM, Ingo Molnar <mingo@...e.hu> wrote:
>
> * Andy Lutomirski <luto@....EDU> wrote:
>
>> vclock_gettime's do_monotonic helper can't ever generate a negative
>> nsec value, so it doesn't need to check whether it's negative.  This
>> saves a single easily-predicted branch.
>>
>> Signed-off-by: Andy Lutomirski <luto@....edu>
>> ---
>>  arch/x86/vdso/vclock_gettime.c |   40 ++++++++++++++++++++++------------------
>>  1 files changed, 22 insertions(+), 18 deletions(-)
>>
>> diff --git a/arch/x86/vdso/vclock_gettime.c b/arch/x86/vdso/vclock_gettime.c
>> index ee55754..67d54bb 100644
>> --- a/arch/x86/vdso/vclock_gettime.c
>> +++ b/arch/x86/vdso/vclock_gettime.c
>> @@ -56,22 +56,6 @@ notrace static noinline int do_realtime(struct timespec *ts)
>>       return 0;
>>  }
>>
>> -/* Copy of the version in kernel/time.c which we cannot directly access */
>> -notrace static void
>> -vset_normalized_timespec(struct timespec *ts, long sec, long nsec)
>> -{
>> -     while (nsec >= NSEC_PER_SEC) {
>> -             nsec -= NSEC_PER_SEC;
>> -             ++sec;
>> -     }
>> -     while (nsec < 0) {
>> -             nsec += NSEC_PER_SEC;
>> -             --sec;
>> -     }
>> -     ts->tv_sec = sec;
>> -     ts->tv_nsec = nsec;
>> -}
>> -
>>  notrace static noinline int do_monotonic(struct timespec *ts)
>>  {
>>       unsigned long seq, ns, secs;
>> @@ -82,7 +66,17 @@ notrace static noinline int do_monotonic(struct timespec *ts)
>>               secs += gtod->wall_to_monotonic.tv_sec;
>>               ns += gtod->wall_to_monotonic.tv_nsec;
>>       } while (unlikely(read_seqretry(&gtod->lock, seq)));
>> -     vset_normalized_timespec(ts, secs, ns);
>> +
>> +     /* wall_time_nsec, vgetns(), and wall_to_monotonic.tv_nsec
>> +      * are all guaranteed to be nonnegative.
>> +      */
>> +     while (ns >= NSEC_PER_SEC) {
>> +             ns -= NSEC_PER_SEC;
>> +             ++secs;
>> +     }
>> +     ts->tv_sec = secs;
>> +     ts->tv_nsec = ns;
>> +
>>       return 0;
>>  }
>>
>> @@ -107,7 +101,17 @@ notrace static noinline int do_monotonic_coarse(struct timespec *ts)
>>               secs += gtod->wall_to_monotonic.tv_sec;
>>               ns += gtod->wall_to_monotonic.tv_nsec;
>>       } while (unlikely(read_seqretry(&gtod->lock, seq)));
>> -     vset_normalized_timespec(ts, secs, ns);
>> +
>> +     /* wall_time_nsec and wall_to_monotonic.tv_nsec are
>> +      * guaranteed to be between 0 and NSEC_PER_SEC.
>> +      */
>> +     if (ns >= NSEC_PER_SEC) {
>> +             ns -= NSEC_PER_SEC;
>> +             ++secs;
>> +     }
>> +     ts->tv_sec = secs;
>> +     ts->tv_nsec = ns;
>> +
>>       return 0;
>
> Beyond the change you describe in the changelog, you also uninlined the helper
> function.
>
> You can use __always_inline instead and still keep the code maintainable.
>

I think I should fix the changelog instead.  The two copies are
different (one uses while because nsec > 2e9 is possible and one uses
if because nsec < 2e9 always).

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/