linux-kernel - Re: [PATCH 4/4] timekeeping: clocksource_cyc2ns: Document intended range limitation

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]

Message-ID: <68f6e792-b2e1-c898-22b1-692ee1e6c367@mellanox.com>
Date:   Mon, 21 Nov 2016 10:28:24 -0500
From:   Chris Metcalf <cmetcalf@...lanox.com>
To:     Ingo Molnar <mingo@...nel.org>,
        John Stultz <john.stultz@...aro.org>
CC:     lkml <linux-kernel@...r.kernel.org>,
        Richard Cochran <richardcochran@...il.com>,
        Prarit Bhargava <prarit@...hat.com>,
        "Thomas Gleixner" <tglx@...utronix.de>
Subject: Re: [PATCH 4/4] timekeeping: clocksource_cyc2ns: Document intended
 range limitation

On 11/21/2016 3:54 AM, Ingo Molnar wrote:
> * John Stultz <john.stultz@...aro.org> wrote:
>
>> From: Chris Metcalf <cmetcalf@...lanox.com>
>>
>> The "cycles" argument should not be an absolute clocksource cycle
>> value, as the implementation's arithmetic will overflow relatively
>> easily with wide (64 bit) clocksource counters.
>>
>> For performance, the implementation is simple and fast, since the
>> function is intended for only relatively small delta values of
>> clocksource cycles.
>>
>> Cc: Richard Cochran <richardcochran@...il.com>
>> Cc: Ingo Molnar <mingo@...nel.org>
>> Cc: Prarit Bhargava <prarit@...hat.com>
>> Cc: Thomas Gleixner <tglx@...utronix.de>
>> Signed-off-by: Chris Metcalf <cmetcalf@...lanox.com>
>> [jstultz: Fixed up to merge against HEAD & commit message tweaks]
>> Signed-off-by: John Stultz <john.stultz@...aro.org>
>> ---
>>   include/linux/clocksource.h | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
>> index 0839818..0881bca 100644
>> --- a/include/linux/clocksource.h
>> +++ b/include/linux/clocksource.h
>> @@ -169,7 +169,10 @@ static inline u32 clocksource_hz2mult(u32 hz, u32 shift_constant)
>>    * @mult:	cycle to nanosecond multiplier
>>    * @shift:	cycle to nanosecond divisor (power of two)
>>    *
>> - * Converts cycles to nanoseconds, using the given mult and shift.
>> + * Converts clocksource cycles to nanoseconds, using the given mult and shift.
>> + * The code is optimized for performance and not intended to work
>> + * with absolute clocksource cycles, as it will easily overflow,
>> + * but just intended for relative (delta) clocksource cycles.
> Had to read this explanation twice, how about:
>
>      * Converts clocksource cycles to nanoseconds, using the given @mult and @shift.
>      * The code is optimized for performance and is not intended to work
>      * with absolute clocksource cycles (as those will easily overflow),
>      * but is only intended to be used with relative (delta) clocksource cycles.
>
> Did I get it right?

Yes, I think that's an improvement.  Thanks!

John, I assume you can just fix this up?

-- 
Chris Metcalf, Mellanox Technologies
http://www.mellanox.com