Re: [PATCH 2/5] introduce CLOCK_MONOTONIC_RAW

!MAILaRCHIVE_VOTE_RePLACE
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: john stultz <johnstul@...>
Cc: lkml <linux-kernel@...>, Ingo Molnar <mingo@...>
Date: Tuesday, March 25, 2008 - 11:41 pm

Hi,

On Tuesday 18. March 2008, john stultz wrote:


I didn't looked at the code, what I meant was moving it close to it, so it's 
done at the same time.


You could use a shift (e.g. SHIFT_HZ for non NO_HZ), to scale these value up a 
little.


It's not cruft, littering the code with unnecessary static variables is IMO 
worse. In OO terms this would be an abstract base class, the actual 
implementation only needs to provide a few functions and otherwise doesn't 
really have to worry about all the details.
Splitting the structure is certainly an option, but hiding related variables 
as static is IMO not a good choice.


Well, here are some ideas where I think it might be useful, it would make it 
possible to chain clocks with different frequencies:
- SMP systems with slighty different clock frequencies.
- instead of deactivating an unstable tsc clock, delay activation until we 
know it's stable.
- deactivate a tsc (which stops during sleep) before sleep and resync when 
needed.
- use different clocks for timer purposes (e.g. jiffies and a slow clock).

(The last point would make IMO more acceptable to use hrtimer for more trivial 
purposes, if it were at least possible to select the used clock.)


The extra resolution is still there, as it doesn't make any sense to use a 
shift larger than 32.


NTP_SCALE_SHIFT is a constant and a very simple shift, which produces better 
code.
There are two ways to use the nsec value here:
1. nsec_now = (((cycle_new - cycle_last) * mult) + nsec) >> shift
2. nsec_now = ((cycle_new - cycle_last) * mult) >> shift +
		nsec >> NTP_SCALE_SHIFT
Both do the same under the condition that the gettime operation is not 
significantly shorter than 1nsec, but the second version can generate 
slightly better code. I intended to use the first for xtime_nsec, but instead 
the value is still splitted at every timer interrupt.
For the raw time please at least get rid of the splitting of raw_nsec in 
update_wall_time(). From the monotonic_raw value you only need the tv_sec 
part and the tv_nsec part can be calculated as above.

bye, Roman
--
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
[PATCH 0/5] time/ntp changes, john stultz, (Sat Mar 15, 12:04 am)
[PATCH 2/5] introduce CLOCK_MONOTONIC_RAW, john stultz, (Sat Mar 15, 12:06 am)
Re: [PATCH 2/5] introduce CLOCK_MONOTONIC_RAW, Roman Zippel, (Sat Mar 15, 12:50 am)
Re: [PATCH 2/5] introduce CLOCK_MONOTONIC_RAW, john stultz, (Mon Mar 17, 10:03 pm)
Re: [PATCH 2/5] introduce CLOCK_MONOTONIC_RAW, Roman Zippel, (Tue Mar 25, 11:41 pm)
Re: [PATCH 2/5] introduce CLOCK_MONOTONIC_RAW, john stultz, (Mon Mar 17, 10:27 pm)
[PATCH 3/5] cleanups ntp.c (from Ingo), john stultz, (Sat Mar 15, 12:10 am)
Re: [PATCH 3/5] cleanups ntp.c (from Ingo), Jörg-Volker Peetz, (Sat Mar 15, 5:32 am)
Re: [PATCH 3/5] cleanups ntp.c (from Ingo), Roman Zippel, (Sat Mar 15, 1:23 pm)
Re: [PATCH 3/5] cleanups ntp.c (from Ingo), Roman Zippel, (Sat Mar 15, 1:06 am)
[PATCH 4/5] ntp.c code flow clenaups (from Ingo), john stultz, (Sat Mar 15, 12:12 am)
Re: [PATCH 4/5] ntp.c code flow clenaups (from Ingo), Ingo Oeser, (Sat Mar 15, 8:39 am)
Re: [PATCH 4/5] ntp.c code flow clenaups (from Ingo), Roman Zippel, (Sat Mar 15, 1:24 pm)
Re: [PATCH 4/5] ntp.c code flow clenaups (from Ingo), Roman Zippel, (Sat Mar 15, 12:52 am)
Re: [PATCH 4/5] ntp.c code flow clenaups (from Ingo), John Stultz, (Sat Mar 15, 1:04 am)
[PATCH 5/5] make more ntp values static, john stultz, (Sat Mar 15, 12:15 am)
Re: [PATCH 5/5] make more ntp values static, Roman Zippel, (Sat Mar 15, 1:09 am)