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

[Ingo Molnar]

* Andi Kleen <ak@xxxxxxx> 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!  But i 
agree that loops_per_jiffy is the wrong metric to use.

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?

[detail: i moved the __delay() after the second attempted trylock, this 
way we'll have 2 trylocks without a delay - for ultra-short critical 
sections.]

	Ingo

----
fix spinlock debugging delays to not time out too early.
Bug found by Andi Kleen.

Signed-off-by: Ingo Molnar <mingo@xxxxxxx>

--- linux/lib/spinlock_debug.c.orig
+++ linux/lib/spinlock_debug.c
@@ -72,9 +72,9 @@ static void __spin_lock_debug(spinlock_t
 
 	for (;;) {
 		for (i = 0; i < loops_per_jiffy * HZ; i++) {
-			cpu_relax();
 			if (__raw_spin_trylock(&lock->raw_lock))
 				return;
+			__delay(1);
 		}
 		/* lockup suspected: */
 		if (print_once) {
@@ -144,9 +144,9 @@ static void __read_lock_debug(rwlock_t *
 
 	for (;;) {
 		for (i = 0; i < loops_per_jiffy * HZ; i++) {
-			cpu_relax();
 			if (__raw_read_trylock(&lock->raw_lock))
 				return;
+			__delay(1);
 		}
 		/* lockup suspected: */
 		if (print_once) {
@@ -217,9 +217,9 @@ static void __write_lock_debug(rwlock_t 
 
 	for (;;) {
 		for (i = 0; i < loops_per_jiffy * HZ; i++) {
-			cpu_relax();
 			if (__raw_write_trylock(&lock->raw_lock))
 				return;
+			__delay(1);
 		}
 		/* lockup suspected: */
 		if (print_once) {
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/