| Message ID | 1479531014-25264-5-git-send-email-john.stultz@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
* 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
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? -- Chris Metcalf, Mellanox Technologies http://www.mellanox.com
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
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 */