Patchwork acpica: fwts_acpica: optimise semaphore tracking

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

Comments

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

The original semaphore tracking kept a track of semaphores used
by ACPICA by keeping a hash table.  Remove this overhead and just
have a static table of pre-allocated semaphores.  This removes the
need to allocate and free semaphores for Create/Destroy operation.

Also make the locking around this structure more restrictive to
ensure no race issues can occur.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/acpica/fwts_acpica.c | 152 +++++++++++++++++++----------------------------
 1 file changed, 61 insertions(+), 91 deletions(-)
Keng-Yu Lin - Jan. 14, 2013, 8:14 a.m.
On Sat, Jan 5, 2013 at 1:12 AM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The original semaphore tracking kept a track of semaphores used
> by ACPICA by keeping a hash table.  Remove this overhead and just
> have a static table of pre-allocated semaphores.  This removes the
> need to allocate and free semaphores for Create/Destroy operation.
>
> Also make the locking around this structure more restrictive to
> ensure no race issues can occur.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/acpica/fwts_acpica.c | 152 +++++++++++++++++++----------------------------
>  1 file changed, 61 insertions(+), 91 deletions(-)
>
> diff --git a/src/acpica/fwts_acpica.c b/src/acpica/fwts_acpica.c
> index d177401..2c489f1 100644
> --- a/src/acpica/fwts_acpica.c
> +++ b/src/acpica/fwts_acpica.c
> @@ -31,7 +31,7 @@
>  #include <signal.h>
>  #include <pthread.h>
>
> -static pthread_mutex_t mutex_lock_count;
> +static pthread_mutex_t mutex_lock_sem_table;
>  static pthread_mutex_t mutex_thread_info;
>
>  #include "fwts.h"
> @@ -52,15 +52,14 @@ static pthread_mutex_t mutex_thread_info;
>  #define ACPI_MAX_INIT_TABLES   (32)
>  #define AEXEC_NUM_REGIONS      (8)
>
> -#define MAX_SEMAPHORES         (1009)
> -#define HASH_FULL              (0xffffffff)
> -
> +#define MAX_SEMAPHORES         (1024)
>  #define MAX_THREADS            (128)
>
>  typedef struct {
> -       sem_t           *sem;   /* Semaphore handle */
> +       sem_t           sem;    /* Semaphore handle */
>         int             count;  /* count > 0 if acquired */
> -} sem_hash;
> +       bool            used;   /* Semaphore being used flag */
> +} sem_info;
>
>  typedef void * (*PTHREAD_CALLBACK)(void *);
>
> @@ -68,9 +67,9 @@ BOOLEAN AcpiGbl_IgnoreErrors = FALSE;
>  UINT8   AcpiGbl_RegionFillValue = 0;
>
>  /*
> - *  Hash table of semaphore handles
> + *  Used to account for semaphores used by AcpiOs*Semaphore()
>   */
> -static sem_hash                        sem_hash_table[MAX_SEMAPHORES];
> +static sem_info                        sem_table[MAX_SEMAPHORES];
>
>  static ACPI_TABLE_DESC         Tables[ACPI_MAX_INIT_TABLES];
>
> @@ -129,14 +128,12 @@ void fwts_acpica_sem_count_clear(void)
>  {
>         int i;
>
> -       pthread_mutex_lock(&mutex_lock_count);
> +       pthread_mutex_lock(&mutex_lock_sem_table);
>
> -       for (i=0;i<MAX_SEMAPHORES;i++) {
> -               sem_hash_table[i].sem = NULL;
> -               sem_hash_table[i].count = 0;
> -       }
> +       for (i = 0; i < MAX_SEMAPHORES; i++)
> +               sem_table[i].count = 0;
>
> -       pthread_mutex_unlock(&mutex_lock_count);
> +       pthread_mutex_unlock(&mutex_lock_sem_table);
>  }
>
>  /*
> @@ -171,13 +168,15 @@ void fwts_acpica_sem_count_get(int *acquired, int *released)
>          * 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) {
> +       pthread_mutex_lock(&mutex_lock_sem_table);
> +       for (i = 0; i < MAX_SEMAPHORES; i++) {
> +               if (sem_table[i].used) {
>                         (*acquired)++;
> -                       if (sem_hash_table[i].count == 0)
> +                       if (sem_table[i].count == 0)
>                                 (*released)++;
>                 }
>         }
> +       pthread_mutex_unlock(&mutex_lock_sem_table);
>  }
>
>  /*
> @@ -190,57 +189,6 @@ void fwts_acpica_simulate_sem_timeout(int timeout)
>         fwts_acpica_force_sem_timeout = timeout;
>  }
>
> -/*
> - *  hash_sem_handle()
> - *     generate a simple hash based on semaphore handle
> - */
> -static unsigned int hash_sem_handle(sem_t *sem)
> -{
> -       unsigned int i = (unsigned int)((unsigned long)sem % MAX_SEMAPHORES);
> -       int j;
> -
> -       for (j = 0; j<MAX_SEMAPHORES; j++) {
> -               if (sem_hash_table[i].sem == sem)
> -                       return i;
> -               if (sem_hash_table[i].sem == NULL)
> -                       return i;
> -               i = (i+1) % MAX_SEMAPHORES;
> -       }
> -       return HASH_FULL;
> -}
> -
> -/*
> - *  hash_sem_inc_count()
> - *     increment semaphore counter
> - */
> -static void hash_sem_inc_count(sem_t *sem)
> -{
> -       unsigned int i = hash_sem_handle(sem);
> -
> -       if (i != HASH_FULL) {
> -               pthread_mutex_lock(&mutex_lock_count);
> -               sem_hash_table[i].sem = sem;
> -               sem_hash_table[i].count++;
> -               pthread_mutex_unlock(&mutex_lock_count);
> -       }
> -}
> -
> -/*
> - *  hash_sem_dec_count()
> - *     decrement semaphore counter
> - */
> -static void hash_sem_dec_count(sem_t *sem)
> -{
> -       unsigned int i = hash_sem_handle(sem);
> -
> -       if (i != HASH_FULL) {
> -               pthread_mutex_lock(&mutex_lock_count);
> -               sem_hash_table[i].sem = sem;
> -               sem_hash_table[i].count--;
> -               pthread_mutex_unlock(&mutex_lock_count);
> -       }
> -}
> -
>  /* ACPICA Handlers */
>
>  /*
> @@ -555,23 +503,36 @@ ACPI_STATUS AcpiOsCreateSemaphore(UINT32 MaxUnits,
>         UINT32 InitialUnits,
>         ACPI_HANDLE *OutHandle)
>  {
> -       sem_t *sem;
> +       sem_info *sem = NULL;
> +       ACPI_STATUS ret = AE_OK;
> +       int i;
>
>         if (!OutHandle)
>                 return AE_BAD_PARAMETER;
>
> -       sem = AcpiOsAllocate(sizeof(sem_t));
> +       pthread_mutex_lock(&mutex_lock_sem_table);
> +       for (i = 0; i < MAX_SEMAPHORES; i++) {
> +               if (!sem_table[i].used) {
> +                       sem = &sem_table[i];
> +                       break;
> +               }
> +       }
> +
>         if (!sem)
>                 return AE_NO_MEMORY;
>
> -       if (sem_init(sem, 0, InitialUnits) == -1) {
> -               AcpiOsFree(sem);
> -               return AE_BAD_PARAMETER;
> -       }
> +       sem->used = true;
> +       sem->count = 0;
>
> -       *OutHandle = (ACPI_HANDLE)sem;
> +       if (sem_init(&sem->sem, 0, InitialUnits) == -1) {
> +               *OutHandle = NULL;
> +               ret = AE_NO_MEMORY;
> +       } else {
> +               *OutHandle = (ACPI_HANDLE)sem;
> +       }
> +       pthread_mutex_unlock(&mutex_lock_sem_table);
>
> -       return AE_OK;
> +       return ret;
>  }
>
>  /*
> @@ -580,17 +541,20 @@ ACPI_STATUS AcpiOsCreateSemaphore(UINT32 MaxUnits,
>   */
>  ACPI_STATUS AcpiOsDeleteSemaphore(ACPI_HANDLE Handle)
>  {
> -       sem_t *sem = (sem_t *)Handle;
> +       sem_info *sem = (sem_info *)Handle;
> +       ACPI_STATUS ret = AE_OK;
>
>         if (!sem)
>                 return AE_BAD_PARAMETER;
>
> -       if (sem_destroy(sem) == -1)
> -               return AE_BAD_PARAMETER;
> +       pthread_mutex_lock(&mutex_lock_sem_table);
> +       if (sem_destroy(&sem->sem) == -1)
> +               ret = AE_BAD_PARAMETER;
> +       sem->used = false;
>
> -       AcpiOsFree(sem);
> +       pthread_mutex_unlock(&mutex_lock_sem_table);
>
> -       return AE_OK;
> +       return ret;
>  }
>
>
> @@ -601,7 +565,7 @@ ACPI_STATUS AcpiOsDeleteSemaphore(ACPI_HANDLE Handle)
>   */
>  ACPI_STATUS AcpiOsWaitSemaphore(ACPI_HANDLE handle, UINT32 Units, UINT16 Timeout)
>  {
> -       sem_t   *sem = (sem_t *)handle;
> +       sem_info *sem = (sem_info *)handle;
>         struct timespec tm;
>
>         if (!handle)
> @@ -612,22 +576,26 @@ ACPI_STATUS AcpiOsWaitSemaphore(ACPI_HANDLE handle, UINT32 Units, UINT16 Timeout
>
>         switch (Timeout) {
>         case 0:
> -               if (sem_trywait(sem) < 0)
> +               if (sem_trywait(&sem->sem))
>                         return AE_TIME;
>                 break;
> +
>         case ACPI_WAIT_FOREVER:
> -               if (sem_wait(sem))
> +               if (sem_wait(&sem->sem))
>                         return AE_TIME;
>                 break;
>         default:
>                 tm.tv_sec = Timeout / 1000;
>                 tm.tv_nsec = (Timeout - (tm.tv_sec * 1000)) * 1000000;
>
> -               if (sem_timedwait(sem, &tm))
> +               if (sem_timedwait(&sem->sem, &tm))
>                         return AE_TIME;
>                 break;
>         }
> -       hash_sem_inc_count(sem);
> +
> +       pthread_mutex_lock(&mutex_lock_sem_table);
> +       sem->count++;
> +       pthread_mutex_unlock(&mutex_lock_sem_table);
>
>         return AE_OK;
>  }
> @@ -639,15 +607,17 @@ ACPI_STATUS AcpiOsWaitSemaphore(ACPI_HANDLE handle, UINT32 Units, UINT16 Timeout
>   */
>  ACPI_STATUS AcpiOsSignalSemaphore(ACPI_HANDLE handle, UINT32 Units)
>  {
> -       sem_t *sem = (sem_t *)handle;
> +       sem_info *sem = (sem_info *)handle;
>
>         if (!handle)
>                 return AE_BAD_PARAMETER;
>
> -       if (sem_post(sem) < 0)
> +       if (sem_post(&sem->sem) < 0)
>                 return AE_LIMIT;
>
> -       hash_sem_dec_count(sem);
> +       pthread_mutex_lock(&mutex_lock_sem_table);
> +       sem->count--;
> +       pthread_mutex_unlock(&mutex_lock_sem_table);
>
>         return AE_OK;
>  }
> @@ -919,7 +889,7 @@ int fwts_acpica_init(fwts_framework *fw)
>         if (fwts_acpica_init_called)
>                 return FWTS_ERROR;
>
> -       pthread_mutex_init(&mutex_lock_count, NULL);
> +       pthread_mutex_init(&mutex_lock_sem_table, NULL);
>         pthread_mutex_init(&mutex_thread_info, NULL);
>
>         fwts_acpica_fw = fw;
> @@ -1108,7 +1078,7 @@ int fwts_acpica_deinit(void)
>                 return FWTS_ERROR;
>
>         AcpiTerminate();
> -       pthread_mutex_destroy(&mutex_lock_count);
> +       pthread_mutex_destroy(&mutex_lock_sem_table);
>         pthread_mutex_destroy(&mutex_thread_info);
>
>         FWTS_ACPICA_FREE(fwts_acpica_XSDT);
> --
> 1.8.0
>
Acked-by: Keng-Yu Lin <kengyu@canonical.com>
Ivan Hu - Jan. 14, 2013, 10:07 a.m.
On 01/05/2013 01:12 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The original semaphore tracking kept a track of semaphores used
> by ACPICA by keeping a hash table.  Remove this overhead and just
> have a static table of pre-allocated semaphores.  This removes the
> need to allocate and free semaphores for Create/Destroy operation.
>
> Also make the locking around this structure more restrictive to
> ensure no race issues can occur.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/acpica/fwts_acpica.c | 152 +++++++++++++++++++----------------------------
>   1 file changed, 61 insertions(+), 91 deletions(-)
>
> diff --git a/src/acpica/fwts_acpica.c b/src/acpica/fwts_acpica.c
> index d177401..2c489f1 100644
> --- a/src/acpica/fwts_acpica.c
> +++ b/src/acpica/fwts_acpica.c
> @@ -31,7 +31,7 @@
>   #include <signal.h>
>   #include <pthread.h>
>
> -static pthread_mutex_t mutex_lock_count;
> +static pthread_mutex_t mutex_lock_sem_table;
>   static pthread_mutex_t mutex_thread_info;
>
>   #include "fwts.h"
> @@ -52,15 +52,14 @@ static pthread_mutex_t mutex_thread_info;
>   #define ACPI_MAX_INIT_TABLES	(32)
>   #define AEXEC_NUM_REGIONS   	(8)
>
> -#define MAX_SEMAPHORES		(1009)
> -#define HASH_FULL		(0xffffffff)
> -
> +#define MAX_SEMAPHORES		(1024)
>   #define MAX_THREADS		(128)
>
>   typedef struct {
> -	sem_t		*sem;	/* Semaphore handle */
> +	sem_t		sem;	/* Semaphore handle */
>   	int		count;	/* count > 0 if acquired */
> -} sem_hash;
> +	bool		used;	/* Semaphore being used flag */
> +} sem_info;
>
>   typedef void * (*PTHREAD_CALLBACK)(void *);
>
> @@ -68,9 +67,9 @@ BOOLEAN AcpiGbl_IgnoreErrors = FALSE;
>   UINT8   AcpiGbl_RegionFillValue = 0;
>
>   /*
> - *  Hash table of semaphore handles
> + *  Used to account for semaphores used by AcpiOs*Semaphore()
>    */
> -static sem_hash			sem_hash_table[MAX_SEMAPHORES];
> +static sem_info			sem_table[MAX_SEMAPHORES];
>
>   static ACPI_TABLE_DESC		Tables[ACPI_MAX_INIT_TABLES];
>
> @@ -129,14 +128,12 @@ void fwts_acpica_sem_count_clear(void)
>   {
>   	int i;
>
> -	pthread_mutex_lock(&mutex_lock_count);
> +	pthread_mutex_lock(&mutex_lock_sem_table);
>
> -	for (i=0;i<MAX_SEMAPHORES;i++) {
> -		sem_hash_table[i].sem = NULL;
> -		sem_hash_table[i].count = 0;
> -	}
> +	for (i = 0; i < MAX_SEMAPHORES; i++)
> +		sem_table[i].count = 0;
>
> -	pthread_mutex_unlock(&mutex_lock_count);
> +	pthread_mutex_unlock(&mutex_lock_sem_table);
>   }
>
>   /*
> @@ -171,13 +168,15 @@ void fwts_acpica_sem_count_get(int *acquired, int *released)
>   	 * 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) {
> +	pthread_mutex_lock(&mutex_lock_sem_table);
> +	for (i = 0; i < MAX_SEMAPHORES; i++) {
> +		if (sem_table[i].used) {
>   			(*acquired)++;
> -			if (sem_hash_table[i].count == 0)
> +			if (sem_table[i].count == 0)
>   				(*released)++;
>   		}
>   	}
> +	pthread_mutex_unlock(&mutex_lock_sem_table);
>   }
>
>   /*
> @@ -190,57 +189,6 @@ void fwts_acpica_simulate_sem_timeout(int timeout)
>   	fwts_acpica_force_sem_timeout = timeout;
>   }
>
> -/*
> - *  hash_sem_handle()
> - *	generate a simple hash based on semaphore handle
> - */
> -static unsigned int hash_sem_handle(sem_t *sem)
> -{
> -	unsigned int i = (unsigned int)((unsigned long)sem % MAX_SEMAPHORES);
> -	int j;
> -
> -	for (j = 0; j<MAX_SEMAPHORES; j++) {
> -		if (sem_hash_table[i].sem == sem)
> -			return i;
> -		if (sem_hash_table[i].sem == NULL)
> -			return i;
> -		i = (i+1) % MAX_SEMAPHORES;
> -	}
> -	return HASH_FULL;
> -}
> -
> -/*
> - *  hash_sem_inc_count()
> - *	increment semaphore counter
> - */
> -static void hash_sem_inc_count(sem_t *sem)
> -{
> -	unsigned int i = hash_sem_handle(sem);
> -
> -	if (i != HASH_FULL) {
> -		pthread_mutex_lock(&mutex_lock_count);
> -		sem_hash_table[i].sem = sem;
> -		sem_hash_table[i].count++;
> -		pthread_mutex_unlock(&mutex_lock_count);
> -	}
> -}
> -
> -/*
> - *  hash_sem_dec_count()
> - *	decrement semaphore counter
> - */
> -static void hash_sem_dec_count(sem_t *sem)
> -{
> -	unsigned int i = hash_sem_handle(sem);
> -
> -	if (i != HASH_FULL) {
> -		pthread_mutex_lock(&mutex_lock_count);
> -		sem_hash_table[i].sem = sem;
> -		sem_hash_table[i].count--;
> -		pthread_mutex_unlock(&mutex_lock_count);
> -	}
> -}
> -
>   /* ACPICA Handlers */
>
>   /*
> @@ -555,23 +503,36 @@ ACPI_STATUS AcpiOsCreateSemaphore(UINT32 MaxUnits,
>   	UINT32 InitialUnits,
>   	ACPI_HANDLE *OutHandle)
>   {
> -	sem_t *sem;
> +	sem_info *sem = NULL;
> +	ACPI_STATUS ret = AE_OK;
> +	int i;
>
>   	if (!OutHandle)
>   		return AE_BAD_PARAMETER;
>
> -	sem = AcpiOsAllocate(sizeof(sem_t));
> +	pthread_mutex_lock(&mutex_lock_sem_table);
> +	for (i = 0; i < MAX_SEMAPHORES; i++) {
> +		if (!sem_table[i].used) {
> +			sem = &sem_table[i];
> +			break;
> +		}
> +	}
> +
>   	if (!sem)
>   		return AE_NO_MEMORY;
>
> -	if (sem_init(sem, 0, InitialUnits) == -1) {
> -		AcpiOsFree(sem);
> -		return AE_BAD_PARAMETER;
> -	}
> +	sem->used = true;
> +	sem->count = 0;
>
> -	*OutHandle = (ACPI_HANDLE)sem;
> +	if (sem_init(&sem->sem, 0, InitialUnits) == -1) {
> +		*OutHandle = NULL;
> +		ret = AE_NO_MEMORY;
> +	} else {
> +		*OutHandle = (ACPI_HANDLE)sem;
> +	}
> +	pthread_mutex_unlock(&mutex_lock_sem_table);
>
> -	return AE_OK;
> +	return ret;
>   }
>
>   /*
> @@ -580,17 +541,20 @@ ACPI_STATUS AcpiOsCreateSemaphore(UINT32 MaxUnits,
>    */
>   ACPI_STATUS AcpiOsDeleteSemaphore(ACPI_HANDLE Handle)
>   {
> -	sem_t *sem = (sem_t *)Handle;
> +	sem_info *sem = (sem_info *)Handle;
> +	ACPI_STATUS ret = AE_OK;
>
>   	if (!sem)
>   		return AE_BAD_PARAMETER;
>
> -	if (sem_destroy(sem) == -1)
> -		return AE_BAD_PARAMETER;
> +	pthread_mutex_lock(&mutex_lock_sem_table);
> +	if (sem_destroy(&sem->sem) == -1)
> +		ret = AE_BAD_PARAMETER;
> +	sem->used = false;
>
> -	AcpiOsFree(sem);
> +	pthread_mutex_unlock(&mutex_lock_sem_table);
>
> -	return AE_OK;
> +	return ret;
>   }
>
>
> @@ -601,7 +565,7 @@ ACPI_STATUS AcpiOsDeleteSemaphore(ACPI_HANDLE Handle)
>    */
>   ACPI_STATUS AcpiOsWaitSemaphore(ACPI_HANDLE handle, UINT32 Units, UINT16 Timeout)
>   {
> -	sem_t	*sem = (sem_t *)handle;
> +	sem_info *sem = (sem_info *)handle;
>   	struct timespec	tm;
>
>   	if (!handle)
> @@ -612,22 +576,26 @@ ACPI_STATUS AcpiOsWaitSemaphore(ACPI_HANDLE handle, UINT32 Units, UINT16 Timeout
>
>   	switch (Timeout) {
>   	case 0:
> -		if (sem_trywait(sem) < 0)
> +		if (sem_trywait(&sem->sem))
>   			return AE_TIME;
>   		break;
> +
>   	case ACPI_WAIT_FOREVER:
> -		if (sem_wait(sem))
> +		if (sem_wait(&sem->sem))
>   			return AE_TIME;
>   		break;
>   	default:
>   		tm.tv_sec = Timeout / 1000;
>   		tm.tv_nsec = (Timeout - (tm.tv_sec * 1000)) * 1000000;
>
> -		if (sem_timedwait(sem, &tm))
> +		if (sem_timedwait(&sem->sem, &tm))
>   			return AE_TIME;
>   		break;
>   	}
> -	hash_sem_inc_count(sem);
> +
> +	pthread_mutex_lock(&mutex_lock_sem_table);
> +	sem->count++;
> +	pthread_mutex_unlock(&mutex_lock_sem_table);
>
>   	return AE_OK;
>   }
> @@ -639,15 +607,17 @@ ACPI_STATUS AcpiOsWaitSemaphore(ACPI_HANDLE handle, UINT32 Units, UINT16 Timeout
>    */
>   ACPI_STATUS AcpiOsSignalSemaphore(ACPI_HANDLE handle, UINT32 Units)
>   {
> -	sem_t *sem = (sem_t *)handle;
> +	sem_info *sem = (sem_info *)handle;
>
>   	if (!handle)
>   		return AE_BAD_PARAMETER;
>
> -	if (sem_post(sem) < 0)
> +	if (sem_post(&sem->sem) < 0)
>   		return AE_LIMIT;
>
> -	hash_sem_dec_count(sem);
> +	pthread_mutex_lock(&mutex_lock_sem_table);
> +	sem->count--;
> +	pthread_mutex_unlock(&mutex_lock_sem_table);
>
>   	return AE_OK;
>   }
> @@ -919,7 +889,7 @@ int fwts_acpica_init(fwts_framework *fw)
>   	if (fwts_acpica_init_called)
>   		return FWTS_ERROR;
>
> -	pthread_mutex_init(&mutex_lock_count, NULL);
> +	pthread_mutex_init(&mutex_lock_sem_table, NULL);
>   	pthread_mutex_init(&mutex_thread_info, NULL);
>
>   	fwts_acpica_fw = fw;
> @@ -1108,7 +1078,7 @@ int fwts_acpica_deinit(void)
>   		return FWTS_ERROR;
>
>   	AcpiTerminate();
> -	pthread_mutex_destroy(&mutex_lock_count);
> +	pthread_mutex_destroy(&mutex_lock_sem_table);
>   	pthread_mutex_destroy(&mutex_thread_info);
>
>   	FWTS_ACPICA_FREE(fwts_acpica_XSDT);
>

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

Patch

diff --git a/src/acpica/fwts_acpica.c b/src/acpica/fwts_acpica.c
index d177401..2c489f1 100644
--- a/src/acpica/fwts_acpica.c
+++ b/src/acpica/fwts_acpica.c
@@ -31,7 +31,7 @@ 
 #include <signal.h>
 #include <pthread.h>
 
-static pthread_mutex_t mutex_lock_count;
+static pthread_mutex_t mutex_lock_sem_table;
 static pthread_mutex_t mutex_thread_info;
 
 #include "fwts.h"
@@ -52,15 +52,14 @@  static pthread_mutex_t mutex_thread_info;
 #define ACPI_MAX_INIT_TABLES	(32)
 #define AEXEC_NUM_REGIONS   	(8)
 
-#define MAX_SEMAPHORES		(1009)
-#define HASH_FULL		(0xffffffff)
-
+#define MAX_SEMAPHORES		(1024)
 #define MAX_THREADS		(128)
 
 typedef struct {
-	sem_t		*sem;	/* Semaphore handle */
+	sem_t		sem;	/* Semaphore handle */
 	int		count;	/* count > 0 if acquired */
-} sem_hash;
+	bool		used;	/* Semaphore being used flag */
+} sem_info;
 
 typedef void * (*PTHREAD_CALLBACK)(void *);
 
@@ -68,9 +67,9 @@  BOOLEAN AcpiGbl_IgnoreErrors = FALSE;
 UINT8   AcpiGbl_RegionFillValue = 0;
 
 /*
- *  Hash table of semaphore handles
+ *  Used to account for semaphores used by AcpiOs*Semaphore()
  */
-static sem_hash			sem_hash_table[MAX_SEMAPHORES];
+static sem_info			sem_table[MAX_SEMAPHORES];
 
 static ACPI_TABLE_DESC		Tables[ACPI_MAX_INIT_TABLES];
 
@@ -129,14 +128,12 @@  void fwts_acpica_sem_count_clear(void)
 {
 	int i;
 
-	pthread_mutex_lock(&mutex_lock_count);
+	pthread_mutex_lock(&mutex_lock_sem_table);
 
-	for (i=0;i<MAX_SEMAPHORES;i++) {
-		sem_hash_table[i].sem = NULL;
-		sem_hash_table[i].count = 0;
-	}
+	for (i = 0; i < MAX_SEMAPHORES; i++)
+		sem_table[i].count = 0;
 
-	pthread_mutex_unlock(&mutex_lock_count);
+	pthread_mutex_unlock(&mutex_lock_sem_table);
 }
 
 /*
@@ -171,13 +168,15 @@  void fwts_acpica_sem_count_get(int *acquired, int *released)
 	 * 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) {
+	pthread_mutex_lock(&mutex_lock_sem_table);
+	for (i = 0; i < MAX_SEMAPHORES; i++) {
+		if (sem_table[i].used) {
 			(*acquired)++;
-			if (sem_hash_table[i].count == 0)
+			if (sem_table[i].count == 0)
 				(*released)++;
 		}
 	}
+	pthread_mutex_unlock(&mutex_lock_sem_table);
 }
 
 /*
@@ -190,57 +189,6 @@  void fwts_acpica_simulate_sem_timeout(int timeout)
 	fwts_acpica_force_sem_timeout = timeout;
 }
 
-/*
- *  hash_sem_handle()
- *	generate a simple hash based on semaphore handle
- */
-static unsigned int hash_sem_handle(sem_t *sem)
-{
-	unsigned int i = (unsigned int)((unsigned long)sem % MAX_SEMAPHORES);
-	int j;
-
-	for (j = 0; j<MAX_SEMAPHORES; j++) {
-		if (sem_hash_table[i].sem == sem)
-			return i;
-		if (sem_hash_table[i].sem == NULL)
-			return i;
-		i = (i+1) % MAX_SEMAPHORES;
-	}
-	return HASH_FULL;
-}
-
-/*
- *  hash_sem_inc_count()
- *	increment semaphore counter
- */
-static void hash_sem_inc_count(sem_t *sem)
-{
-	unsigned int i = hash_sem_handle(sem);
-
-	if (i != HASH_FULL) {
-		pthread_mutex_lock(&mutex_lock_count);
-		sem_hash_table[i].sem = sem;
-		sem_hash_table[i].count++;
-		pthread_mutex_unlock(&mutex_lock_count);
-	}
-}
-
-/*
- *  hash_sem_dec_count()
- *	decrement semaphore counter
- */
-static void hash_sem_dec_count(sem_t *sem)
-{
-	unsigned int i = hash_sem_handle(sem);
-
-	if (i != HASH_FULL) {
-		pthread_mutex_lock(&mutex_lock_count);
-		sem_hash_table[i].sem = sem;
-		sem_hash_table[i].count--;
-		pthread_mutex_unlock(&mutex_lock_count);
-	}
-}
-
 /* ACPICA Handlers */
 
 /*
@@ -555,23 +503,36 @@  ACPI_STATUS AcpiOsCreateSemaphore(UINT32 MaxUnits,
 	UINT32 InitialUnits,
 	ACPI_HANDLE *OutHandle)
 {
-	sem_t *sem;
+	sem_info *sem = NULL;
+	ACPI_STATUS ret = AE_OK;
+	int i;
 
 	if (!OutHandle)
 		return AE_BAD_PARAMETER;
 
-	sem = AcpiOsAllocate(sizeof(sem_t));
+	pthread_mutex_lock(&mutex_lock_sem_table);
+	for (i = 0; i < MAX_SEMAPHORES; i++) {
+		if (!sem_table[i].used) {
+			sem = &sem_table[i];
+			break;
+		}
+	}
+
 	if (!sem)
 		return AE_NO_MEMORY;
 
-	if (sem_init(sem, 0, InitialUnits) == -1) {
-		AcpiOsFree(sem);
-		return AE_BAD_PARAMETER;
-	}
+	sem->used = true;
+	sem->count = 0;
 
-	*OutHandle = (ACPI_HANDLE)sem;
+	if (sem_init(&sem->sem, 0, InitialUnits) == -1) {
+		*OutHandle = NULL;
+		ret = AE_NO_MEMORY;
+	} else {
+		*OutHandle = (ACPI_HANDLE)sem;
+	}
+	pthread_mutex_unlock(&mutex_lock_sem_table);
 
-	return AE_OK;
+	return ret;
 }
 
 /*
@@ -580,17 +541,20 @@  ACPI_STATUS AcpiOsCreateSemaphore(UINT32 MaxUnits,
  */
 ACPI_STATUS AcpiOsDeleteSemaphore(ACPI_HANDLE Handle)
 {
-	sem_t *sem = (sem_t *)Handle;
+	sem_info *sem = (sem_info *)Handle;
+	ACPI_STATUS ret = AE_OK;
 
 	if (!sem)
 		return AE_BAD_PARAMETER;
 
-	if (sem_destroy(sem) == -1)
-		return AE_BAD_PARAMETER;
+	pthread_mutex_lock(&mutex_lock_sem_table);
+	if (sem_destroy(&sem->sem) == -1)
+		ret = AE_BAD_PARAMETER;
+	sem->used = false;
 
-	AcpiOsFree(sem);
+	pthread_mutex_unlock(&mutex_lock_sem_table);
 
-	return AE_OK;
+	return ret;
 }
 
 
@@ -601,7 +565,7 @@  ACPI_STATUS AcpiOsDeleteSemaphore(ACPI_HANDLE Handle)
  */
 ACPI_STATUS AcpiOsWaitSemaphore(ACPI_HANDLE handle, UINT32 Units, UINT16 Timeout)
 {
-	sem_t	*sem = (sem_t *)handle;
+	sem_info *sem = (sem_info *)handle;
 	struct timespec	tm;
 
 	if (!handle)
@@ -612,22 +576,26 @@  ACPI_STATUS AcpiOsWaitSemaphore(ACPI_HANDLE handle, UINT32 Units, UINT16 Timeout
 
 	switch (Timeout) {
 	case 0:
-		if (sem_trywait(sem) < 0)
+		if (sem_trywait(&sem->sem))
 			return AE_TIME;
 		break;
+
 	case ACPI_WAIT_FOREVER:
-		if (sem_wait(sem))
+		if (sem_wait(&sem->sem))
 			return AE_TIME;
 		break;
 	default:
 		tm.tv_sec = Timeout / 1000;
 		tm.tv_nsec = (Timeout - (tm.tv_sec * 1000)) * 1000000;
 
-		if (sem_timedwait(sem, &tm))
+		if (sem_timedwait(&sem->sem, &tm))
 			return AE_TIME;
 		break;
 	}
-	hash_sem_inc_count(sem);
+
+	pthread_mutex_lock(&mutex_lock_sem_table);
+	sem->count++;
+	pthread_mutex_unlock(&mutex_lock_sem_table);
 
 	return AE_OK;
 }
@@ -639,15 +607,17 @@  ACPI_STATUS AcpiOsWaitSemaphore(ACPI_HANDLE handle, UINT32 Units, UINT16 Timeout
  */
 ACPI_STATUS AcpiOsSignalSemaphore(ACPI_HANDLE handle, UINT32 Units)
 {
-	sem_t *sem = (sem_t *)handle;
+	sem_info *sem = (sem_info *)handle;
 
 	if (!handle)
 		return AE_BAD_PARAMETER;
 
-	if (sem_post(sem) < 0)
+	if (sem_post(&sem->sem) < 0)
 		return AE_LIMIT;
 
-	hash_sem_dec_count(sem);
+	pthread_mutex_lock(&mutex_lock_sem_table);
+	sem->count--;
+	pthread_mutex_unlock(&mutex_lock_sem_table);
 
 	return AE_OK;
 }
@@ -919,7 +889,7 @@  int fwts_acpica_init(fwts_framework *fw)
 	if (fwts_acpica_init_called)
 		return FWTS_ERROR;
 
-	pthread_mutex_init(&mutex_lock_count, NULL);
+	pthread_mutex_init(&mutex_lock_sem_table, NULL);
 	pthread_mutex_init(&mutex_thread_info, NULL);
 
 	fwts_acpica_fw = fw;
@@ -1108,7 +1078,7 @@  int fwts_acpica_deinit(void)
 		return FWTS_ERROR;
 
 	AcpiTerminate();
-	pthread_mutex_destroy(&mutex_lock_count);
+	pthread_mutex_destroy(&mutex_lock_sem_table);
 	pthread_mutex_destroy(&mutex_thread_info);
 
 	FWTS_ACPICA_FREE(fwts_acpica_XSDT);