Patchwork acpica: fwts_acpica: force indefinite AML Wait() to timeout

login
register
mail settings
Submitter Colin King
Date Jan. 4, 2013, 5:46 p.m.
Message ID <1357321594-2062-1-git-send-email-colin.king@canonical.com>
Download mbox | patch
Permalink /patch/209504/
State Accepted
Headers show

Comments

Colin King - Jan. 4, 2013, 5:46 p.m.
From: Colin Ian King <colin.king@canonical.com>

Quite a few _EJ0 implementations make the ACPI method test block
forever because they are using an AML Wait() that is waiting
indefinitely for an external event (which fwts cannot generate).
Instead of the test waiting forever for the event, make these
waits timeout after 20 seconds.  We force this by causing an alarm
signal which interrupts the blocked sem_wait and we can detect this
and report back to the user that we aborted the wait.  This is ugly
but effective.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/acpica/fwts_acpica.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)
Keng-Yu Lin - Jan. 14, 2013, 8:03 a.m.
On Sat, Jan 5, 2013 at 1:46 AM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Quite a few _EJ0 implementations make the ACPI method test block
> forever because they are using an AML Wait() that is waiting
> indefinitely for an external event (which fwts cannot generate).
> Instead of the test waiting forever for the event, make these
> waits timeout after 20 seconds.  We force this by causing an alarm
> signal which interrupts the blocked sem_wait and we can detect this
> and report back to the user that we aborted the wait.  This is ugly
> but effective.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/acpica/fwts_acpica.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/src/acpica/fwts_acpica.c b/src/acpica/fwts_acpica.c
> index 26b6a13..65caa18 100644
> --- a/src/acpica/fwts_acpica.c
> +++ b/src/acpica/fwts_acpica.c
> @@ -55,6 +55,8 @@ static pthread_mutex_t mutex_thread_info;
>  #define MAX_SEMAPHORES         (1024)
>  #define MAX_THREADS            (128)
>
> +#define MAX_WAIT_TIMEOUT       (20)    /* Seconds */
> +
>  typedef struct {
>         sem_t           sem;    /* Semaphore handle */
>         int             count;  /* count > 0 if acquired */
> @@ -546,6 +548,10 @@ ACPI_STATUS AcpiOsDeleteSemaphore(ACPI_HANDLE Handle)
>         return ret;
>  }
>
> +static void sem_alarm(int dummy)
> +{
> +       /* Do nothing apart from interrupt sem_wait() */
> +}
>
>  /*
>   *  AcpiOsWaitSemaphore()
> @@ -556,6 +562,7 @@ ACPI_STATUS AcpiOsWaitSemaphore(ACPI_HANDLE handle, UINT32 Units, UINT16 Timeout
>  {
>         sem_info *sem = (sem_info *)handle;
>         struct timespec tm;
> +       struct sigaction sa;
>
>         if (!handle)
>                 return AE_BAD_PARAMETER;
> @@ -567,8 +574,34 @@ ACPI_STATUS AcpiOsWaitSemaphore(ACPI_HANDLE handle, UINT32 Units, UINT16 Timeout
>                 break;
>
>         case ACPI_WAIT_FOREVER:
> -               if (sem_wait(&sem->sem))
> +               /*
> +                *  The semantics are "wait forever", but
> +                *  really we actually detect a lock up
> +                *  after a long wait and break out rather
> +                *  than waiting for the sun to turn into
> +                *  a lump of coal.  This allows us to
> +                *  handle controls such as _EJ0 that may
> +                *  be waiting forever for a event to signal
> +                *  control to continue.  It is evil.
> +                */
> +               sa.sa_handler = sem_alarm;
> +               sa.sa_flags = 0;
> +               sigaction(SIGALRM, &sa, NULL);
> +               alarm(MAX_WAIT_TIMEOUT);
> +
> +               if (sem_wait(&sem->sem)) {
> +                       alarm(0);
> +                       if (errno == EINTR)
> +                               fwts_log_info(fwts_acpica_fw,
> +                                       "AML was blocked waiting for "
> +                                       "an external event, fwts detected "
> +                                       "this and forced a timeout after "
> +                                       "%d seconds on a Wait() that had "
> +                                       "an indefinite timeout.",
> +                                       MAX_WAIT_TIMEOUT);
>                         return AE_TIME;
> +               }
> +               alarm(0);
>                 break;
>         default:
>                 tm.tv_sec = Timeout / 1000;
> --
> 1.8.0
>
Acked-by: Keng-Yu Lin <kengyu@canonical.com>
Ivan Hu - Jan. 14, 2013, 10:08 a.m.
On 01/05/2013 01:46 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Quite a few _EJ0 implementations make the ACPI method test block
> forever because they are using an AML Wait() that is waiting
> indefinitely for an external event (which fwts cannot generate).
> Instead of the test waiting forever for the event, make these
> waits timeout after 20 seconds.  We force this by causing an alarm
> signal which interrupts the blocked sem_wait and we can detect this
> and report back to the user that we aborted the wait.  This is ugly
> but effective.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/acpica/fwts_acpica.c | 35 ++++++++++++++++++++++++++++++++++-
>   1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/src/acpica/fwts_acpica.c b/src/acpica/fwts_acpica.c
> index 26b6a13..65caa18 100644
> --- a/src/acpica/fwts_acpica.c
> +++ b/src/acpica/fwts_acpica.c
> @@ -55,6 +55,8 @@ static pthread_mutex_t mutex_thread_info;
>   #define MAX_SEMAPHORES		(1024)
>   #define MAX_THREADS		(128)
>
> +#define MAX_WAIT_TIMEOUT	(20)	/* Seconds */
> +
>   typedef struct {
>   	sem_t		sem;	/* Semaphore handle */
>   	int		count;	/* count > 0 if acquired */
> @@ -546,6 +548,10 @@ ACPI_STATUS AcpiOsDeleteSemaphore(ACPI_HANDLE Handle)
>   	return ret;
>   }
>
> +static void sem_alarm(int dummy)
> +{
> +	/* Do nothing apart from interrupt sem_wait() */
> +}
>
>   /*
>    *  AcpiOsWaitSemaphore()
> @@ -556,6 +562,7 @@ ACPI_STATUS AcpiOsWaitSemaphore(ACPI_HANDLE handle, UINT32 Units, UINT16 Timeout
>   {
>   	sem_info *sem = (sem_info *)handle;
>   	struct timespec	tm;
> +	struct sigaction sa;
>
>   	if (!handle)
>   		return AE_BAD_PARAMETER;
> @@ -567,8 +574,34 @@ ACPI_STATUS AcpiOsWaitSemaphore(ACPI_HANDLE handle, UINT32 Units, UINT16 Timeout
>   		break;
>
>   	case ACPI_WAIT_FOREVER:
> -		if (sem_wait(&sem->sem))
> +		/*
> +		 *  The semantics are "wait forever", but
> +		 *  really we actually detect a lock up
> +		 *  after a long wait and break out rather
> +		 *  than waiting for the sun to turn into
> +		 *  a lump of coal.  This allows us to
> +		 *  handle controls such as _EJ0 that may
> +		 *  be waiting forever for a event to signal
> +		 *  control to continue.  It is evil.
> +		 */
> +		sa.sa_handler = sem_alarm;
> +		sa.sa_flags = 0;
> +		sigaction(SIGALRM, &sa, NULL);
> +		alarm(MAX_WAIT_TIMEOUT);
> +
> +		if (sem_wait(&sem->sem)) {
> +			alarm(0);
> +			if (errno == EINTR)
> +				fwts_log_info(fwts_acpica_fw,
> +					"AML was blocked waiting for "
> +					"an external event, fwts detected "
> +					"this and forced a timeout after "
> +					"%d seconds on a Wait() that had "
> +					"an indefinite timeout.",
> +					MAX_WAIT_TIMEOUT);
>   			return AE_TIME;
> +		}
> +		alarm(0);
>   		break;
>   	default:
>   		tm.tv_sec = Timeout / 1000;
>

Acked-by: Ivan Hu <ivan.hu@canonical.com>

Patch

diff --git a/src/acpica/fwts_acpica.c b/src/acpica/fwts_acpica.c
index 26b6a13..65caa18 100644
--- a/src/acpica/fwts_acpica.c
+++ b/src/acpica/fwts_acpica.c
@@ -55,6 +55,8 @@  static pthread_mutex_t mutex_thread_info;
 #define MAX_SEMAPHORES		(1024)
 #define MAX_THREADS		(128)
 
+#define MAX_WAIT_TIMEOUT	(20)	/* Seconds */
+
 typedef struct {
 	sem_t		sem;	/* Semaphore handle */
 	int		count;	/* count > 0 if acquired */
@@ -546,6 +548,10 @@  ACPI_STATUS AcpiOsDeleteSemaphore(ACPI_HANDLE Handle)
 	return ret;
 }
 
+static void sem_alarm(int dummy)
+{
+	/* Do nothing apart from interrupt sem_wait() */
+}
 
 /*
  *  AcpiOsWaitSemaphore()
@@ -556,6 +562,7 @@  ACPI_STATUS AcpiOsWaitSemaphore(ACPI_HANDLE handle, UINT32 Units, UINT16 Timeout
 {
 	sem_info *sem = (sem_info *)handle;
 	struct timespec	tm;
+	struct sigaction sa;
 
 	if (!handle)
 		return AE_BAD_PARAMETER;
@@ -567,8 +574,34 @@  ACPI_STATUS AcpiOsWaitSemaphore(ACPI_HANDLE handle, UINT32 Units, UINT16 Timeout
 		break;
 
 	case ACPI_WAIT_FOREVER:
-		if (sem_wait(&sem->sem))
+		/*
+		 *  The semantics are "wait forever", but
+		 *  really we actually detect a lock up
+		 *  after a long wait and break out rather
+		 *  than waiting for the sun to turn into
+		 *  a lump of coal.  This allows us to
+		 *  handle controls such as _EJ0 that may
+		 *  be waiting forever for a event to signal
+		 *  control to continue.  It is evil.
+		 */
+		sa.sa_handler = sem_alarm;
+		sa.sa_flags = 0;
+		sigaction(SIGALRM, &sa, NULL);
+		alarm(MAX_WAIT_TIMEOUT);
+
+		if (sem_wait(&sem->sem)) {
+			alarm(0);
+			if (errno == EINTR)
+				fwts_log_info(fwts_acpica_fw,
+					"AML was blocked waiting for "
+					"an external event, fwts detected "
+					"this and forced a timeout after "
+					"%d seconds on a Wait() that had "
+					"an indefinite timeout.",
+					MAX_WAIT_TIMEOUT);
 			return AE_TIME;
+		}
+		alarm(0);
 		break;
 	default:
 		tm.tv_sec = Timeout / 1000;