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

[john stultz]

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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/