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

Groups > linux.kernel > #1525851 > unrolled thread

[PATCH 4/4] timekeeping: clocksource_cyc2ns: Document intended range limitation

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 4/4] timekeeping: clocksource_cyc2ns: Document intended range limitation John Stultz <john.stultz@linaro.org> - 2016-11-19 06:00 +0100
    Re: [PATCH 4/4] timekeeping: clocksource_cyc2ns: Document intended  range limitation Ingo Molnar <mingo@kernel.org> - 2016-11-21 10:00 +0100
      Re: [PATCH 4/4] timekeeping: clocksource_cyc2ns: Document intended  range limitation John Stultz <john.stultz@linaro.org> - 2016-11-21 23:10 +0100

#1525851 — [PATCH 4/4] timekeeping: clocksource_cyc2ns: Document intended range limitation

From: Chris Metcalf <cmetcalf@mellanox.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@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Chris Metcalf <cmetcalf@mellanox.com>
[jstultz: Fixed up to merge against HEAD & commit message tweaks]
Signed-off-by: John Stultz <john.stultz@linaro.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.
  *
  * XXX - This could use some mult_lxl_ll() asm optimization
  */
-- 
2.7.4

[toc] | [next] | [standalone]

#1526465 — Re: [PATCH 4/4] timekeeping: clocksource_cyc2ns: Document intended range limitation

* John Stultz <john.stultz@linaro.org> wrote:

> From: Chris Metcalf <cmetcalf@mellanox.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@gmail.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Prarit Bhargava <prarit@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Chris Metcalf <cmetcalf@mellanox.com>
> [jstultz: Fixed up to merge against HEAD & commit message tweaks]
> Signed-off-by: John Stultz <john.stultz@linaro.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?

Thanks,

	Ingo

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

#1527079 — Re: [PATCH 4/4] timekeeping: clocksource_cyc2ns: Document intended range limitation

On Mon, Nov 21, 2016 at 7:28 AM, Chris Metcalf <cmetcalf@mellanox.com> wrote:
> On 11/21/2016 3:54 AM, Ingo Molnar wrote:
>>
>> * John Stultz <john.stultz@linaro.org> wrote:
>>
>>> From: Chris Metcalf <cmetcalf@mellanox.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@gmail.com>
>>> Cc: Ingo Molnar <mingo@kernel.org>
>>> Cc: Prarit Bhargava <prarit@redhat.com>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Signed-off-by: Chris Metcalf <cmetcalf@mellanox.com>
>>> [jstultz: Fixed up to merge against HEAD & commit message tweaks]
>>> Signed-off-by: John Stultz <john.stultz@linaro.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?

Sure. Reworded to take Ingo's suggestions.

thanks
-john

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web