Re: [patch 02/14] remove clocksource inline functions

[Martin Schwidefsky <schwidefsky@de.ibm.com>]

On Thu, 13 Aug 2009 15:14:34 -0700
john stultz <johnstul@us.ibm.com> wrote:

> On Thu, 2009-08-13 at 17:40 +0200, Martin Schwidefsky wrote:
> > Index: linux-2.6/kernel/time/timekeeping.c
> > ===================================================================
> > --- linux-2.6.orig/kernel/time/timekeeping.c
> > +++ linux-2.6/kernel/time/timekeeping.c
> [snip]
> > @@ -477,8 +492,7 @@ static int timekeeping_resume(struct sys
> >  	}
> >  	update_xtime_cache(0);
> >  	/* re-base the last cycle value */
> > -	clock->cycle_last = 0;
> > -	clock->cycle_last = clocksource_read(clock);
> > +	clock->cycle_last = clock->read(clock);
> 
> Minor bug here, the clearing of cycle_last has a side-effect of making
> sure the TSC doesn't trip over its own cycle_last checking in the read()
> function. This is part of the uglyness of the TSC pulling this internal
> timeekping state to avoid minor inconsistencies, but until we find a
> better way, we have to live with it.
> 
> So you'll need to preserve the cycle_last = 0 line.

Whoa, now that is subtle. Good spotting. I would prefer to reset the 
cycle_last in a resume function though:

Index: linux-2.6/arch/x86/kernel/tsc.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/tsc.c
+++ linux-2.6/arch/x86/kernel/tsc.c
@@ -744,10 +744,16 @@ static cycle_t __vsyscall_fn vread_tsc(v
 }
 #endif

+static void resume_tsc(void)
+{
+       clocksource_tsc.cycle_last = 0;
+}
+
 static struct clocksource clocksource_tsc = {
        .name                   = "tsc",
        .rating                 = 300,
        .read                   = read_tsc,
+       .resume                 = resume_tsc,
        .mask                   = CLOCKSOURCE_MASK(64),
        .shift                  = 22,
        .flags                  = CLOCK_SOURCE_IS_CONTINUOUS |

That puts the subtlety where it belongs.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.