Patchwork lib: acpica: fix semaphore counting by waiting for threads to complete

login
register
mail settings
Submitter Colin King
Date July 19, 2012, 8:44 a.m.
Message ID <1342687458-11966-1-git-send-email-colin.king@canonical.com>
Download mbox | patch
Permalink /patch/171908/
State Accepted
Headers show

Comments

Colin King - July 19, 2012, 8:44 a.m.
From: Colin Ian King <colin.king@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@canonical.com>
---
 src/acpica/Makefile.am   |    3 +-
 src/acpica/fwts_acpica.c |  106 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 108 insertions(+), 1 deletion(-)
Ivan Hu - July 20, 2012, 9:55 a.m.
On 07/19/2012 04:44 PM, Colin King wrote:
> From: Colin Ian King <colin.king@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@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
Colin King - July 20, 2012, 4:02 p.m.
On 20/07/12 02:55, IvanHu wrote:
> On 07/19/2012 04:44 PM, Colin King wrote:
>> From: Colin Ian King <colin.king@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@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.
Ivan Hu - July 23, 2012, 3:55 a.m.
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@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@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@canonical.com>
Keng-Yu Lin - July 23, 2012, 5:33 a.m.
On Thu, Jul 19, 2012 at 4:44 PM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@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@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);
> --
> 1.7.10.4
>
Acked-by: Keng-Yu Lin <kengyu@canonical.com>

Patch

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);