[PATCH] lib: acpica: fix semaphore counting by waiting for threads to complete
IvanHu
ivan.hu at canonical.com
Mon Jul 23 03:55:11 UTC 2012
On 07/21/2012 12:02 AM, Colin Ian King wrote:
> On 20/07/12 02:55, IvanHu wrote:
>> On 07/19/2012 04:44 PM, Colin King wrote:
>>> From: Colin Ian King <colin.king at canonical.com>
>>>
>>> When executing Notify() handlers ACPICA creates separate threads which
>>> we need to wait to complete before we do any semaphore usage accounting.
>>> This is because these threads can themselves use internal or AML
>>> semaphores
>>> which will be in an unknown state if we do the start doing semaphore
>>> usage accounted before we wait for them to terminate. This patch adds
>>> in an array to keep track of threads created by AcpiOsExecute and also
>>> a pthread join on these running threads before we start counting any
>>> used semaphores.
>>>
>>> Note that we have to deprecate the ACPICA verison of AcpiOsExecute and
>>> re-implement this in fwts to allow us to do the thread creation
>>> tracking.
>>>
>>> Signed-off-by: Colin Ian King <colin.king at canonical.com>
>>> ---
>>> src/acpica/Makefile.am | 3 +-
>>> src/acpica/fwts_acpica.c | 106
>>> ++++++++++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 108 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/acpica/Makefile.am b/src/acpica/Makefile.am
>>> index e2213d5..6b3bfa5 100644
>>> --- a/src/acpica/Makefile.am
>>> +++ b/src/acpica/Makefile.am
>>> @@ -24,7 +24,8 @@ osunixxf_munged.c: $(ACPICA_OSL)/osunixxf.c
>>> sed 's/^AcpiOsDeleteSemaphore/__AcpiOsDeleteSemaphore/' | \
>>> sed 's/^AcpiOsVprintf/__AcpiOsVprintf/' | \
>>> sed 's/^AcpiOsSignal/__AcpiOsSignal/' | \
>>> - sed 's/^AcpiOsSleep/__AcpiOsSleep/' \
>>> + sed 's/^AcpiOsSleep/__AcpiOsSleep/' | \
>>> + sed 's/^AcpiOsExecute/__AcpiOsExecute/' \
>>> > osunixxf_munged.c
>>>
>>> #
>>> diff --git a/src/acpica/fwts_acpica.c b/src/acpica/fwts_acpica.c
>>> index cf9a4fe..08b012d 100644
>>> --- a/src/acpica/fwts_acpica.c
>>> +++ b/src/acpica/fwts_acpica.c
>>> @@ -32,6 +32,7 @@
>>> #include <pthread.h>
>>>
>>> static pthread_mutex_t mutex_lock_count;
>>> +static pthread_mutex_t mutex_thread_info;
>>>
>>> #include "fwts.h"
>>>
>>> @@ -54,11 +55,15 @@ static pthread_mutex_t mutex_lock_count;
>>> #define MAX_SEMAPHORES (1009)
>>> #define HASH_FULL (0xffffffff)
>>>
>>> +#define MAX_THREADS (128)
>>> +
>>> typedef struct {
>>> sem_t *sem; /* Semaphore handle */
>>> int count; /* count > 0 if acquired */
>>> } sem_hash;
>>>
>>> +typedef void * (*PTHREAD_CALLBACK)(void *);
>>> +
>>> BOOLEAN AcpiGbl_IgnoreErrors = FALSE;
>>> UINT8 AcpiGbl_RegionFillValue = 0;
>>>
>>> @@ -70,6 +75,16 @@ static sem_hash
>>> sem_hash_table[MAX_SEMAPHORES];
>>> static ACPI_TABLE_DESC Tables[ACPI_MAX_INIT_TABLES];
>>>
>>> /*
>>> + * Used to account for threads used by AcpiOsExecute
>>> + */
>>> +typedef struct {
>>> + bool used; /* true if the thread accounting info in use
>>> by a thread */
>>> + pthread_t thread; /* thread info */
>>> +} fwts_thread;
>>> +
>>> +static fwts_thread threads[MAX_THREADS];
>>> +
>>> +/*
>>> * Static copies of ACPI tables used by ACPICA execution engine
>>> */
>>> static ACPI_TABLE_XSDT *fwts_acpica_XSDT;
>>> @@ -133,6 +148,27 @@ void fwts_acpica_sem_count_get(int *acquired, int
>>> *released)
>>> *acquired = 0;
>>> *released = 0;
>>>
>>> + /* Wait for any pending threads to complete */
>>> +
>>> + for (i = 0; i < MAX_THREADS; i++) {
>>> + pthread_mutex_lock(&mutex_thread_info);
>>> + if (threads[i].used) {
>>> + pthread_mutex_unlock(&mutex_thread_info);
>>> +
>>> + /* Wait for thread to complete */
>>> + pthread_join(threads[i].thread, NULL);
>>> +
>>> + pthread_mutex_lock(&mutex_thread_info);
>>> + threads[i].used = false;
>>> + pthread_mutex_unlock(&mutex_thread_info);
>>> + }
>>> + pthread_mutex_unlock(&mutex_thread_info);
>>> + }
>>> +
>>> + /*
>>> + * All threads (such as Notify() calls now complete, so
>>> + * we can now do the semaphore accounting calculations
>>> + */
>>> for (i=0;i<MAX_SEMAPHORES;i++) {
>>> if (sem_hash_table[i].sem != NULL) {
>>> (*acquired)++;
>>> @@ -624,6 +660,74 @@ ACPI_STATUS AcpiOsReadPort(ACPI_IO_ADDRESS addr,
>>> UINT32 *value, UINT32 width)
>>> return AE_OK;
>>> }
>>>
>>> +typedef struct {
>>> + PTHREAD_CALLBACK func;
>>> + void * context;
>>> + int thread_index;
>>> +} fwts_func_wrapper_context;
>>> +
>>> +/*
>>> + * fwts_pthread_func_wrapper()
>>> + * wrap the AcpiOsExecute function so we can mark the thread
>>> + * accounting free once the function has completed.
>>> + */
>>> +void *fwts_pthread_func_wrapper(fwts_func_wrapper_context *ctx)
>>> +{
>>> + void *ret;
>>> +
>>> + ret = ctx->func(ctx->context);
>>> +
>>> + pthread_mutex_lock(&mutex_thread_info);
>>> + threads[ctx->thread_index].used = false;
>>> + pthread_mutex_unlock(&mutex_thread_info);
>>> +
>>> + free(ctx);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +ACPI_STATUS AcpiOsExecute(
>>> + ACPI_EXECUTE_TYPE type,
>>> + ACPI_OSD_EXEC_CALLBACK function,
>>> + void *func_context)
>>> +{
>>> + int ret;
>>> + int i;
>>> +
>>> + pthread_mutex_lock(&mutex_thread_info);
>>> +
>>> + /* Find a free slot to do per-thread join tracking */
>>> + for (i = 0; i < MAX_THREADS; i++) {
>>> + if (!threads[i].used) {
>>> + fwts_func_wrapper_context *ctx;
>>> +
>>> + /* We need some context to pass through to the thread
>>> wrapper */
>>> + if ((ctx = malloc(sizeof(fwts_func_wrapper_context))) ==
>>> NULL) {
>>> + pthread_mutex_unlock(&mutex_thread_info);
>>> + return AE_NO_MEMORY;
>>> + }
>>> +
>>> + ctx->func = (PTHREAD_CALLBACK)function;
>>> + ctx->context = func_context;
>>> + ctx->thread_index = i;
>>> + threads[i].used = true;
>>> +
>>> + ret = pthread_create(&threads[i].thread, NULL,
>>> + (PTHREAD_CALLBACK)fwts_pthread_func_wrapper, ctx);
>>> + pthread_mutex_unlock(&mutex_thread_info);
>>> +
>>> + if (ret)
>>> + return AE_ERROR;
>>> +
>>> + return AE_OK;
>>> + }
>>> + }
>>> +
>>> + /* No free slots, failed! */
>>> + pthread_mutex_unlock(&mutex_thread_info);
>>> + return AE_NO_MEMORY;
>>> +}
>>> +
>>> /*
>>> * AcpiOsReadPciConfiguration()
>>> * Override ACPICA AcpiOsReadPciConfiguration to fake PCI reads
>>> @@ -802,6 +906,7 @@ int fwts_acpica_init(fwts_framework *fw)
>>> return FWTS_ERROR;
>>>
>>> pthread_mutex_init(&mutex_lock_count, NULL);
>>> + pthread_mutex_init(&mutex_thread_info, NULL);
>>>
>>> fwts_acpica_fw = fw;
>>>
>>> @@ -990,6 +1095,7 @@ int fwts_acpica_deinit(void)
>>>
>>> AcpiTerminate();
>>> pthread_mutex_destroy(&mutex_lock_count);
>>> + pthread_mutex_destroy(&mutex_thread_info);
>>>
>>> FWTS_ACPICA_FREE(fwts_acpica_XSDT);
>>> FWTS_ACPICA_FREE(fwts_acpica_RSDT);
>>>
>>
>> it seems that multiple definition of `AcpiOsExecute' cause build error:
>>
>> .libs/osunixxf_munged.o: In function `AcpiOsExecute':
>> /home/ivanhu/fwts/src/acpica/osunixxf_munged.c:1306: multiple definition
>> of `AcpiOsExecute'
>> .libs/fwts_acpica.o:/home/ivanhu/fwts/src/acpica/fwts_acpica.c:693:
>> first defined here
>> collect2: ld returned 1 exit status
>> make[4]: *** [libfwtsacpica.la] Error 1
>> make[4]: Leaving directory `/home/ivanhu/fwts/src/acpica'
>> make[3]: *** [all] Error 2
>> make[3]: Leaving directory `/home/ivanhu/fwts/src/acpica'
>> make[2]: *** [all-recursive] Error 1
>> make[2]: Leaving directory `/home/ivanhu/fwts/src'
>> make[1]: *** [all-recursive] Error 1
>> make[1]: Leaving directory `/home/ivanhu/fwts'
>> make: *** [all] Error 2
>>
>>
>
> The "munged" file osunixxf_munged.c hasn't been re-munged, so do a "make
> clean" and re-make, it should be OK after that.
>
Acked-by: Ivan Hu<ivan.hu at canonical.com>
More information about the fwts-devel
mailing list