On Wed, Dec 17, 2014 at 4:40 AM, Jianyu Zhan <[email protected]> wrote: > Currently, in timekeeping_forward_now, we just use delta nsecs from > clocksource to update raw_time; however, in getnstime_raw_and_real, > getrawmonotonic64, etc, raw_time has slightly diffrent meaning: it not > only counts the raw monotonic raw time, but also with arch_gettimeoffset, > by using the timekeeping_get_ns_raw helper.
Wait.. So I'm not sure I understand the issue here. The arch_gettimeoffset() is really only used with tick-based architectures that cannot support clocksources. Thus its used in combination with the jiffies clocksource to provide the "time since the last tick". timekeeping_forward_now() accumulates the clocksource based time since the last update, and intentionally doesn't include the arch_gettimeoffset(), since we cannot re-base that value. > This patch unifies the meaning of raw_time. Also, it cleanups the > timekeeping_get_ns_raw helper, to make timekeeping_get_ns and > timekeeping_forward_now based on it. > > Signed-off-by: Jianyu Zhan <[email protected]> > --- > kernel/time/timekeeping.c | 52 > ++++++++++++++++------------------------------- > 1 file changed, 17 insertions(+), 35 deletions(-) > > diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c > index 6a93185..1348c9c 100644 > --- a/kernel/time/timekeeping.c > +++ b/kernel/time/timekeeping.c > @@ -191,41 +191,32 @@ u32 (*arch_gettimeoffset)(void) = > default_arch_gettimeoffset; > static inline u32 arch_gettimeoffset(void) { return 0; } > #endif > > -static inline s64 timekeeping_get_ns(struct tk_read_base *tkr) > +static inline s64 timekeeping_get_ns_raw(struct tk_read_base *tkr, > + bool update_cycle) > { > + struct clocksource *clock = tkr->clock; > cycle_t cycle_now, delta; > s64 nsec; > > /* read clocksource: */ > - cycle_now = tkr->read(tkr->clock); > + cycle_now = tkr->read(clock); > > /* calculate the delta since the last update_wall_time: */ > delta = clocksource_delta(cycle_now, tkr->cycle_last, tkr->mask); > + if (update_cycle) > + tkr->cycle_last = cycle_now; > Hrm. I can't say I'm a fan of modifying a read helper to also do writing. > - nsec = delta * tkr->mult + tkr->xtime_nsec; > - nsec >>= tkr->shift; > + /* convert delta to nanoseconds. */ > + nsec = clocksource_cyc2ns(delta, clock->mult, clock->shift); > > - /* If arch requires, add in get_arch_timeoffset() */ > + /* If arch requires, add in arch_gettimeoffset() */ > return nsec + arch_gettimeoffset(); > } > > -static inline s64 timekeeping_get_ns_raw(struct timekeeper *tk) > +static inline s64 timekeeping_get_ns(struct tk_read_base *tkr) > { > - struct clocksource *clock = tk->tkr.clock; > - cycle_t cycle_now, delta; > - s64 nsec; > - > - /* read clocksource: */ > - cycle_now = tk->tkr.read(clock); > - > - /* calculate the delta since the last update_wall_time: */ > - delta = clocksource_delta(cycle_now, tk->tkr.cycle_last, > tk->tkr.mask); > - > - /* convert delta to nanoseconds. */ > - nsec = clocksource_cyc2ns(delta, clock->mult, clock->shift); > - > - /* If arch requires, add in get_arch_timeoffset() */ > - return nsec + arch_gettimeoffset(); > + return timekeeping_get_ns_raw(tkr, false) + > + (tkr->xtime_nsec >> tkr->shift); This seems broken. The raw clock is not ntp addjusted, but here you're trying to convert it to the system time by just adding the nsec portion of xtime_nsec to it? I think you've misunderstood the semantics of the raw clock, and why its managed separately from the monotonic clock base other clockids are based off of. It really is a separate time domain. thanks -john -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/

