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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090814101013.43e88657@skybase>
Date:	Fri, 14 Aug 2009 10:10:13 +0200
From:	Martin Schwidefsky <schwidefsky@...ibm.com>
To:	john stultz <johnstul@...ibm.com>
Cc:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
	Thomas Gleixner <tglx@...utronix.de>,
	Daniel Walker <dwalker@...o99.com>
Subject: Re: [patch 02/14] remove clocksource inline functions

On Thu, 13 Aug 2009 15:14:34 -0700
john stultz <johnstul@...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.

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