[PATCH 0/3] ACPI/ACPICA: Resolve spinlock and global lock handling code divergence

[PATCH 0/3] ACPI/ACPICA: Resolve spinlock and global lock handling code divergence

[Lin Ming]

Currently, linux osl code does not implement the interface acpi_os_create_lock.
This series implements and uses it. So the spinlock code divergence is fixed.

Also, fix the code divergence introduced by commit
9cd0314(ACPI / ACPICA: Fix global lock acquisition).

[PATCH 1/3] ACPI: osl, add acpi_os_create_lock interface
[PATCH 2/3] ACPICA: Use acpi_os_create_lock interface
[PATCH 3/3] ACPICA: Fix code divergence of global lock handling

Thanks,
Lin Ming

[PATCH 1/3] ACPI: osl, add acpi_os_create_lock interface

[Lin Ming]

Signed-off-by: Lin Ming <ming.m.lin@intel.com>
---
 drivers/acpi/osl.c      |   33 +++++++++++++++++++++++++--------
 include/acpi/acpiosxf.h |    3 +++
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 45ad4ff..52ca964 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -902,14 +902,6 @@ void acpi_os_wait_events_complete(void *context)
 
 EXPORT_SYMBOL(acpi_os_wait_events_complete);
 
-/*
- * Deallocate the memory for a spinlock.
- */
-void acpi_os_delete_lock(acpi_spinlock handle)
-{
-	return;
-}
-
 acpi_status
 acpi_os_create_semaphore(u32 max_units, u32 initial_units, acpi_handle * handle)
 {
@@ -1341,6 +1333,31 @@ int acpi_resources_are_enforced(void)
 EXPORT_SYMBOL(acpi_resources_are_enforced);
 
 /*
+ * Create and initialize a spinlock.
+ */
+acpi_status
+acpi_os_create_lock(acpi_spinlock *out_handle)
+{
+	spinlock_t *lock;
+
+	lock = ACPI_ALLOCATE(sizeof(spinlock_t));
+	if (!lock)
+		return AE_NO_MEMORY;
+	spin_lock_init(lock);
+	*out_handle = lock;
+
+	return AE_OK;
+}
+
+/*
+ * Deallocate the memory for a spinlock.
+ */
+void acpi_os_delete_lock(acpi_spinlock handle)
+{
+	ACPI_FREE(handle);
+}
+
+/*
  * Acquire a spinlock.
  *
  * handle is a pointer to the spinlock_t.
diff --git a/include/acpi/acpiosxf.h b/include/acpi/acpiosxf.h
index a3252a5..a756bc8 100644
--- a/include/acpi/acpiosxf.h
+++ b/include/acpi/acpiosxf.h
@@ -98,6 +98,9 @@ acpi_os_table_override(struct acpi_table_header *existing_table,
 /*
  * Spinlock primitives
  */
+acpi_status
+acpi_os_create_lock(acpi_spinlock *out_handle);
+
 void acpi_os_delete_lock(acpi_spinlock handle);
 
 acpi_cpu_flags acpi_os_acquire_lock(acpi_spinlock handle);
-- 
1.7.3

[PATCH 2/3] ACPICA: Use acpi_os_create_lock interface

[Lin Ming]

Replace spin_lock_init with acpi_os_create_lock.

Signed-off-by: Lin Ming <ming.m.lin@intel.com>
---
 drivers/acpi/acpica/acglobal.h |    9 +++------
 drivers/acpi/acpica/utmutex.c  |   17 ++++++++++++++---
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
index d69750b..6d512fc 100644
--- a/drivers/acpi/acpica/acglobal.h
+++ b/drivers/acpi/acpica/acglobal.h
@@ -226,12 +226,9 @@ ACPI_EXTERN u8 acpi_gbl_global_lock_present;
  * Spinlocks are used for interfaces that can be possibly called at
  * interrupt level
  */
-ACPI_EXTERN spinlock_t _acpi_gbl_gpe_lock;	/* For GPE data structs and registers */
-ACPI_EXTERN spinlock_t _acpi_gbl_hardware_lock;	/* For ACPI H/W except GPE registers */
-ACPI_EXTERN spinlock_t _acpi_ev_global_lock_pending_lock; /* For global lock */
-#define acpi_gbl_gpe_lock	&_acpi_gbl_gpe_lock
-#define acpi_gbl_hardware_lock	&_acpi_gbl_hardware_lock
-#define acpi_ev_global_lock_pending_lock &_acpi_ev_global_lock_pending_lock
+ACPI_EXTERN acpi_spinlock acpi_gbl_gpe_lock;	/* For GPE data structs and registers */
+ACPI_EXTERN acpi_spinlock acpi_gbl_hardware_lock;	/* For ACPI H/W except GPE registers */
+ACPI_EXTERN acpi_spinlock acpi_ev_global_lock_pending_lock; /* For global lock */
 
 /*****************************************************************************
  *
diff --git a/drivers/acpi/acpica/utmutex.c b/drivers/acpi/acpica/utmutex.c
index a946c68..519d4ee 100644
--- a/drivers/acpi/acpica/utmutex.c
+++ b/drivers/acpi/acpica/utmutex.c
@@ -83,9 +83,20 @@ acpi_status acpi_ut_mutex_initialize(void)
 
 	/* Create the spinlocks for use at interrupt level */
 
-	spin_lock_init(acpi_gbl_gpe_lock);
-	spin_lock_init(acpi_gbl_hardware_lock);
-	spin_lock_init(acpi_ev_global_lock_pending_lock);
+	status = acpi_os_create_lock (&acpi_gbl_gpe_lock);
+	if (ACPI_FAILURE (status)) {
+		return_ACPI_STATUS (status);
+	}
+
+	status = acpi_os_create_lock (&acpi_gbl_hardware_lock);
+	if (ACPI_FAILURE (status)) {
+		return_ACPI_STATUS (status);
+	}
+
+	status = acpi_os_create_lock (&acpi_ev_global_lock_pending_lock);
+	if (ACPI_FAILURE (status)) {
+		return_ACPI_STATUS (status);
+	}
 
 	/* Mutex for _OSI support */
 	status = acpi_os_create_mutex(&acpi_gbl_osi_mutex);
-- 
1.7.3

[PATCH 3/3] ACPICA: Fix code divergence of global lock handling

[Lin Ming]

Commit 9cd0314(ACPI / ACPICA: Fix global lock acquisition) was backported
into ACPICA code base, and some divergence was introduced.

This patch fixed it,
- rename acpi_ev_global_lock_pending/acpi_ev_global_lock_pending_lock
  to acpi_gbl_global_lock_pending/acpi_gbl_global_lock_pending_lock.

- move the initialization of acpi_gbl_global_lock_pending_lock from
  acpi_ut_mutex_initialize to acpi_ev_init_global_lock_handler.

Signed-off-by: Lin Ming <ming.m.lin@intel.com>
---
 drivers/acpi/acpica/acglobal.h |    6 ++-
 drivers/acpi/acpica/evmisc.c   |   74 +++++++++++++++++++++------------------
 drivers/acpi/acpica/utmutex.c  |    5 ---
 3 files changed, 44 insertions(+), 41 deletions(-)

diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
index 6d512fc..73863d8 100644
--- a/drivers/acpi/acpica/acglobal.h
+++ b/drivers/acpi/acpica/acglobal.h
@@ -214,13 +214,16 @@ ACPI_EXTERN struct acpi_mutex_info acpi_gbl_mutex_info[ACPI_NUM_MUTEX];
 
 /*
  * Global lock mutex is an actual AML mutex object
- * Global lock semaphore works in conjunction with the HW global lock
+ * Global lock semaphore works in conjunction with the actual global lock
+ * Global lock spinlock is used for "pending" handshake
  */
 ACPI_EXTERN union acpi_operand_object *acpi_gbl_global_lock_mutex;
 ACPI_EXTERN acpi_semaphore acpi_gbl_global_lock_semaphore;
+ACPI_EXTERN acpi_spinlock acpi_gbl_global_lock_pending_lock;
 ACPI_EXTERN u16 acpi_gbl_global_lock_handle;
 ACPI_EXTERN u8 acpi_gbl_global_lock_acquired;
 ACPI_EXTERN u8 acpi_gbl_global_lock_present;
+ACPI_EXTERN u8 acpi_gbl_global_lock_pending;
 
 /*
  * Spinlocks are used for interfaces that can be possibly called at
@@ -228,7 +231,6 @@ ACPI_EXTERN u8 acpi_gbl_global_lock_present;
  */
 ACPI_EXTERN acpi_spinlock acpi_gbl_gpe_lock;	/* For GPE data structs and registers */
 ACPI_EXTERN acpi_spinlock acpi_gbl_hardware_lock;	/* For ACPI H/W except GPE registers */
-ACPI_EXTERN acpi_spinlock acpi_ev_global_lock_pending_lock; /* For global lock */
 
 /*****************************************************************************
  *
diff --git a/drivers/acpi/acpica/evmisc.c b/drivers/acpi/acpica/evmisc.c
index 7dc8094..69a3b4a 100644
--- a/drivers/acpi/acpica/evmisc.c
+++ b/drivers/acpi/acpica/evmisc.c
@@ -284,39 +284,41 @@ static void ACPI_SYSTEM_XFACE acpi_ev_notify_dispatch(void *context)
  * RETURN:      ACPI_INTERRUPT_HANDLED
  *
  * DESCRIPTION: Invoked directly from the SCI handler when a global lock
- *              release interrupt occurs.  If there's a thread waiting for
- *              the global lock, signal it.
- *
- * NOTE: Assumes that the semaphore can be signaled from interrupt level. If
- * this is not possible for some reason, a separate thread will have to be
- * scheduled to do this.
+ *              release interrupt occurs. If there is actually a pending
+ *              request for the lock, signal the waiting thread.
  *
  ******************************************************************************/
-static u8 acpi_ev_global_lock_pending;
 
 static u32 acpi_ev_global_lock_handler(void *context)
 {
 	acpi_status status;
 	acpi_cpu_flags flags;
 
-	flags = acpi_os_acquire_lock(acpi_ev_global_lock_pending_lock);
+	flags = acpi_os_acquire_lock(acpi_gbl_global_lock_pending_lock);
 
-	if (!acpi_ev_global_lock_pending) {
-		goto out;
+	/*
+	 * If a request for the global lock is not actually pending,
+	 * we are done. This handles "spurious" global lock interrupts
+	 * which are possible (and have been seen) with bad BIOSs.
+	 */
+	if (!acpi_gbl_global_lock_pending) {
+		goto cleanup_and_exit;
 	}
 
-	/* Send a unit to the semaphore */
-
+	/*
+	 * Send a unit to the global lock semaphore. The actual acquisition
+	 * of the global lock will be performed by the waiting thread.
+	 */
 	status = acpi_os_signal_semaphore(acpi_gbl_global_lock_semaphore, 1);
 	if (ACPI_FAILURE(status)) {
 		ACPI_ERROR((AE_INFO, "Could not signal Global Lock semaphore"));
 	}
 
-	acpi_ev_global_lock_pending = FALSE;
+	acpi_gbl_global_lock_pending = FALSE;
 
- out:
-	acpi_os_release_lock(acpi_ev_global_lock_pending_lock, flags);
+cleanup_and_exit:
 
+	acpi_os_release_lock(acpi_gbl_global_lock_pending_lock, flags);
 	return (ACPI_INTERRUPT_HANDLED);
 }
 
@@ -350,14 +352,20 @@ acpi_status acpi_ev_init_global_lock_handler(void)
 	 * Map to AE_OK, but mark global lock as not present. Any attempt to
 	 * actually use the global lock will be flagged with an error.
 	 */
+	acpi_gbl_global_lock_present = FALSE;
 	if (status == AE_NO_HARDWARE_RESPONSE) {
 		ACPI_ERROR((AE_INFO,
 			    "No response from Global Lock hardware, disabling lock"));
 
-		acpi_gbl_global_lock_present = FALSE;
 		return_ACPI_STATUS(AE_OK);
 	}
 
+	status = acpi_os_create_lock(&acpi_gbl_global_lock_pending_lock);
+	if (ACPI_FAILURE(status)) {
+		return_ACPI_STATUS(status);
+	}
+
+	acpi_gbl_global_lock_pending = FALSE;
 	acpi_gbl_global_lock_present = TRUE;
 	return_ACPI_STATUS(status);
 }
@@ -414,7 +422,7 @@ static int acpi_ev_global_lock_acquired;
 acpi_status acpi_ev_acquire_global_lock(u16 timeout)
 {
 	acpi_cpu_flags flags;
-	acpi_status status = AE_OK;
+	acpi_status status;
 	u8 acquired = FALSE;
 
 	ACPI_FUNCTION_TRACE(ev_acquire_global_lock);
@@ -458,15 +466,15 @@ acpi_status acpi_ev_acquire_global_lock(u16 timeout)
 	}
 
 	/*
-	 * Make sure that a global lock actually exists. If not, just treat the
-	 * lock as a standard mutex.
+	 * Make sure that a global lock actually exists. If not, just
+	 * treat the lock as a standard mutex.
 	 */
 	if (!acpi_gbl_global_lock_present) {
 		acpi_gbl_global_lock_acquired = TRUE;
 		return_ACPI_STATUS(AE_OK);
 	}
 
-	flags = acpi_os_acquire_lock(acpi_ev_global_lock_pending_lock);
+	flags = acpi_os_acquire_lock(acpi_gbl_global_lock_pending_lock);
 
 	do {
 
@@ -475,20 +483,19 @@ acpi_status acpi_ev_acquire_global_lock(u16 timeout)
 		ACPI_ACQUIRE_GLOBAL_LOCK(acpi_gbl_FACS, acquired);
 		if (acquired) {
 			acpi_gbl_global_lock_acquired = TRUE;
-
 			ACPI_DEBUG_PRINT((ACPI_DB_EXEC,
 					  "Acquired hardware Global Lock\n"));
 			break;
 		}
 
-		acpi_ev_global_lock_pending = TRUE;
-
-		acpi_os_release_lock(acpi_ev_global_lock_pending_lock, flags);
-
 		/*
-		 * Did not get the lock. The pending bit was set above, and we
-		 * must wait until we get the global lock released interrupt.
+		 * Did not get the lock. The pending bit was set above, and
+		 * we must now wait until we receive the global lock
+		 * released interrupt.
 		 */
+		acpi_gbl_global_lock_pending = TRUE;
+		acpi_os_release_lock(acpi_gbl_global_lock_pending_lock, flags);
+
 		ACPI_DEBUG_PRINT((ACPI_DB_EXEC,
 				  "Waiting for hardware Global Lock\n"));
 
@@ -496,17 +503,16 @@ acpi_status acpi_ev_acquire_global_lock(u16 timeout)
 		 * Wait for handshake with the global lock interrupt handler.
 		 * This interface releases the interpreter if we must wait.
 		 */
-		status = acpi_ex_system_wait_semaphore(
-						acpi_gbl_global_lock_semaphore,
-						ACPI_WAIT_FOREVER);
+		status =
+		    acpi_ex_system_wait_semaphore
+		    (acpi_gbl_global_lock_semaphore, ACPI_WAIT_FOREVER);
 
-		flags = acpi_os_acquire_lock(acpi_ev_global_lock_pending_lock);
+		flags = acpi_os_acquire_lock(acpi_gbl_global_lock_pending_lock);
 
 	} while (ACPI_SUCCESS(status));
 
-	acpi_ev_global_lock_pending = FALSE;
-
-	acpi_os_release_lock(acpi_ev_global_lock_pending_lock, flags);
+	acpi_gbl_global_lock_pending = FALSE;
+	acpi_os_release_lock(acpi_gbl_global_lock_pending_lock, flags);
 
 	return_ACPI_STATUS(status);
 }
diff --git a/drivers/acpi/acpica/utmutex.c b/drivers/acpi/acpica/utmutex.c
index 519d4ee..7d797e2 100644
--- a/drivers/acpi/acpica/utmutex.c
+++ b/drivers/acpi/acpica/utmutex.c
@@ -93,11 +93,6 @@ acpi_status acpi_ut_mutex_initialize(void)
 		return_ACPI_STATUS (status);
 	}
 
-	status = acpi_os_create_lock (&acpi_ev_global_lock_pending_lock);
-	if (ACPI_FAILURE (status)) {
-		return_ACPI_STATUS (status);
-	}
-
 	/* Mutex for _OSI support */
 	status = acpi_os_create_mutex(&acpi_gbl_osi_mutex);
 	if (ACPI_FAILURE(status)) {
-- 
1.7.3

Re: [PATCH 3/3] ACPICA: Fix code divergence of global lock handling

[Rafael J. Wysocki]

On Wednesday, March 23, 2011, Lin Ming wrote:
> Commit 9cd0314(ACPI / ACPICA: Fix global lock acquisition) was backported
> into ACPICA code base, and some divergence was introduced.
> 
> This patch fixed it,
> - rename acpi_ev_global_lock_pending/acpi_ev_global_lock_pending_lock
>   to acpi_gbl_global_lock_pending/acpi_gbl_global_lock_pending_lock.
> 
> - move the initialization of acpi_gbl_global_lock_pending_lock from
>   acpi_ut_mutex_initialize to acpi_ev_init_global_lock_handler.
> 
> Signed-off-by: Lin Ming <ming.m.lin@intel.com>

Reviewed-by: Rafael J. Wysocki <rjw@sisk.pl>

> ---
>  drivers/acpi/acpica/acglobal.h |    6 ++-
>  drivers/acpi/acpica/evmisc.c   |   74 +++++++++++++++++++++------------------
>  drivers/acpi/acpica/utmutex.c  |    5 ---
>  3 files changed, 44 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
> index 6d512fc..73863d8 100644
> --- a/drivers/acpi/acpica/acglobal.h
> +++ b/drivers/acpi/acpica/acglobal.h
> @@ -214,13 +214,16 @@ ACPI_EXTERN struct acpi_mutex_info acpi_gbl_mutex_info[ACPI_NUM_MUTEX];
>  
>  /*
>   * Global lock mutex is an actual AML mutex object
> - * Global lock semaphore works in conjunction with the HW global lock
> + * Global lock semaphore works in conjunction with the actual global lock
> + * Global lock spinlock is used for "pending" handshake
>   */
>  ACPI_EXTERN union acpi_operand_object *acpi_gbl_global_lock_mutex;
>  ACPI_EXTERN acpi_semaphore acpi_gbl_global_lock_semaphore;
> +ACPI_EXTERN acpi_spinlock acpi_gbl_global_lock_pending_lock;
>  ACPI_EXTERN u16 acpi_gbl_global_lock_handle;
>  ACPI_EXTERN u8 acpi_gbl_global_lock_acquired;
>  ACPI_EXTERN u8 acpi_gbl_global_lock_present;
> +ACPI_EXTERN u8 acpi_gbl_global_lock_pending;
>  
>  /*
>   * Spinlocks are used for interfaces that can be possibly called at
> @@ -228,7 +231,6 @@ ACPI_EXTERN u8 acpi_gbl_global_lock_present;
>   */
>  ACPI_EXTERN acpi_spinlock acpi_gbl_gpe_lock;	/* For GPE data structs and registers */
>  ACPI_EXTERN acpi_spinlock acpi_gbl_hardware_lock;	/* For ACPI H/W except GPE registers */
> -ACPI_EXTERN acpi_spinlock acpi_ev_global_lock_pending_lock; /* For global lock */
>  
>  /*****************************************************************************
>   *
> diff --git a/drivers/acpi/acpica/evmisc.c b/drivers/acpi/acpica/evmisc.c
> index 7dc8094..69a3b4a 100644
> --- a/drivers/acpi/acpica/evmisc.c
> +++ b/drivers/acpi/acpica/evmisc.c
> @@ -284,39 +284,41 @@ static void ACPI_SYSTEM_XFACE acpi_ev_notify_dispatch(void *context)
>   * RETURN:      ACPI_INTERRUPT_HANDLED
>   *
>   * DESCRIPTION: Invoked directly from the SCI handler when a global lock
> - *              release interrupt occurs.  If there's a thread waiting for
> - *              the global lock, signal it.
> - *
> - * NOTE: Assumes that the semaphore can be signaled from interrupt level. If
> - * this is not possible for some reason, a separate thread will have to be
> - * scheduled to do this.
> + *              release interrupt occurs. If there is actually a pending
> + *              request for the lock, signal the waiting thread.
>   *
>   ******************************************************************************/
> -static u8 acpi_ev_global_lock_pending;
>  
>  static u32 acpi_ev_global_lock_handler(void *context)
>  {
>  	acpi_status status;
>  	acpi_cpu_flags flags;
>  
> -	flags = acpi_os_acquire_lock(acpi_ev_global_lock_pending_lock);
> +	flags = acpi_os_acquire_lock(acpi_gbl_global_lock_pending_lock);
>  
> -	if (!acpi_ev_global_lock_pending) {
> -		goto out;
> +	/*
> +	 * If a request for the global lock is not actually pending,
> +	 * we are done. This handles "spurious" global lock interrupts
> +	 * which are possible (and have been seen) with bad BIOSs.
> +	 */
> +	if (!acpi_gbl_global_lock_pending) {
> +		goto cleanup_and_exit;
>  	}
>  
> -	/* Send a unit to the semaphore */
> -
> +	/*
> +	 * Send a unit to the global lock semaphore. The actual acquisition
> +	 * of the global lock will be performed by the waiting thread.
> +	 */
>  	status = acpi_os_signal_semaphore(acpi_gbl_global_lock_semaphore, 1);
>  	if (ACPI_FAILURE(status)) {
>  		ACPI_ERROR((AE_INFO, "Could not signal Global Lock semaphore"));
>  	}
>  
> -	acpi_ev_global_lock_pending = FALSE;
> +	acpi_gbl_global_lock_pending = FALSE;
>  
> - out:
> -	acpi_os_release_lock(acpi_ev_global_lock_pending_lock, flags);
> +cleanup_and_exit:
>  
> +	acpi_os_release_lock(acpi_gbl_global_lock_pending_lock, flags);
>  	return (ACPI_INTERRUPT_HANDLED);
>  }
>  
> @@ -350,14 +352,20 @@ acpi_status acpi_ev_init_global_lock_handler(void)
>  	 * Map to AE_OK, but mark global lock as not present. Any attempt to
>  	 * actually use the global lock will be flagged with an error.
>  	 */
> +	acpi_gbl_global_lock_present = FALSE;
>  	if (status == AE_NO_HARDWARE_RESPONSE) {
>  		ACPI_ERROR((AE_INFO,
>  			    "No response from Global Lock hardware, disabling lock"));
>  
> -		acpi_gbl_global_lock_present = FALSE;
>  		return_ACPI_STATUS(AE_OK);
>  	}
>  
> +	status = acpi_os_create_lock(&acpi_gbl_global_lock_pending_lock);
> +	if (ACPI_FAILURE(status)) {
> +		return_ACPI_STATUS(status);
> +	}
> +
> +	acpi_gbl_global_lock_pending = FALSE;
>  	acpi_gbl_global_lock_present = TRUE;
>  	return_ACPI_STATUS(status);
>  }
> @@ -414,7 +422,7 @@ static int acpi_ev_global_lock_acquired;
>  acpi_status acpi_ev_acquire_global_lock(u16 timeout)
>  {
>  	acpi_cpu_flags flags;
> -	acpi_status status = AE_OK;
> +	acpi_status status;
>  	u8 acquired = FALSE;
>  
>  	ACPI_FUNCTION_TRACE(ev_acquire_global_lock);
> @@ -458,15 +466,15 @@ acpi_status acpi_ev_acquire_global_lock(u16 timeout)
>  	}
>  
>  	/*
> -	 * Make sure that a global lock actually exists. If not, just treat the
> -	 * lock as a standard mutex.
> +	 * Make sure that a global lock actually exists. If not, just
> +	 * treat the lock as a standard mutex.
>  	 */
>  	if (!acpi_gbl_global_lock_present) {
>  		acpi_gbl_global_lock_acquired = TRUE;
>  		return_ACPI_STATUS(AE_OK);
>  	}
>  
> -	flags = acpi_os_acquire_lock(acpi_ev_global_lock_pending_lock);
> +	flags = acpi_os_acquire_lock(acpi_gbl_global_lock_pending_lock);
>  
>  	do {
>  
> @@ -475,20 +483,19 @@ acpi_status acpi_ev_acquire_global_lock(u16 timeout)
>  		ACPI_ACQUIRE_GLOBAL_LOCK(acpi_gbl_FACS, acquired);
>  		if (acquired) {
>  			acpi_gbl_global_lock_acquired = TRUE;
> -
>  			ACPI_DEBUG_PRINT((ACPI_DB_EXEC,
>  					  "Acquired hardware Global Lock\n"));
>  			break;
>  		}
>  
> -		acpi_ev_global_lock_pending = TRUE;
> -
> -		acpi_os_release_lock(acpi_ev_global_lock_pending_lock, flags);
> -
>  		/*
> -		 * Did not get the lock. The pending bit was set above, and we
> -		 * must wait until we get the global lock released interrupt.
> +		 * Did not get the lock. The pending bit was set above, and
> +		 * we must now wait until we receive the global lock
> +		 * released interrupt.
>  		 */
> +		acpi_gbl_global_lock_pending = TRUE;
> +		acpi_os_release_lock(acpi_gbl_global_lock_pending_lock, flags);
> +
>  		ACPI_DEBUG_PRINT((ACPI_DB_EXEC,
>  				  "Waiting for hardware Global Lock\n"));
>  
> @@ -496,17 +503,16 @@ acpi_status acpi_ev_acquire_global_lock(u16 timeout)
>  		 * Wait for handshake with the global lock interrupt handler.
>  		 * This interface releases the interpreter if we must wait.
>  		 */
> -		status = acpi_ex_system_wait_semaphore(
> -						acpi_gbl_global_lock_semaphore,
> -						ACPI_WAIT_FOREVER);
> +		status =
> +		    acpi_ex_system_wait_semaphore
> +		    (acpi_gbl_global_lock_semaphore, ACPI_WAIT_FOREVER);
>  
> -		flags = acpi_os_acquire_lock(acpi_ev_global_lock_pending_lock);
> +		flags = acpi_os_acquire_lock(acpi_gbl_global_lock_pending_lock);
>  
>  	} while (ACPI_SUCCESS(status));
>  
> -	acpi_ev_global_lock_pending = FALSE;
> -
> -	acpi_os_release_lock(acpi_ev_global_lock_pending_lock, flags);
> +	acpi_gbl_global_lock_pending = FALSE;
> +	acpi_os_release_lock(acpi_gbl_global_lock_pending_lock, flags);
>  
>  	return_ACPI_STATUS(status);
>  }
> diff --git a/drivers/acpi/acpica/utmutex.c b/drivers/acpi/acpica/utmutex.c
> index 519d4ee..7d797e2 100644
> --- a/drivers/acpi/acpica/utmutex.c
> +++ b/drivers/acpi/acpica/utmutex.c
> @@ -93,11 +93,6 @@ acpi_status acpi_ut_mutex_initialize(void)
>  		return_ACPI_STATUS (status);
>  	}
>  
> -	status = acpi_os_create_lock (&acpi_ev_global_lock_pending_lock);
> -	if (ACPI_FAILURE (status)) {
> -		return_ACPI_STATUS (status);
> -	}
> -
>  	/* Mutex for _OSI support */
>  	status = acpi_os_create_mutex(&acpi_gbl_osi_mutex);
>  	if (ACPI_FAILURE(status)) {
> 

Re: [PATCH 3/3] ACPICA: Fix code divergence of global lock handling

[Lin Ming]

2011/3/24 Rafael J. Wysocki <rjw@sisk.pl>:
> On Wednesday, March 23, 2011, Lin Ming wrote:
>> Commit 9cd0314(ACPI / ACPICA: Fix global lock acquisition) was backported
>> into ACPICA code base, and some divergence was introduced.
>>
>> This patch fixed it,
>> - rename acpi_ev_global_lock_pending/acpi_ev_global_lock_pending_lock
>>   to acpi_gbl_global_lock_pending/acpi_gbl_global_lock_pending_lock.
>>
>> - move the initialization of acpi_gbl_global_lock_pending_lock from
>>   acpi_ut_mutex_initialize to acpi_ev_init_global_lock_handler.
>>
>> Signed-off-by: Lin Ming <ming.m.lin@intel.com>
>
> Reviewed-by: Rafael J. Wysocki <rjw@sisk.pl>

Len,

Could you consider applying these patches for 2.6.39-rc1?

Thanks,
Lin Ming

>
>> ---
>>  drivers/acpi/acpica/acglobal.h |    6 ++-
>>  drivers/acpi/acpica/evmisc.c   |   74 +++++++++++++++++++++------------------
>>  drivers/acpi/acpica/utmutex.c  |    5 ---
>>  3 files changed, 44 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/acpi/acpica/acglobal.h b/drivers/acpi/acpica/acglobal.h
>> index 6d512fc..73863d8 100644
>> --- a/drivers/acpi/acpica/acglobal.h
>> +++ b/drivers/acpi/acpica/acglobal.h
>> @@ -214,13 +214,16 @@ ACPI_EXTERN struct acpi_mutex_info acpi_gbl_mutex_info[ACPI_NUM_MUTEX];
>>
>>  /*
>>   * Global lock mutex is an actual AML mutex object
>> - * Global lock semaphore works in conjunction with the HW global lock
>> + * Global lock semaphore works in conjunction with the actual global lock
>> + * Global lock spinlock is used for "pending" handshake
>>   */
>>  ACPI_EXTERN union acpi_operand_object *acpi_gbl_global_lock_mutex;
>>  ACPI_EXTERN acpi_semaphore acpi_gbl_global_lock_semaphore;
>> +ACPI_EXTERN acpi_spinlock acpi_gbl_global_lock_pending_lock;
>>  ACPI_EXTERN u16 acpi_gbl_global_lock_handle;
>>  ACPI_EXTERN u8 acpi_gbl_global_lock_acquired;
>>  ACPI_EXTERN u8 acpi_gbl_global_lock_present;
>> +ACPI_EXTERN u8 acpi_gbl_global_lock_pending;
>>
>>  /*
>>   * Spinlocks are used for interfaces that can be possibly called at
>> @@ -228,7 +231,6 @@ ACPI_EXTERN u8 acpi_gbl_global_lock_present;
>>   */
>>  ACPI_EXTERN acpi_spinlock acpi_gbl_gpe_lock; /* For GPE data structs and registers */
>>  ACPI_EXTERN acpi_spinlock acpi_gbl_hardware_lock;    /* For ACPI H/W except GPE registers */
>> -ACPI_EXTERN acpi_spinlock acpi_ev_global_lock_pending_lock; /* For global lock */
>>
>>  /*****************************************************************************
>>   *
>> diff --git a/drivers/acpi/acpica/evmisc.c b/drivers/acpi/acpica/evmisc.c
>> index 7dc8094..69a3b4a 100644
>> --- a/drivers/acpi/acpica/evmisc.c
>> +++ b/drivers/acpi/acpica/evmisc.c
>> @@ -284,39 +284,41 @@ static void ACPI_SYSTEM_XFACE acpi_ev_notify_dispatch(void *context)
>>   * RETURN:      ACPI_INTERRUPT_HANDLED
>>   *
>>   * DESCRIPTION: Invoked directly from the SCI handler when a global lock
>> - *              release interrupt occurs.  If there's a thread waiting for
>> - *              the global lock, signal it.
>> - *
>> - * NOTE: Assumes that the semaphore can be signaled from interrupt level. If
>> - * this is not possible for some reason, a separate thread will have to be
>> - * scheduled to do this.
>> + *              release interrupt occurs. If there is actually a pending
>> + *              request for the lock, signal the waiting thread.
>>   *
>>   ******************************************************************************/
>> -static u8 acpi_ev_global_lock_pending;
>>
>>  static u32 acpi_ev_global_lock_handler(void *context)
>>  {
>>       acpi_status status;
>>       acpi_cpu_flags flags;
>>
>> -     flags = acpi_os_acquire_lock(acpi_ev_global_lock_pending_lock);
>> +     flags = acpi_os_acquire_lock(acpi_gbl_global_lock_pending_lock);
>>
>> -     if (!acpi_ev_global_lock_pending) {
>> -             goto out;
>> +     /*
>> +      * If a request for the global lock is not actually pending,
>> +      * we are done. This handles "spurious" global lock interrupts
>> +      * which are possible (and have been seen) with bad BIOSs.
>> +      */
>> +     if (!acpi_gbl_global_lock_pending) {
>> +             goto cleanup_and_exit;
>>       }
>>
>> -     /* Send a unit to the semaphore */
>> -
>> +     /*
>> +      * Send a unit to the global lock semaphore. The actual acquisition
>> +      * of the global lock will be performed by the waiting thread.
>> +      */
>>       status = acpi_os_signal_semaphore(acpi_gbl_global_lock_semaphore, 1);
>>       if (ACPI_FAILURE(status)) {
>>               ACPI_ERROR((AE_INFO, "Could not signal Global Lock semaphore"));
>>       }
>>
>> -     acpi_ev_global_lock_pending = FALSE;
>> +     acpi_gbl_global_lock_pending = FALSE;
>>
>> - out:
>> -     acpi_os_release_lock(acpi_ev_global_lock_pending_lock, flags);
>> +cleanup_and_exit:
>>
>> +     acpi_os_release_lock(acpi_gbl_global_lock_pending_lock, flags);
>>       return (ACPI_INTERRUPT_HANDLED);
>>  }
>>
>> @@ -350,14 +352,20 @@ acpi_status acpi_ev_init_global_lock_handler(void)
>>        * Map to AE_OK, but mark global lock as not present. Any attempt to
>>        * actually use the global lock will be flagged with an error.
>>        */
>> +     acpi_gbl_global_lock_present = FALSE;
>>       if (status == AE_NO_HARDWARE_RESPONSE) {
>>               ACPI_ERROR((AE_INFO,
>>                           "No response from Global Lock hardware, disabling lock"));
>>
>> -             acpi_gbl_global_lock_present = FALSE;
>>               return_ACPI_STATUS(AE_OK);
>>       }
>>
>> +     status = acpi_os_create_lock(&acpi_gbl_global_lock_pending_lock);
>> +     if (ACPI_FAILURE(status)) {
>> +             return_ACPI_STATUS(status);
>> +     }
>> +
>> +     acpi_gbl_global_lock_pending = FALSE;
>>       acpi_gbl_global_lock_present = TRUE;
>>       return_ACPI_STATUS(status);
>>  }
>> @@ -414,7 +422,7 @@ static int acpi_ev_global_lock_acquired;
>>  acpi_status acpi_ev_acquire_global_lock(u16 timeout)
>>  {
>>       acpi_cpu_flags flags;
>> -     acpi_status status = AE_OK;
>> +     acpi_status status;
>>       u8 acquired = FALSE;
>>
>>       ACPI_FUNCTION_TRACE(ev_acquire_global_lock);
>> @@ -458,15 +466,15 @@ acpi_status acpi_ev_acquire_global_lock(u16 timeout)
>>       }
>>
>>       /*
>> -      * Make sure that a global lock actually exists. If not, just treat the
>> -      * lock as a standard mutex.
>> +      * Make sure that a global lock actually exists. If not, just
>> +      * treat the lock as a standard mutex.
>>        */
>>       if (!acpi_gbl_global_lock_present) {
>>               acpi_gbl_global_lock_acquired = TRUE;
>>               return_ACPI_STATUS(AE_OK);
>>       }
>>
>> -     flags = acpi_os_acquire_lock(acpi_ev_global_lock_pending_lock);
>> +     flags = acpi_os_acquire_lock(acpi_gbl_global_lock_pending_lock);
>>
>>       do {
>>
>> @@ -475,20 +483,19 @@ acpi_status acpi_ev_acquire_global_lock(u16 timeout)
>>               ACPI_ACQUIRE_GLOBAL_LOCK(acpi_gbl_FACS, acquired);
>>               if (acquired) {
>>                       acpi_gbl_global_lock_acquired = TRUE;
>> -
>>                       ACPI_DEBUG_PRINT((ACPI_DB_EXEC,
>>                                         "Acquired hardware Global Lock\n"));
>>                       break;
>>               }
>>
>> -             acpi_ev_global_lock_pending = TRUE;
>> -
>> -             acpi_os_release_lock(acpi_ev_global_lock_pending_lock, flags);
>> -
>>               /*
>> -              * Did not get the lock. The pending bit was set above, and we
>> -              * must wait until we get the global lock released interrupt.
>> +              * Did not get the lock. The pending bit was set above, and
>> +              * we must now wait until we receive the global lock
>> +              * released interrupt.
>>                */
>> +             acpi_gbl_global_lock_pending = TRUE;
>> +             acpi_os_release_lock(acpi_gbl_global_lock_pending_lock, flags);
>> +
>>               ACPI_DEBUG_PRINT((ACPI_DB_EXEC,
>>                                 "Waiting for hardware Global Lock\n"));
>>
>> @@ -496,17 +503,16 @@ acpi_status acpi_ev_acquire_global_lock(u16 timeout)
>>                * Wait for handshake with the global lock interrupt handler.
>>                * This interface releases the interpreter if we must wait.
>>                */
>> -             status = acpi_ex_system_wait_semaphore(
>> -                                             acpi_gbl_global_lock_semaphore,
>> -                                             ACPI_WAIT_FOREVER);
>> +             status =
>> +                 acpi_ex_system_wait_semaphore
>> +                 (acpi_gbl_global_lock_semaphore, ACPI_WAIT_FOREVER);
>>
>> -             flags = acpi_os_acquire_lock(acpi_ev_global_lock_pending_lock);
>> +             flags = acpi_os_acquire_lock(acpi_gbl_global_lock_pending_lock);
>>
>>       } while (ACPI_SUCCESS(status));
>>
>> -     acpi_ev_global_lock_pending = FALSE;
>> -
>> -     acpi_os_release_lock(acpi_ev_global_lock_pending_lock, flags);
>> +     acpi_gbl_global_lock_pending = FALSE;
>> +     acpi_os_release_lock(acpi_gbl_global_lock_pending_lock, flags);
>>
>>       return_ACPI_STATUS(status);
>>  }
>> diff --git a/drivers/acpi/acpica/utmutex.c b/drivers/acpi/acpica/utmutex.c
>> index 519d4ee..7d797e2 100644
>> --- a/drivers/acpi/acpica/utmutex.c
>> +++ b/drivers/acpi/acpica/utmutex.c
>> @@ -93,11 +93,6 @@ acpi_status acpi_ut_mutex_initialize(void)
>>               return_ACPI_STATUS (status);
>>       }
>>
>> -     status = acpi_os_create_lock (&acpi_ev_global_lock_pending_lock);
>> -     if (ACPI_FAILURE (status)) {
>> -             return_ACPI_STATUS (status);
>> -     }
>> -
>>       /* Mutex for _OSI support */
>>       status = acpi_os_create_mutex(&acpi_gbl_osi_mutex);
>>       if (ACPI_FAILURE(status)) {
>>
>
> --
> 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
>
--
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

Re: [PATCH 0/3] ACPI/ACPICA: Resolve spinlock and global lock handling code divergence

[Len Brown]

series applied to acpica branch and acpi-test

thanks,
Len Brown, Intel Open Source Technology Center