NAK. This is not a busy wait loop, and thus does not justify this optimization.
The ACPI global lock is used to coordinate between the firmware and the OS. It has a specific format dictated by the ACPI spec where bit 1 is OWNED and bit 0 is PENDING. When the OS finds the lock owned, it sets the PENDING bit and returns -- it does not loop waiting for OWNED to clear. The reason that there is a cmpxchg() around writing the lock is only for the critical section where the firmware changed the lock between when we read the lock and when we wrote it. While you could construct a firmware test to thrash this lock, and make this loop more, that isn't how the lock is used -- it is used to guard (relatively high latency) access to shared hardware registers. thanks, -Len >-----Original Message----- >From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] >Sent: Friday, June 30, 2006 5:15 AM >To: Brown, Len >Cc: [email protected]; [EMAIL PROTECTED]; >[EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED] >Subject: [patch 5/6] cpu_relax(): use in ACPI lock > >From: Andreas Mohr <[EMAIL PROTECTED]> > >Use cpu_relax() in __acpi_acquire_global_lock() etc. > >[EMAIL PROTECTED]: build fix] >Signed-off-by: Andreas Mohr <[EMAIL PROTECTED]> >Cc: "Brown, Len" <[EMAIL PROTECTED]> >Signed-off-by: Ingo Molnar <[EMAIL PROTECTED]> >Signed-off-by: Andrew Morton <[EMAIL PROTECTED]> >--- > > include/asm-i386/acpi.h | 15 +++++++++++---- > include/asm-x86_64/acpi.h | 14 ++++++++++---- > 2 files changed, 21 insertions(+), 8 deletions(-) > >diff -puN include/asm-i386/acpi.h~cpu_relax-use-in-acpi-lock >include/asm-i386/acpi.h >--- a/include/asm-i386/acpi.h~cpu_relax-use-in-acpi-lock >+++ a/include/asm-i386/acpi.h >@@ -31,6 +31,7 @@ > #include <acpi/pdc_intel.h> > > #include <asm/system.h> /* defines cmpxchg */ >+#include <asm/processor.h> /* cpu_relax() */ > > #define COMPILER_DEPENDENT_INT64 long long > #define COMPILER_DEPENDENT_UINT64 unsigned long long >@@ -61,11 +62,14 @@ static inline int > __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 +77,14 @@ static inline int > __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 -puN include/asm-x86_64/acpi.h~cpu_relax-use-in-acpi-lock >include/asm-x86_64/acpi.h >--- a/include/asm-x86_64/acpi.h~cpu_relax-use-in-acpi-lock >+++ a/include/asm-x86_64/acpi.h >@@ -59,11 +59,14 @@ static inline int > __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; > } > >@@ -71,11 +74,14 @@ static inline int > __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; > } > >_ > - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
