RE: [PATCH -mm 5/6] cpu_relax(): use in ACPI lock

RE: [PATCH -mm 5/6] cpu_relax(): use in ACPI lock

[Moore, Robert]

Loop may be correctly waiting for the owner/pending bit to be set, just
checking, I don't remember all the bit values.

> -----Original Message-----
> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> owner@vger.kernel.org] On Behalf Of Moore, Robert
> Sent: Wednesday, June 21, 2006 2:09 PM
> To: Andreas Mohr; Andrew Morton
> Cc: Brown, Len; linux-acpi@vger.kernel.org;
linux-kernel@vger.kernel.org
> Subject: RE: [PATCH -mm 5/6] cpu_relax(): use in ACPI lock
> 
> I may be interpreting this incorrectly, but are you busy-waiting on
the
> ACPI Global Lock to become free?
> 
> Bob
> 
> 
> > -----Original Message-----
> > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> > owner@vger.kernel.org] On Behalf Of Andreas Mohr
> > Sent: Wednesday, June 21, 2006 2:01 PM
> > To: Andrew Morton
> > Cc: Brown, Len; linux-acpi@vger.kernel.org;
> linux-kernel@vger.kernel.org
> > Subject: [PATCH -mm 5/6] cpu_relax(): use in ACPI lock
> >
> >
> > Use cpu_relax() in __acpi_acquire_global_lock() etc.
> >
> >

RE: [PATCH -mm 5/6] cpu_relax(): use in ACPI lock

[Brown, Len]

>> I may be interpreting this incorrectly, but are you 
>> busy-waiting on the ACPI Global Lock to become free?

No.

>Loop may be correctly waiting for the owner/pending bit to be 
>set, just checking, I don't remember all the bit values.

Yes.

__acpi_acquire_global_lock is basically a lock-try
and set the pending bit on failure.

bit 0 is the PENDING bit
bit 1 is the OWNED bit

so the loop is doing this:

	old = lock_value
try_again:
	new = OWNED
	if (old & OWNED)
		new |= PENDING
	cmpxchg(lock_value, old, new)
	if (old != new) goto try_again;

	return(!(new & PENDING)) /* ACQUIRED or not */


so we loop only if somebody else changed the lock_value
to be different from old at the same time this code did.

if the lock were held, we simply set the pending bit
and return that we failed to acquire the lock.

-Len

RE: [PATCH -mm 5/6] cpu_relax(): use in ACPI lock

[Moore, Robert]

I may be interpreting this incorrectly, but are you busy-waiting on the
ACPI Global Lock to become free?

Bob


> -----Original Message-----
> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> owner@vger.kernel.org] On Behalf Of Andreas Mohr
> Sent: Wednesday, June 21, 2006 2:01 PM
> To: Andrew Morton
> Cc: Brown, Len; linux-acpi@vger.kernel.org;
linux-kernel@vger.kernel.org
> Subject: [PATCH -mm 5/6] cpu_relax(): use in ACPI lock
> 
> 
> Use cpu_relax() in __acpi_acquire_global_lock() etc.
> 
> 
> This could be considered overkill given the previous unlikely(),
> but it is busy-looping in case of false condition after all...
> 
> Tested on 2.6.17-mm1, i386 only (no x86_64 here).
> 
> Signed-off-by: Andreas Mohr <andi@lisas.de>
> 
> 
> diff -urN linux-2.6.17-mm1.orig/include/asm-i386/acpi.h linux-2.6.17-
> mm1.my/include/asm-i386/acpi.h
> --- linux-2.6.17-mm1.orig/include/asm-i386/acpi.h	2006-06-19
> 10:57:27.000000000 +0200
> +++ linux-2.6.17-mm1.my/include/asm-i386/acpi.h	2006-06-21
> 14:43:24.000000000 +0200
> @@ -61,11 +61,14 @@
>  __acpi_acquire_global_lock (unsigned int *lock)
>  {
>  	unsigned int old, new, val;
> -	do {
> +	while (1) {
>  		old = *lock;
>  		new = (((old & ~0x3) + 2) + ((old >> 1) & 0x1));
>  		val = cmpxchg(lock, old, new);
> -	} while (unlikely (val != old));
> +		if (likely(val == old))
> +			break;
> +		cpu_relax();
> +	}
>  	return (new < 3) ? -1 : 0;
>  }
> 
> @@ -73,11 +76,14 @@
>  __acpi_release_global_lock (unsigned int *lock)
>  {
>  	unsigned int old, new, val;
> -	do {
> +	while (1) {
>  		old = *lock;
>  		new = old & ~0x3;
>  		val = cmpxchg(lock, old, new);
> -	} while (unlikely (val != old));
> +		if (likely(val == old))
> +			break;
> +		cpu_relax();
> +	}
>  	return old & 0x1;
>  }
> 
> diff -urN linux-2.6.17-mm1.orig/include/asm-x86_64/acpi.h
linux-2.6.17-
> mm1.my/include/asm-x86_64/acpi.h
> --- linux-2.6.17-mm1.orig/include/asm-x86_64/acpi.h	2006-06-21
> 14:28:19.000000000 +0200
> +++ linux-2.6.17-mm1.my/include/asm-x86_64/acpi.h	2006-06-21
> 14:43:24.000000000 +0200
> @@ -59,11 +59,14 @@
>  __acpi_acquire_global_lock (unsigned int *lock)
>  {
>  	unsigned int old, new, val;
> -	do {
> +	while (1) {
>  		old = *lock;
>  		new = (((old & ~0x3) + 2) + ((old >> 1) & 0x1));
>  		val = cmpxchg(lock, old, new);
> -	} while (unlikely (val != old));
> +		if (likely(val == old))
> +			break;
> +		cpu_relax();
> +	}
>  	return (new < 3) ? -1 : 0;
>  }
> 
> @@ -75,7 +78,10 @@
>  		old = *lock;
>  		new = old & ~0x3;
>  		val = cmpxchg(lock, old, new);
> -	} while (unlikely (val != old));
> +		if (likely(val == old))
> +			break;
> +		cpu_relax();
> +	}
>  	return old & 0x1;
>  }
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-acpi"
in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH -mm 5/6] cpu_relax(): use in ACPI lock

[Andreas Mohr]

Use cpu_relax() in __acpi_acquire_global_lock() etc.


This could be considered overkill given the previous unlikely(),
but it is busy-looping in case of false condition after all...

Tested on 2.6.17-mm1, i386 only (no x86_64 here).

Signed-off-by: Andreas Mohr <andi@lisas.de>


diff -urN linux-2.6.17-mm1.orig/include/asm-i386/acpi.h linux-2.6.17-mm1.my/include/asm-i386/acpi.h
--- linux-2.6.17-mm1.orig/include/asm-i386/acpi.h	2006-06-19 10:57:27.000000000 +0200
+++ linux-2.6.17-mm1.my/include/asm-i386/acpi.h	2006-06-21 14:43:24.000000000 +0200
@@ -61,11 +61,14 @@
 __acpi_acquire_global_lock (unsigned int *lock)
 {
 	unsigned int old, new, val;
-	do {
+	while (1) {
 		old = *lock;
 		new = (((old & ~0x3) + 2) + ((old >> 1) & 0x1));
 		val = cmpxchg(lock, old, new);
-	} while (unlikely (val != old));
+		if (likely(val == old))
+			break;
+		cpu_relax();
+	}
 	return (new < 3) ? -1 : 0;
 }
 
@@ -73,11 +76,14 @@
 __acpi_release_global_lock (unsigned int *lock)
 {
 	unsigned int old, new, val;
-	do {
+	while (1) {
 		old = *lock;
 		new = old & ~0x3;
 		val = cmpxchg(lock, old, new);
-	} while (unlikely (val != old));
+		if (likely(val == old))
+			break;
+		cpu_relax();
+	}
 	return old & 0x1;
 }
 
diff -urN linux-2.6.17-mm1.orig/include/asm-x86_64/acpi.h linux-2.6.17-mm1.my/include/asm-x86_64/acpi.h
--- linux-2.6.17-mm1.orig/include/asm-x86_64/acpi.h	2006-06-21 14:28:19.000000000 +0200
+++ linux-2.6.17-mm1.my/include/asm-x86_64/acpi.h	2006-06-21 14:43:24.000000000 +0200
@@ -59,11 +59,14 @@
 __acpi_acquire_global_lock (unsigned int *lock)
 {
 	unsigned int old, new, val;
-	do {
+	while (1) {
 		old = *lock;
 		new = (((old & ~0x3) + 2) + ((old >> 1) & 0x1));
 		val = cmpxchg(lock, old, new);
-	} while (unlikely (val != old));
+		if (likely(val == old))
+			break;
+		cpu_relax();
+	}
 	return (new < 3) ? -1 : 0;
 }
 
@@ -75,7 +78,10 @@
 		old = *lock;
 		new = old & ~0x3;
 		val = cmpxchg(lock, old, new);
-	} while (unlikely (val != old));
+		if (likely(val == old))
+			break;
+		cpu_relax();
+	}
 	return old & 0x1;
 }