> Let me give you an example: > > When tsc_init() enables the usage of TSC for sched_clock() the > initialization of the tsc sched clock conversion data starts from zero > and not from the current jiffies based sched_clock() value. This makes > the timestamps jump backwards: > > [ 0.010000] tsc: Detected 3192.137 MHz processor > [ 0.011000] clocksource: tsc-early: mask: ... > [ 0.002233] Calibrating delay loop (skipped), .... > > To address this, extend set_cyc2ns_scale() with an argument to base the > cyc2ns offset on the current sched_clock() value. During run time this > offset is 0 as the cyc2ns offset is based on the TSC sched_clock() > itself. > > See? Precise and pure technical. No we/us/would/ and no irrelevant > information.
Yes, thank you Thomas. I will update the changelog based on your suggestions, and no longer will impersonating my commit comments. > >> Signed-off-by: Pavel Tatashin <[email protected]> >> --- >> arch/x86/kernel/tsc.c | 15 +++++++++------ >> 1 file changed, 9 insertions(+), 6 deletions(-) >> >> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c >> index 186395041725..654a01cc0358 100644 >> --- a/arch/x86/kernel/tsc.c >> +++ b/arch/x86/kernel/tsc.c >> @@ -133,7 +133,9 @@ static inline unsigned long long cycles_2_ns(unsigned >> long long cyc) >> return ns; >> } >> >> -static void set_cyc2ns_scale(unsigned long khz, int cpu, unsigned long long >> tsc_now) >> +static void set_cyc2ns_scale(unsigned long khz, int cpu, >> + unsigned long long tsc_now, >> + unsigned long long sched_now) > > sched_now is not a real good name for this as it's only used at > initialization time. So the argument name should reflect this otherwise you > wonder yourself when looking at that code 6 month from now, why it's 0 on > all the run time call sites. init_offset or some other sensible name which > makes the purpose entirely clear. > >> void __init tsc_init(void) >> { >> - u64 lpj, cyc; >> + u64 lpj, cyc, sch; > > sch? what's wrong with sched_now or now? It's not that there is a 3 letter > limit. Sometimes I get caught into following the local style too much: void __init tsc_init(void) { u64 lpj, cyc; int cpu; Hm, all the above are 3-letter variables, lets add another 3 letter one :) I will change it to init_offset as you suggested above for set_cyc2ns_scale(). Also, I will address all the other comments that you provided in this series. Thank you, Pavel
