Re: [PATCH libdrm 1/6] amdgpu: stop using the hash table for fd_tab

["Zhang, Jerry (Junwei)" <Jerry.Zhang-5C7GfCeVMHo@public.gmane.org>]

On 08/03/2018 07:34 PM, Christian König wrote:
> We have so few devices that just walking a linked list is probably
> faster.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>

Series is

Reviewed-and-Tested-by: Junwei Zhang <Jerry.Zhang@amd.com>

> ---
>   amdgpu/amdgpu_device.c   | 49 ++++++++++++++++--------------------------------
>   amdgpu/amdgpu_internal.h |  1 +
>   2 files changed, 17 insertions(+), 33 deletions(-)
>
> diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c
> index d7aec6a4..38fd186d 100644
> --- a/amdgpu/amdgpu_device.c
> +++ b/amdgpu/amdgpu_device.c
> @@ -43,10 +43,9 @@
>   #include "util_math.h"
>
>   #define PTR_TO_UINT(x) ((unsigned)((intptr_t)(x)))
> -#define UINT_TO_PTR(x) ((void *)((intptr_t)(x)))
>
>   static pthread_mutex_t fd_mutex = PTHREAD_MUTEX_INITIALIZER;
> -static struct util_hash_table *fd_tab;
> +static amdgpu_device_handle fd_list;
>
>   static unsigned handle_hash(void *key)
>   {
> @@ -58,28 +57,8 @@ static int handle_compare(void *key1, void *key2)
>   	return PTR_TO_UINT(key1) != PTR_TO_UINT(key2);
>   }
>
> -static unsigned fd_hash(void *key)
> +static int fd_compare(int fd1, int fd2)
>   {
> -	int fd = PTR_TO_UINT(key);
> -	char *name = drmGetPrimaryDeviceNameFromFd(fd);
> -	unsigned result = 0;
> -	char *c;
> -
> -	if (name == NULL)
> -		return 0;
> -
> -	for (c = name; *c; ++c)
> -		result += *c;
> -
> -	free(name);
> -
> -	return result;
> -}
> -
> -static int fd_compare(void *key1, void *key2)
> -{
> -	int fd1 = PTR_TO_UINT(key1);
> -	int fd2 = PTR_TO_UINT(key2);
>   	char *name1 = drmGetPrimaryDeviceNameFromFd(fd1);
>   	char *name2 = drmGetPrimaryDeviceNameFromFd(fd2);
>   	int result;
> @@ -127,16 +106,17 @@ static int amdgpu_get_auth(int fd, int *auth)
>
>   static void amdgpu_device_free_internal(amdgpu_device_handle dev)
>   {
> +	amdgpu_device_handle *node = &fd_list;
> +
>   	pthread_mutex_lock(&fd_mutex);
> -	util_hash_table_remove(fd_tab, UINT_TO_PTR(dev->fd));
> -	if (util_hash_table_count(fd_tab) == 0) {
> -		util_hash_table_destroy(fd_tab);
> -		fd_tab = NULL;
> -	}
> +	while (*node != dev && (*node)->next)
> +		node = &(*node)->next;
> +	*node = (*node)->next;
> +	pthread_mutex_unlock(&fd_mutex);
> +
>   	close(dev->fd);
>   	if ((dev->flink_fd >= 0) && (dev->fd != dev->flink_fd))
>   		close(dev->flink_fd);
> -	pthread_mutex_unlock(&fd_mutex);
>
>   	amdgpu_vamgr_deinit(&dev->vamgr_32);
>   	amdgpu_vamgr_deinit(&dev->vamgr);
> @@ -187,8 +167,6 @@ int amdgpu_device_initialize(int fd,
>   	*device_handle = NULL;
>
>   	pthread_mutex_lock(&fd_mutex);
> -	if (!fd_tab)
> -		fd_tab = util_hash_table_create(fd_hash, fd_compare);
>   	r = amdgpu_get_auth(fd, &flag_auth);
>   	if (r) {
>   		fprintf(stderr, "%s: amdgpu_get_auth (1) failed (%i)\n",
> @@ -196,7 +174,11 @@ int amdgpu_device_initialize(int fd,
>   		pthread_mutex_unlock(&fd_mutex);
>   		return r;
>   	}
> -	dev = util_hash_table_get(fd_tab, UINT_TO_PTR(fd));
> +
> +	for (dev = fd_list; dev; dev = dev->next)
> +		if (fd_compare(dev->fd, fd) == 0)
> +			break;
> +
>   	if (dev) {
>   		r = amdgpu_get_auth(dev->fd, &flag_authexist);
>   		if (r) {
> @@ -297,7 +279,8 @@ int amdgpu_device_initialize(int fd,
>   	*major_version = dev->major_version;
>   	*minor_version = dev->minor_version;
>   	*device_handle = dev;
> -	util_hash_table_set(fd_tab, UINT_TO_PTR(dev->fd), dev);
> +	dev->next = fd_list;
> +	fd_list = dev;
>   	pthread_mutex_unlock(&fd_mutex);
>
>   	return 0;
> diff --git a/amdgpu/amdgpu_internal.h b/amdgpu/amdgpu_internal.h
> index 99b8ce0b..83012cab 100644
> --- a/amdgpu/amdgpu_internal.h
> +++ b/amdgpu/amdgpu_internal.h
> @@ -65,6 +65,7 @@ struct amdgpu_va {
>
>   struct amdgpu_device {
>   	atomic_t refcount;
> +	struct amdgpu_device *next;
>   	int fd;
>   	int flink_fd;
>   	unsigned major_version;
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx