Re: [RFC PATCH 16/22 -v2] add get_monotonic_cycles

john stultz
2008-01-17T02:28:31Z

On Wed, 2008-01-16 at 18:33 -0500, Steven Rostedt wrote:
> Thanks John for doing this!
>
> (comments imbedded)
>
> On Wed, 16 Jan 2008, john stultz wrote:
> > + int num = !cs->base_num;
> > + cycle_t offset = (now - cs->base[!num].cycle_base_last);
> > + offset &= cs->mask;
> > + cs->base[num].cycle_base = cs->base[!num].cycle_base + offset;
> > + cs->base[num].cycle_base_last = now;
>
> I would think that we would need some sort of barrier here. Otherwise,
> base_num could be updated before all the cycle_base. I'd expect a smp_wmb
> is needed.

Hopefully addressed in the current version.

> > Index: monotonic-cleanup/kernel/time/timekeeping.c
> > ===================================================================
> > --- monotonic-cleanup.orig/kernel/time/timekeeping.c 2008-01-16 12:21:46.000000000 -0800
> > +++ monotonic-cleanup/kernel/time/timekeeping.c 2008-01-16 14:15:31.000000000 -0800
> > @@ -71,10 +71,12 @@
> > */
> > static inline s64 __get_nsec_offset(void)
> > {
> > - cycle_t cycle_delta;
> > + cycle_t now, cycle_delta;
> > s64 ns_offset;
> >
> > - cycle_delta = clocksource_get_cycles(clock, clocksource_read(clock));
> > + now = clocksource_read(clock);
> > + cycle_delta = (now - clock->cycle_last) & clock->mask;
> > + cycle_delta += clock->cycle_accumulated;
>
> Is the above just to decouple the two methods?

Yep. clocksource_get_cycles() ended up not being as useful as an helper
function (I was hoping the arch vsyscall implementations could use it,
but they've done too much optimization - although that may reflect a
need up the chain to the clocksource structure).

thanks
-john