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: akpm@xxxxxxxx [mailto:akpm@xxxxxxxx] >Sent: Friday, June 30, 2006 5:15 AM >To: Brown, Len >Cc: linux-acpi@xxxxxxxxxxxxxxx; akpm@xxxxxxxx; >andi@xxxxxxxxxxxxxxxxxxxxxxx; andi@xxxxxxxx; mingo@xxxxxxx >Subject: [patch 5/6] cpu_relax(): use in ACPI lock > >From: Andreas Mohr <andi@xxxxxxxxxxxxxxxxxxxxxxx> > >Use cpu_relax() in __acpi_acquire_global_lock() etc. > >[mingo@xxxxxxx: build fix] >Signed-off-by: Andreas Mohr <andi@xxxxxxxx> >Cc: "Brown, Len" <len.brown@xxxxxxxxx> >Signed-off-by: Ingo Molnar <mingo@xxxxxxx> >Signed-off-by: Andrew Morton <akpm@xxxxxxxx> >--- > > 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
