On Thu, 2009-03-12 at 02:30 +0100, Thomas Gleixner wrote: > On Wed, 11 Mar 2009, john stultz wrote: > > For a cleaner version, could you try the following, against 2.6.29-git > > with no other modification? > > cleaner ? > > > xtime_nsec is expected at times to be negative. Instead of trying to > > handle all the shifting properly via casts, define it as a s64 instead > > of a u64. > > > > NOT FOR INCLUSION > > Signed-off-by: John Stultz <johnstul@xxxxxxxxxx> > > > > diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h > > index f88d32f..e217000 100644 > > --- a/include/linux/clocksource.h > > +++ b/include/linux/clocksource.h > > @@ -86,7 +86,7 @@ struct clocksource { > > * more than one cache line. > > */ > > cycle_t cycle_last ____cacheline_aligned_in_smp; > > - u64 xtime_nsec; > > + s64 xtime_nsec; > > s64 error; > > struct timespec raw_time; > > > > diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c > > index 900f1b6..387be3c 100644 > > --- a/kernel/time/timekeeping.c > > +++ b/kernel/time/timekeeping.c > > @@ -546,7 +546,7 @@ void update_wall_time(void) > > /* store full nanoseconds into xtime after rounding it up and > > * add the remainder to the error difference. > > */ > > - xtime.tv_nsec = ((s64)clock->xtime_nsec >> clock->shift) + 1; > > + xtime.tv_nsec = (clock->xtime_nsec >> clock->shift) + 1; > > clock->xtime_nsec -= (s64)xtime.tv_nsec << clock->shift; > > clock->error += clock->xtime_nsec << (NTP_SCALE_SHIFT - clock->shift); > > This code sequence does: > > xtime.tv_nsec = (clock->xtime_nsec >> clock->shift) + 1; > clock->xtime_nsec -= xtime.tv_nsec << clock->shift; > > Lets substitute a bit: > > a = xtime.tv_nsec > b = clock->xtime_nsec > c = clock->shift > r = result (which ends up in b aka clock->xtime_nsec again for the next round) > > => a = (b >> c) + 1 > r = b - (a << c) > > b >> c = b / 2^c > a << c = a * 2^c > > => a = (b / 2^c) + 1 > r = b - (a * 2^c) > > => r = b - ((b / 2^c) + 1) * 2^c > r = b - ((2^c * b / 2^c) + 2^c) > r = b - (2^c * b / 2^c) - 2^c > r = b - b - 2^c > r = -2^c > > => r = -(1 << c) > > So the whole business boils down to: > > clock->xtime_nsec = -(1 << clock->shift); Err, not quite. See, we truncate clock->xtime_nsec when we shift it down and store it into xtime.tv_nsec. Since we don't want to lose truncated remainder, we simply add one to xtime.tv_nsec, taking the ceiling in effect. However, we have to balance this out in order to not rush forward in time each tick. So we accumulate amount we ended up adding into the error. Lets walk through a simple example with actual values. b = 99 c = 4 => a = (b >> c) + 1 r = b - (a << c) => a = (99 >> 4) + 1 a = 6 + 1 a = 7 r = 99 - (7 << 4) r = 99 - 112 r = -13 (Your reduction would give -16, since it ignores that shift down truncates) In effect, adding 1 to xtime.tv_nsec, is the same as adding 13 to clock->xtime_nsec. Maybe a different way of expressing what we're calculating is the following: xtime.tv_nsec = (clock->xtime_nsec + (1<<clock->shift)) >> clock->shift clock->xtime_nsec = (1<<clock->shift) - (clock->xtime_nsec &((1<<clock->shift)-1) In other words: The rounded up portion - the masked remainder in xtime_nsec. Does that make sense? thanks -john -- To unsubscribe from this list: send the line "unsubscribe linux-s390" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html