Re: [PATCH] fix get_jiffies_64 to work on voyager

[Andrew Morton <akpm@osdl.org>]

Linus Torvalds <torvalds@osdl.org> wrote:
>
> 
> 
> On Tue, 6 Jan 2004, Andrew Morton wrote:
> > 
> > Hm, OK.  I hit the same deadlock when running with the "don't require TSCs
> > to be synchronised in sched_clock()" patch from -mm.  The fix for that is
> > below.  I shall accelerate it.
> > 
> > --- 25/arch/i386/kernel/timers/timer_tsc.c~sched_clock-2.6.0-A1-deadlock-fix	2003-12-30 00:45:09.000000000 -0800
> > +++ 25-akpm/arch/i386/kernel/timers/timer_tsc.c	2003-12-30 00:45:09.000000000 -0800
> > @@ -140,7 +140,8 @@ unsigned long long sched_clock(void)
> >  #ifndef CONFIG_NUMA
> >  	if (!use_tsc)
> >  #endif
> > -		return (unsigned long long)get_jiffies_64() * (1000000000 / HZ);
> > +		/* jiffies might overflow but this is not a big deal here */
> > +		return (unsigned long long)jiffies * (1000000000 / HZ);
> 
> Augh. If you cast it to "unsigned long long" anyway, why not just use the 
> right value? It's "jiffies_64".

Sounds sane.

> It has other problems, of course, but at least that makes them slightly 
> less.

Yes, it's slightly sleazy.

I haven't tested this...


From: Ingo Molnar <mingo@elte.hu>

Voyager is getting odd deadlocks due to the taking of xtime_lock() in
sched_clock()->get_jiffies_64().

I had this patch queued up to fix a different deadlock, which ocurs when we
relax the requirement that TSC's be synchronised across CPUs.  But it will
fix James' deadlock too.




 arch/i386/kernel/timers/timer_tsc.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletion(-)

diff -puN arch/i386/kernel/timers/timer_tsc.c~sched_clock-2.6.0-A1-deadlock-fix arch/i386/kernel/timers/timer_tsc.c
--- 25/arch/i386/kernel/timers/timer_tsc.c~sched_clock-2.6.0-A1-deadlock-fix	2004-01-06 08:56:36.000000000 -0800
+++ 25-akpm/arch/i386/kernel/timers/timer_tsc.c	2004-01-06 08:58:37.000000000 -0800
@@ -140,7 +140,12 @@ unsigned long long sched_clock(void)
 #ifndef CONFIG_NUMA
 	if (!use_tsc)
 #endif
-		return (unsigned long long)get_jiffies_64() * (1000000000 / HZ);
+		/*
+		 * Avoid xtime_lock deadlocks by not locking the read of
+		 * jiffies_64.  Can possibly cause glitches on 32-bit rollovers
+		 * but the CPU scheduler will recover OK.
+		 */
+		return jiffies_64 * (1000000000 / HZ);
 
 	/* Read the Time Stamp Counter */
 	rdtscll(this_offset);

_