| Message ID | 1357321594-2062-1-git-send-email-colin.king@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
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>
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>
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;