Re: [PATCH] Prevent spinlock debug from timing out too early

[Andi Kleen <ak@suse.de>]

On Monday 06 February 2006 22:36, Ingo Molnar wrote:
> 
> * Andi Kleen <ak@suse.de> wrote:
> 
> > Index: linux-2.6.15/lib/spinlock_debug.c
> > ===================================================================
> > --- linux-2.6.15.orig/lib/spinlock_debug.c
> > +++ linux-2.6.15/lib/spinlock_debug.c
> > @@ -68,13 +68,13 @@ static inline void debug_spin_unlock(spi
> >  static void __spin_lock_debug(spinlock_t *lock)
> >  {
> >  	int print_once = 1;
> > -	u64 i;
> >  
> >  	for (;;) {
> > -		for (i = 0; i < loops_per_jiffy * HZ; i++) {
> > -			cpu_relax();
> > +		unsigned long timeout = jiffies + HZ;
> > +		while (time_before(jiffies, timeout)) {
> >  			if (__raw_spin_trylock(&lock->raw_lock))
> >  				return;
> > +			cpu_relax();
> 
> The reason i added a loop counter was to solve the case where we are 
> spinning with interrupts disabled - jiffies wont increase there! 

Yes but the NMI watchdog should catch it eventually

[we really should enable it by default on i386 too - local APIC 
NMI should work everywhere with APIC]

Oops I missed the write lock case. Thanks.

 
> a better solution would be to call __delay(1) after the first failed 
> attempt, that would make the delay at least 1 second long. It seems 
> __delay() is de-facto exported by every architecture, so we can rely on 
> it in the global spinlock code.
> 
> So how about the patch below instead?

Are you sure loops_per_jiffie is always in delay(1) units?


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