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

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

[Christian König]

We have so few devices that just walking a linked list is probably
faster.

Signed-off-by: Christian König <christian.koenig@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;
-- 
2.14.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH libdrm 2/6] amdgpu: add handle table implementation v2

[Christian König]

The kernel handles are dense and the kernel always tries to use the
lowest free id. Use this to implement a more efficient handle table
by using a resizeable array instead of a hash.

v2: add handle_table_fini function, extra key checks,
    fix typo in function name

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 amdgpu/Makefile.sources |  4 ++-
 amdgpu/handle_table.c   | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
 amdgpu/handle_table.h   | 41 ++++++++++++++++++++++++++++
 3 files changed, 116 insertions(+), 1 deletion(-)
 create mode 100644 amdgpu/handle_table.c
 create mode 100644 amdgpu/handle_table.h

diff --git a/amdgpu/Makefile.sources b/amdgpu/Makefile.sources
index 498b64cc..62577ba5 100644
--- a/amdgpu/Makefile.sources
+++ b/amdgpu/Makefile.sources
@@ -10,7 +10,9 @@ LIBDRM_AMDGPU_FILES := \
 	util_hash.c \
 	util_hash.h \
 	util_hash_table.c \
-	util_hash_table.h
+	util_hash_table.h \
+	handle_table.c \
+	handle_table.h
 
 LIBDRM_AMDGPU_H_FILES := \
 	amdgpu.h
diff --git a/amdgpu/handle_table.c b/amdgpu/handle_table.c
new file mode 100644
index 00000000..15cd4763
--- /dev/null
+++ b/amdgpu/handle_table.c
@@ -0,0 +1,72 @@
+/*
+ * Copyright 2018 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <unistd.h>
+#include "handle_table.h"
+#include "util_math.h"
+
+drm_private int handle_table_insert(struct handle_table *table, uint32_t key,
+				    void *value)
+{
+	if (key >= table->max_key) {
+		uint32_t alignment = sysconf(_SC_PAGESIZE) / sizeof(void*);
+		uint32_t max_key = ALIGN(key, alignment);
+		void **values;
+
+		values = realloc(table->values, max_key * sizeof(void *));
+		if (!values)
+			return -ENOMEM;
+
+		memset(values + table->max_key, 0, (max_key - table->max_key) *
+		       sizeof(void *));
+
+		table->max_key = max_key;
+		table->values = values;
+	}
+	table->values[key] = value;
+	return 0;
+}
+
+drm_private void handle_table_remove(struct handle_table *table, uint32_t key)
+{
+	if (key < table->max_key)
+		table->values[key] = NULL;
+}
+
+drm_private void *handle_table_lookup(struct handle_table *table, uint32_t key)
+{
+	if (key < table->max_key)
+		return table->values[key];
+	else
+		return NULL;
+}
+
+drm_private void handle_table_fini(struct handle_table *table)
+{
+	free(table->values);
+	table->max_key = 0;
+	table->values = NULL;
+}
diff --git a/amdgpu/handle_table.h b/amdgpu/handle_table.h
new file mode 100644
index 00000000..461193f6
--- /dev/null
+++ b/amdgpu/handle_table.h
@@ -0,0 +1,41 @@
+/*
+ * Copyright 2018 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#ifndef _HANDLE_TABLE_H_
+#define _HANDLE_TABLE_H_
+
+#include <stdint.h>
+#include "libdrm_macros.h"
+
+struct handle_table {
+	uint32_t	max_key;
+	void		**values;
+};
+
+drm_private int handle_table_insert(struct handle_table *table, uint32_t key,
+				    void *value);
+drm_private void handle_table_remove(struct handle_table *table, uint32_t key);
+drm_private void *handle_table_lookup(struct handle_table *table, uint32_t key);
+drm_private void handle_table_fini(struct handle_table *table);
+
+#endif /* _HANDLE_TABLE_H_ */
-- 
2.14.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 2/6] amdgpu: add handle table implementation v2

[Zhang, Jerry (Junwei)]

On 08/03/2018 07:34 PM, Christian König wrote:
> The kernel handles are dense and the kernel always tries to use the
> lowest free id. Use this to implement a more efficient handle table
> by using a resizeable array instead of a hash.
>
> v2: add handle_table_fini function, extra key checks,
>      fix typo in function name
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   amdgpu/Makefile.sources |  4 ++-
>   amdgpu/handle_table.c   | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
>   amdgpu/handle_table.h   | 41 ++++++++++++++++++++++++++++
>   3 files changed, 116 insertions(+), 1 deletion(-)
>   create mode 100644 amdgpu/handle_table.c
>   create mode 100644 amdgpu/handle_table.h
>
> diff --git a/amdgpu/Makefile.sources b/amdgpu/Makefile.sources
> index 498b64cc..62577ba5 100644
> --- a/amdgpu/Makefile.sources
> +++ b/amdgpu/Makefile.sources
> @@ -10,7 +10,9 @@ LIBDRM_AMDGPU_FILES := \
>   	util_hash.c \
>   	util_hash.h \
>   	util_hash_table.c \
> -	util_hash_table.h
> +	util_hash_table.h \
> +	handle_table.c \
> +	handle_table.h
>
>   LIBDRM_AMDGPU_H_FILES := \
>   	amdgpu.h
> diff --git a/amdgpu/handle_table.c b/amdgpu/handle_table.c
> new file mode 100644
> index 00000000..15cd4763
> --- /dev/null
> +++ b/amdgpu/handle_table.c
> @@ -0,0 +1,72 @@
> +/*
> + * Copyright 2018 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + */
> +
> +#include <stdlib.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <unistd.h>
> +#include "handle_table.h"
> +#include "util_math.h"
> +
> +drm_private int handle_table_insert(struct handle_table *table, uint32_t key,
> +				    void *value)
> +{
> +	if (key >= table->max_key) {
> +		uint32_t alignment = sysconf(_SC_PAGESIZE) / sizeof(void*);
> +		uint32_t max_key = ALIGN(key, alignment);

On 2nd thought, we may add table->count to take the role of current table->max_key,
and table_max_key could be updated by key rather than aligned value.

Then we could traverse the table by max_key instead of the entire table.

Regards,
Jerry

> +		void **values;
> +
> +		values = realloc(table->values, max_key * sizeof(void *));
> +		if (!values)
> +			return -ENOMEM;
> +
> +		memset(values + table->max_key, 0, (max_key - table->max_key) *
> +		       sizeof(void *));
> +
> +		table->max_key = max_key;
> +		table->values = values;
> +	}
> +	table->values[key] = value;
> +	return 0;
> +}
> +
> +drm_private void handle_table_remove(struct handle_table *table, uint32_t key)
> +{
> +	if (key < table->max_key)
> +		table->values[key] = NULL;
> +}
> +
> +drm_private void *handle_table_lookup(struct handle_table *table, uint32_t key)
> +{
> +	if (key < table->max_key)
> +		return table->values[key];
> +	else
> +		return NULL;
> +}
> +
> +drm_private void handle_table_fini(struct handle_table *table)
> +{
> +	free(table->values);
> +	table->max_key = 0;
> +	table->values = NULL;
> +}
> diff --git a/amdgpu/handle_table.h b/amdgpu/handle_table.h
> new file mode 100644
> index 00000000..461193f6
> --- /dev/null
> +++ b/amdgpu/handle_table.h
> @@ -0,0 +1,41 @@
> +/*
> + * Copyright 2018 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + */
> +
> +#ifndef _HANDLE_TABLE_H_
> +#define _HANDLE_TABLE_H_
> +
> +#include <stdint.h>
> +#include "libdrm_macros.h"
> +
> +struct handle_table {
> +	uint32_t	max_key;
> +	void		**values;
> +};
> +
> +drm_private int handle_table_insert(struct handle_table *table, uint32_t key,
> +				    void *value);
> +drm_private void handle_table_remove(struct handle_table *table, uint32_t key);
> +drm_private void *handle_table_lookup(struct handle_table *table, uint32_t key);
> +drm_private void handle_table_fini(struct handle_table *table);
> +
> +#endif /* _HANDLE_TABLE_H_ */
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH libdrm 3/6] amdgpu: use handle table for KMS handles

[Christian König]

Instead of the hash use the handle table.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 amdgpu/amdgpu_bo.c       | 19 ++++++++++---------
 amdgpu/amdgpu_device.c   |  3 +--
 amdgpu/amdgpu_internal.h |  3 ++-
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
index d29be244..66edb8d4 100644
--- a/amdgpu/amdgpu_bo.c
+++ b/amdgpu/amdgpu_bo.c
@@ -90,8 +90,12 @@ int amdgpu_bo_alloc(amdgpu_device_handle dev,
 
 	pthread_mutex_init(&bo->cpu_access_mutex, NULL);
 
-	*buf_handle = bo;
-	return 0;
+	if (r)
+		amdgpu_bo_free(bo);
+	else
+		*buf_handle = bo;
+
+	return r;
 }
 
 int amdgpu_bo_set_metadata(amdgpu_bo_handle bo,
@@ -171,8 +175,7 @@ int amdgpu_bo_query_info(amdgpu_bo_handle bo,
 static void amdgpu_add_handle_to_table(amdgpu_bo_handle bo)
 {
 	pthread_mutex_lock(&bo->dev->bo_table_mutex);
-	util_hash_table_set(bo->dev->bo_handles,
-			    (void*)(uintptr_t)bo->handle, bo);
+	handle_table_insert(&bo->dev->bo_handles, bo->handle, bo);
 	pthread_mutex_unlock(&bo->dev->bo_table_mutex);
 }
 
@@ -303,8 +306,7 @@ int amdgpu_bo_import(amdgpu_device_handle dev,
 		break;
 
 	case amdgpu_bo_handle_type_dma_buf_fd:
-		bo = util_hash_table_get(dev->bo_handles,
-					 (void*)(uintptr_t)shared_handle);
+		bo = handle_table_lookup(&dev->bo_handles, shared_handle);
 		break;
 
 	case amdgpu_bo_handle_type_kms:
@@ -387,7 +389,7 @@ int amdgpu_bo_import(amdgpu_device_handle dev,
 	bo->dev = dev;
 	pthread_mutex_init(&bo->cpu_access_mutex, NULL);
 
-	util_hash_table_set(dev->bo_handles, (void*)(uintptr_t)bo->handle, bo);
+	handle_table_insert(&dev->bo_handles, bo->handle, bo);
 	pthread_mutex_unlock(&dev->bo_table_mutex);
 
 	output->buf_handle = bo;
@@ -406,8 +408,7 @@ int amdgpu_bo_free(amdgpu_bo_handle buf_handle)
 
 	if (update_references(&bo->refcount, NULL)) {
 		/* Remove the buffer from the hash tables. */
-		util_hash_table_remove(dev->bo_handles,
-					(void*)(uintptr_t)bo->handle);
+		handle_table_remove(&dev->bo_handles, bo->handle);
 
 		if (bo->flink_name) {
 			util_hash_table_remove(dev->bo_flink_names,
diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c
index 38fd186d..824c1dae 100644
--- a/amdgpu/amdgpu_device.c
+++ b/amdgpu/amdgpu_device.c
@@ -122,8 +122,8 @@ static void amdgpu_device_free_internal(amdgpu_device_handle dev)
 	amdgpu_vamgr_deinit(&dev->vamgr);
 	amdgpu_vamgr_deinit(&dev->vamgr_high_32);
 	amdgpu_vamgr_deinit(&dev->vamgr_high);
+	handle_table_fini(&dev->bo_handles);
 	util_hash_table_destroy(dev->bo_flink_names);
-	util_hash_table_destroy(dev->bo_handles);
 	pthread_mutex_destroy(&dev->bo_table_mutex);
 	free(dev->marketing_name);
 	free(dev);
@@ -230,7 +230,6 @@ int amdgpu_device_initialize(int fd,
 
 	dev->bo_flink_names = util_hash_table_create(handle_hash,
 						     handle_compare);
-	dev->bo_handles = util_hash_table_create(handle_hash, handle_compare);
 	pthread_mutex_init(&dev->bo_table_mutex, NULL);
 
 	/* Check if acceleration is working. */
diff --git a/amdgpu/amdgpu_internal.h b/amdgpu/amdgpu_internal.h
index 83012cab..36ebc738 100644
--- a/amdgpu/amdgpu_internal.h
+++ b/amdgpu/amdgpu_internal.h
@@ -32,6 +32,7 @@
 #include "xf86atomic.h"
 #include "amdgpu.h"
 #include "util_double_list.h"
+#include "handle_table.h"
 
 #define AMDGPU_CS_MAX_RINGS 8
 /* do not use below macro if b is not power of 2 aligned value */
@@ -73,7 +74,7 @@ struct amdgpu_device {
 
 	char *marketing_name;
 	/** List of buffer handles. Protected by bo_table_mutex. */
-	struct util_hash_table *bo_handles;
+	struct handle_table bo_handles;
 	/** List of buffer GEM flink names. Protected by bo_table_mutex. */
 	struct util_hash_table *bo_flink_names;
 	/** This protects all hash tables. */
-- 
2.14.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH libdrm 4/6] amdgpu: use handle table for flink names

[Christian König]

Instead of the hash use the handle table.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 amdgpu/amdgpu_bo.c       | 26 +++++++++++++-------------
 amdgpu/amdgpu_device.c   | 15 +--------------
 amdgpu/amdgpu_internal.h |  2 +-
 3 files changed, 15 insertions(+), 28 deletions(-)

diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
index 66edb8d4..02592377 100644
--- a/amdgpu/amdgpu_bo.c
+++ b/amdgpu/amdgpu_bo.c
@@ -37,7 +37,6 @@
 #include "xf86drm.h"
 #include "amdgpu_drm.h"
 #include "amdgpu_internal.h"
-#include "util_hash_table.h"
 #include "util_math.h"
 
 static void amdgpu_close_kms_handle(amdgpu_device_handle dev,
@@ -219,12 +218,10 @@ static int amdgpu_bo_export_flink(amdgpu_bo_handle bo)
 	}
 
 	pthread_mutex_lock(&bo->dev->bo_table_mutex);
-	util_hash_table_set(bo->dev->bo_flink_names,
-			    (void*)(uintptr_t)bo->flink_name,
-			    bo);
+	r = handle_table_insert(&bo->dev->bo_flink_names, bo->flink_name, bo);
 	pthread_mutex_unlock(&bo->dev->bo_table_mutex);
 
-	return 0;
+	return r;
 }
 
 int amdgpu_bo_export(amdgpu_bo_handle bo,
@@ -301,8 +298,7 @@ int amdgpu_bo_import(amdgpu_device_handle dev,
 	/* If we have already created a buffer with this handle, find it. */
 	switch (type) {
 	case amdgpu_bo_handle_type_gem_flink_name:
-		bo = util_hash_table_get(dev->bo_flink_names,
-					 (void*)(uintptr_t)shared_handle);
+		bo = handle_table_lookup(&dev->bo_flink_names, shared_handle);
 		break;
 
 	case amdgpu_bo_handle_type_dma_buf_fd:
@@ -370,8 +366,13 @@ int amdgpu_bo_import(amdgpu_device_handle dev,
 		}
 		bo->flink_name = shared_handle;
 		bo->alloc_size = open_arg.size;
-		util_hash_table_set(dev->bo_flink_names,
-				    (void*)(uintptr_t)bo->flink_name, bo);
+		r = handle_table_insert(&dev->bo_flink_names, shared_handle,
+					bo);
+		if (r) {
+			pthread_mutex_unlock(&dev->bo_table_mutex);
+			amdgpu_bo_free(bo);
+			return r;
+		}
 		break;
 
 	case amdgpu_bo_handle_type_dma_buf_fd:
@@ -410,10 +411,9 @@ int amdgpu_bo_free(amdgpu_bo_handle buf_handle)
 		/* Remove the buffer from the hash tables. */
 		handle_table_remove(&dev->bo_handles, bo->handle);
 
-		if (bo->flink_name) {
-			util_hash_table_remove(dev->bo_flink_names,
-						(void*)(uintptr_t)bo->flink_name);
-		}
+		if (bo->flink_name)
+			handle_table_remove(&dev->bo_flink_names,
+					    bo->flink_name);
 
 		/* Release CPU access. */
 		if (bo->cpu_map_count > 0) {
diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c
index 824c1dae..73eeeed6 100644
--- a/amdgpu/amdgpu_device.c
+++ b/amdgpu/amdgpu_device.c
@@ -39,7 +39,6 @@
 #include "xf86drm.h"
 #include "amdgpu_drm.h"
 #include "amdgpu_internal.h"
-#include "util_hash_table.h"
 #include "util_math.h"
 
 #define PTR_TO_UINT(x) ((unsigned)((intptr_t)(x)))
@@ -47,16 +46,6 @@
 static pthread_mutex_t fd_mutex = PTHREAD_MUTEX_INITIALIZER;
 static amdgpu_device_handle fd_list;
 
-static unsigned handle_hash(void *key)
-{
-	return PTR_TO_UINT(key);
-}
-
-static int handle_compare(void *key1, void *key2)
-{
-	return PTR_TO_UINT(key1) != PTR_TO_UINT(key2);
-}
-
 static int fd_compare(int fd1, int fd2)
 {
 	char *name1 = drmGetPrimaryDeviceNameFromFd(fd1);
@@ -123,7 +112,7 @@ static void amdgpu_device_free_internal(amdgpu_device_handle dev)
 	amdgpu_vamgr_deinit(&dev->vamgr_high_32);
 	amdgpu_vamgr_deinit(&dev->vamgr_high);
 	handle_table_fini(&dev->bo_handles);
-	util_hash_table_destroy(dev->bo_flink_names);
+	handle_table_fini(&dev->bo_flink_names);
 	pthread_mutex_destroy(&dev->bo_table_mutex);
 	free(dev->marketing_name);
 	free(dev);
@@ -228,8 +217,6 @@ int amdgpu_device_initialize(int fd,
 	dev->minor_version = version->version_minor;
 	drmFreeVersion(version);
 
-	dev->bo_flink_names = util_hash_table_create(handle_hash,
-						     handle_compare);
 	pthread_mutex_init(&dev->bo_table_mutex, NULL);
 
 	/* Check if acceleration is working. */
diff --git a/amdgpu/amdgpu_internal.h b/amdgpu/amdgpu_internal.h
index 36ebc738..a340abbd 100644
--- a/amdgpu/amdgpu_internal.h
+++ b/amdgpu/amdgpu_internal.h
@@ -76,7 +76,7 @@ struct amdgpu_device {
 	/** List of buffer handles. Protected by bo_table_mutex. */
 	struct handle_table bo_handles;
 	/** List of buffer GEM flink names. Protected by bo_table_mutex. */
-	struct util_hash_table *bo_flink_names;
+	struct handle_table bo_flink_names;
 	/** This protects all hash tables. */
 	pthread_mutex_t bo_table_mutex;
 	struct drm_amdgpu_info_device dev_info;
-- 
2.14.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH libdrm 5/6] amdgpu: remove the hash table implementation

[Christian König]

Not used any more.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 amdgpu/Makefile.sources  |   4 -
 amdgpu/util_hash.c       | 383 -----------------------------------------------
 amdgpu/util_hash.h       | 103 -------------
 amdgpu/util_hash_table.c | 270 ---------------------------------
 amdgpu/util_hash_table.h |  71 ---------
 5 files changed, 831 deletions(-)
 delete mode 100644 amdgpu/util_hash.c
 delete mode 100644 amdgpu/util_hash.h
 delete mode 100644 amdgpu/util_hash_table.c
 delete mode 100644 amdgpu/util_hash_table.h

diff --git a/amdgpu/Makefile.sources b/amdgpu/Makefile.sources
index 62577ba5..d6df324a 100644
--- a/amdgpu/Makefile.sources
+++ b/amdgpu/Makefile.sources
@@ -7,10 +7,6 @@ LIBDRM_AMDGPU_FILES := \
 	amdgpu_internal.h \
 	amdgpu_vamgr.c \
 	amdgpu_vm.c \
-	util_hash.c \
-	util_hash.h \
-	util_hash_table.c \
-	util_hash_table.h \
 	handle_table.c \
 	handle_table.h
 
diff --git a/amdgpu/util_hash.c b/amdgpu/util_hash.c
deleted file mode 100644
index 7e590419..00000000
--- a/amdgpu/util_hash.c
+++ /dev/null
@@ -1,383 +0,0 @@
-/**************************************************************************
- *
- * Copyright 2007 VMware, Inc.
- * All Rights Reserved.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sub license, and/or sell copies of the Software, and to
- * permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- *
- * The above copyright notice and this permission notice (including the
- * next paragraph) shall be included in all copies or substantial portions
- * of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
- * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
- * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
- * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE LIABLE FOR
- * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
- * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
- * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
- *
- **************************************************************************/
-
- /*
-  * Authors:
-  *   Zack Rusin <zackr@vmware.com>
-  */
-
-#include "util_hash.h"
-
-#include <stdlib.h>
-#include <assert.h>
-
-#define MAX(a, b) ((a > b) ? (a) : (b))
-
-static const int MinNumBits = 4;
-
-static const unsigned char prime_deltas[] = {
-	0,  0,  1,  3,  1,  5,  3,  3,  1,  9,  7,  5,  3,  9, 25,  3,
-	1, 21,  3, 21,  7, 15,  9,  5,  3, 29, 15,  0,  0,  0,  0,  0
-};
-
-static int primeForNumBits(int numBits)
-{
-	return (1 << numBits) + prime_deltas[numBits];
-}
-
-/* Returns the smallest integer n such that
-   primeForNumBits(n) >= hint.
-*/
-static int countBits(int hint)
-{
-	int numBits = 0;
-	int bits = hint;
-
-	while (bits > 1) {
-		bits >>= 1;
-		numBits++;
-	}
-
-	if (numBits >= (int)sizeof(prime_deltas)) {
-		numBits = sizeof(prime_deltas) - 1;
-	} else if (primeForNumBits(numBits) < hint) {
-		++numBits;
-	}
-	return numBits;
-}
-
-struct util_node {
-   struct util_node *next;
-   unsigned key;
-   void *value;
-};
-
-struct util_hash_data {
-   struct util_node *fakeNext;
-   struct util_node **buckets;
-   int size;
-   int nodeSize;
-   short userNumBits;
-   short numBits;
-   int numBuckets;
-};
-
-struct util_hash {
-   union {
-      struct util_hash_data *d;
-      struct util_node      *e;
-   } data;
-};
-
-static void *util_data_allocate_node(struct util_hash_data *hash)
-{
-   return malloc(hash->nodeSize);
-}
-
-static void util_free_node(struct util_node *node)
-{
-   free(node);
-}
-
-static struct util_node *
-util_hash_create_node(struct util_hash *hash,
-                      unsigned akey, void *avalue,
-                      struct util_node **anextNode)
-{
-   struct util_node *node = util_data_allocate_node(hash->data.d);
-
-   if (!node)
-      return NULL;
-
-   node->key = akey;
-   node->value = avalue;
-
-   node->next = (struct util_node*)(*anextNode);
-   *anextNode = node;
-   ++hash->data.d->size;
-   return node;
-}
-
-static void util_data_rehash(struct util_hash_data *hash, int hint)
-{
-   if (hint < 0) {
-      hint = countBits(-hint);
-      if (hint < MinNumBits)
-         hint = MinNumBits;
-      hash->userNumBits = (short)hint;
-      while (primeForNumBits(hint) < (hash->size >> 1))
-         ++hint;
-   } else if (hint < MinNumBits) {
-      hint = MinNumBits;
-   }
-
-   if (hash->numBits != hint) {
-      struct util_node *e = (struct util_node *)(hash);
-      struct util_node **oldBuckets = hash->buckets;
-      int oldNumBuckets = hash->numBuckets;
-      int  i = 0;
-
-      hash->numBits = (short)hint;
-      hash->numBuckets = primeForNumBits(hint);
-      hash->buckets = malloc(sizeof(struct util_node*) * hash->numBuckets);
-      for (i = 0; i < hash->numBuckets; ++i)
-         hash->buckets[i] = e;
-
-      for (i = 0; i < oldNumBuckets; ++i) {
-         struct util_node *firstNode = oldBuckets[i];
-         while (firstNode != e) {
-            unsigned h = firstNode->key;
-            struct util_node *lastNode = firstNode;
-            struct util_node *afterLastNode;
-            struct util_node **beforeFirstNode;
-            
-            while (lastNode->next != e && lastNode->next->key == h)
-               lastNode = lastNode->next;
-
-            afterLastNode = lastNode->next;
-            beforeFirstNode = &hash->buckets[h % hash->numBuckets];
-            while (*beforeFirstNode != e)
-               beforeFirstNode = &(*beforeFirstNode)->next;
-            lastNode->next = *beforeFirstNode;
-            *beforeFirstNode = firstNode;
-            firstNode = afterLastNode;
-         }
-      }
-      free(oldBuckets);
-   }
-}
-
-static void util_data_might_grow(struct util_hash_data *hash)
-{
-   if (hash->size >= hash->numBuckets)
-      util_data_rehash(hash, hash->numBits + 1);
-}
-
-static void util_data_has_shrunk(struct util_hash_data *hash)
-{
-   if (hash->size <= (hash->numBuckets >> 3) &&
-       hash->numBits > hash->userNumBits) {
-      int max = MAX(hash->numBits-2, hash->userNumBits);
-      util_data_rehash(hash,  max);
-   }
-}
-
-static struct util_node *util_data_first_node(struct util_hash_data *hash)
-{
-   struct util_node *e = (struct util_node *)(hash);
-   struct util_node **bucket = hash->buckets;
-   int n = hash->numBuckets;
-   while (n--) {
-      if (*bucket != e)
-         return *bucket;
-      ++bucket;
-   }
-   return e;
-}
-
-static struct util_node **util_hash_find_node(struct util_hash *hash, unsigned akey)
-{
-   struct util_node **node;
-
-   if (hash->data.d->numBuckets) {
-      node = (struct util_node **)(&hash->data.d->buckets[akey % hash->data.d->numBuckets]);
-      assert(*node == hash->data.e || (*node)->next);
-      while (*node != hash->data.e && (*node)->key != akey)
-         node = &(*node)->next;
-   } else {
-      node = (struct util_node **)((const struct util_node * const *)(&hash->data.e));
-   }
-   return node;
-}
-
-drm_private struct util_hash_iter
-util_hash_insert(struct util_hash *hash, unsigned key, void *data)
-{
-   util_data_might_grow(hash->data.d);
-
-   {
-      struct util_node **nextNode = util_hash_find_node(hash, key);
-      struct util_node *node = util_hash_create_node(hash, key, data, nextNode);
-      if (!node) {
-         struct util_hash_iter null_iter = {hash, 0};
-         return null_iter;
-      }
-
-      {
-         struct util_hash_iter iter = {hash, node};
-         return iter;
-      }
-   }
-}
-
-drm_private struct util_hash *util_hash_create(void)
-{
-   struct util_hash *hash = malloc(sizeof(struct util_hash));
-   if (!hash)
-      return NULL;
-
-   hash->data.d = malloc(sizeof(struct util_hash_data));
-   if (!hash->data.d) {
-      free(hash);
-      return NULL;
-   }
-
-   hash->data.d->fakeNext = 0;
-   hash->data.d->buckets = 0;
-   hash->data.d->size = 0;
-   hash->data.d->nodeSize = sizeof(struct util_node);
-   hash->data.d->userNumBits = (short)MinNumBits;
-   hash->data.d->numBits = 0;
-   hash->data.d->numBuckets = 0;
-
-   return hash;
-}
-
-drm_private void util_hash_delete(struct util_hash *hash)
-{
-   struct util_node *e_for_x = (struct util_node *)(hash->data.d);
-   struct util_node **bucket = (struct util_node **)(hash->data.d->buckets);
-   int n = hash->data.d->numBuckets;
-   while (n--) {
-      struct util_node *cur = *bucket++;
-      while (cur != e_for_x) {
-         struct util_node *next = cur->next;
-         util_free_node(cur);
-         cur = next;
-      }
-   }
-   free(hash->data.d->buckets);
-   free(hash->data.d);
-   free(hash);
-}
-
-drm_private struct util_hash_iter
-util_hash_find(struct util_hash *hash, unsigned key)
-{
-   struct util_node **nextNode = util_hash_find_node(hash, key);
-   struct util_hash_iter iter = {hash, *nextNode};
-   return iter;
-}
-
-drm_private unsigned util_hash_iter_key(struct util_hash_iter iter)
-{
-   if (!iter.node || iter.hash->data.e == iter.node)
-      return 0;
-   return iter.node->key;
-}
-
-drm_private void *util_hash_iter_data(struct util_hash_iter iter)
-{
-   if (!iter.node || iter.hash->data.e == iter.node)
-      return 0;
-   return iter.node->value;
-}
-
-static struct util_node *util_hash_data_next(struct util_node *node)
-{
-   union {
-      struct util_node *next;
-      struct util_node *e;
-      struct util_hash_data *d;
-   } a;
-   int start;
-   struct util_node **bucket;
-   int n;
-
-   a.next = node->next;
-   if (!a.next) {
-      /* iterating beyond the last element */
-      return 0;
-   }
-   if (a.next->next)
-      return a.next;
-
-   start = (node->key % a.d->numBuckets) + 1;
-   bucket = a.d->buckets + start;
-   n = a.d->numBuckets - start;
-   while (n--) {
-      if (*bucket != a.e)
-         return *bucket;
-      ++bucket;
-   }
-   return a.e;
-}
-
-drm_private struct util_hash_iter
-util_hash_iter_next(struct util_hash_iter iter)
-{
-   struct util_hash_iter next = {iter.hash, util_hash_data_next(iter.node)};
-   return next;
-}
-
-drm_private int util_hash_iter_is_null(struct util_hash_iter iter)
-{
-   if (!iter.node || iter.node == iter.hash->data.e)
-      return 1;
-   return 0;
-}
-
-drm_private void *util_hash_take(struct util_hash *hash, unsigned akey)
-{
-   struct util_node **node = util_hash_find_node(hash, akey);
-   if (*node != hash->data.e) {
-      void *t = (*node)->value;
-      struct util_node *next = (*node)->next;
-      util_free_node(*node);
-      *node = next;
-      --hash->data.d->size;
-      util_data_has_shrunk(hash->data.d);
-      return t;
-   }
-   return 0;
-}
-
-drm_private struct util_hash_iter util_hash_first_node(struct util_hash *hash)
-{
-   struct util_hash_iter iter = {hash, util_data_first_node(hash->data.d)};
-   return iter;
-}
-
-drm_private struct util_hash_iter
-util_hash_erase(struct util_hash *hash, struct util_hash_iter iter)
-{
-   struct util_hash_iter ret = iter;
-   struct util_node *node = iter.node;
-   struct util_node **node_ptr;
-
-   if (node == hash->data.e)
-      return iter;
-
-   ret = util_hash_iter_next(ret);
-   node_ptr = (struct util_node**)(&hash->data.d->buckets[node->key % hash->data.d->numBuckets]);
-   while (*node_ptr != node)
-      node_ptr = &(*node_ptr)->next;
-   *node_ptr = node->next;
-   util_free_node(node);
-   --hash->data.d->size;
-   return ret;
-}
diff --git a/amdgpu/util_hash.h b/amdgpu/util_hash.h
deleted file mode 100644
index 6eed1569..00000000
--- a/amdgpu/util_hash.h
+++ /dev/null
@@ -1,103 +0,0 @@
-/**************************************************************************
- *
- * Copyright 2007 VMware, Inc.
- * All Rights Reserved.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sub license, and/or sell copies of the Software, and to
- * permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- *
- * The above copyright notice and this permission notice (including the
- * next paragraph) shall be included in all copies or substantial portions
- * of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
- * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
- * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
- * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE LIABLE FOR
- * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
- * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
- * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
- *
- **************************************************************************/
-
-/**
- * @file
- * Hash implementation.
- * 
- * This file provides a hash implementation that is capable of dealing
- * with collisions. It stores colliding entries in linked list. All
- * functions operating on the hash return an iterator. The iterator
- * itself points to the collision list. If there wasn't any collision
- * the list will have just one entry, otherwise client code should
- * iterate over the entries to find the exact entry among ones that
- * had the same key (e.g. memcmp could be used on the data to check
- * that)
- * 
- * @author Zack Rusin <zackr@vmware.com>
- */
-
-#ifndef UTIL_HASH_H
-#define UTIL_HASH_H
-
-#include <stdbool.h>
-
-#include "libdrm_macros.h"
-
-struct util_hash;
-struct util_node;
-
-struct util_hash_iter {
-	struct util_hash *hash;
-	struct util_node *node;
-};
-
-
-drm_private struct util_hash *util_hash_create(void);
-drm_private void util_hash_delete(struct util_hash *hash);
-
-
-/**
- * Adds a data with the given key to the hash. If entry with the given
- * key is already in the hash, this current entry is instered before it
- * in the collision list.
- * Function returns iterator pointing to the inserted item in the hash.
- */
-drm_private struct util_hash_iter
-util_hash_insert(struct util_hash *hash, unsigned key, void *data);
-
-/**
- * Removes the item pointed to by the current iterator from the hash.
- * Note that the data itself is not erased and if it was a malloc'ed pointer
- * it will have to be freed after calling this function by the callee.
- * Function returns iterator pointing to the item after the removed one in
- * the hash.
- */
-drm_private struct util_hash_iter
-util_hash_erase(struct util_hash *hash, struct util_hash_iter iter);
-
-drm_private void *util_hash_take(struct util_hash *hash, unsigned key);
-
-
-drm_private struct util_hash_iter util_hash_first_node(struct util_hash *hash);
-
-/**
- * Return an iterator pointing to the first entry in the collision list.
- */
-drm_private struct util_hash_iter
-util_hash_find(struct util_hash *hash, unsigned key);
-
-
-drm_private int util_hash_iter_is_null(struct util_hash_iter iter);
-drm_private unsigned util_hash_iter_key(struct util_hash_iter iter);
-drm_private void *util_hash_iter_data(struct util_hash_iter iter);
-
-
-drm_private struct util_hash_iter
-util_hash_iter_next(struct util_hash_iter iter);
-
-#endif
diff --git a/amdgpu/util_hash_table.c b/amdgpu/util_hash_table.c
deleted file mode 100644
index e06d4415..00000000
--- a/amdgpu/util_hash_table.c
+++ /dev/null
@@ -1,270 +0,0 @@
-/**************************************************************************
- *
- * Copyright 2008 VMware, Inc.
- * All Rights Reserved.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sub license, and/or sell copies of the Software, and to
- * permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- *
- * The above copyright notice and this permission notice (including the
- * next paragraph) shall be included in all copies or substantial portions
- * of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
- * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
- * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
- * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE LIABLE FOR
- * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
- * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
- * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
- *
- **************************************************************************/
-
-/**
- * @file
- * General purpose hash table implementation.
- * 
- * Just uses the util_hash for now, but it might be better switch to a linear
- * probing hash table implementation at some point -- as it is said they have 
- * better lookup and cache performance and it appears to be possible to write 
- * a lock-free implementation of such hash tables . 
- * 
- * @author José Fonseca <jfonseca@vmware.com>
- */
-
-
-#include "util_hash_table.h"
-#include "util_hash.h"
-
-#include <stdlib.h>
-#include <assert.h>
-
-struct util_hash_table
-{
-	struct util_hash *head;
-
-	/** Hash function */
-	unsigned (*make_hash)(void *key);
-
-	/** Compare two keys */
-	int (*compare)(void *key1, void *key2);
-};
-
-struct util_hash_table_item
-{
-	void *key;
-	void *value;
-};
-
-
-static struct util_hash_table_item *
-util_hash_table_item(struct util_hash_iter iter)
-{
-	return (struct util_hash_table_item *)util_hash_iter_data(iter);
-}
-
-drm_private struct util_hash_table *
-util_hash_table_create(unsigned (*hash)(void *key),
-		       int (*compare)(void *key1, void *key2))
-{
-	struct util_hash_table *ht;
-
-	ht = malloc(sizeof(struct util_hash_table));
-	if(!ht)
-		return NULL;
-
-	ht->head = util_hash_create();
-	if(!ht->head) {
-		free(ht);
-		return NULL;
-	}
-
-	ht->make_hash = hash;
-	ht->compare = compare;
-
-	return ht;
-}
-
-static struct util_hash_iter
-util_hash_table_find_iter(struct util_hash_table *ht,
-			  void *key, unsigned key_hash)
-{
-	struct util_hash_iter iter;
-	struct util_hash_table_item *item;
-
-	iter = util_hash_find(ht->head, key_hash);
-	while (!util_hash_iter_is_null(iter)) {
-		item = (struct util_hash_table_item *)util_hash_iter_data(iter);
-		if (!ht->compare(item->key, key))
-			break;
-		iter = util_hash_iter_next(iter);
-	}
-
-	return iter;
-}
-
-static struct util_hash_table_item *
-util_hash_table_find_item(struct util_hash_table *ht,
-                          void *key, unsigned key_hash)
-{
-	struct util_hash_iter iter;
-	struct util_hash_table_item *item;
-
-	iter = util_hash_find(ht->head, key_hash);
-	while (!util_hash_iter_is_null(iter)) {
-		item = (struct util_hash_table_item *)util_hash_iter_data(iter);
-		if (!ht->compare(item->key, key))
-			return item;
-		iter = util_hash_iter_next(iter);
-	}
-
-	return NULL;
-}
-
-drm_private void
-util_hash_table_set(struct util_hash_table *ht, void *key, void *value)
-{
-	unsigned key_hash;
-	struct util_hash_table_item *item;
-	struct util_hash_iter iter;
-
-	assert(ht);
-	if (!ht)
-		return;
-
-	key_hash = ht->make_hash(key);
-
-	item = util_hash_table_find_item(ht, key, key_hash);
-	if(item) {
-		/* TODO: key/value destruction? */
-		item->value = value;
-		return;
-	}
-
-	item = malloc(sizeof(struct util_hash_table_item));
-	if(!item)
-		return;
-
-	item->key = key;
-	item->value = value;
-
-	iter = util_hash_insert(ht->head, key_hash, item);
-	if(util_hash_iter_is_null(iter)) {
-		free(item);
-		return;
-	}
-}
-
-drm_private void *util_hash_table_get(struct util_hash_table *ht, void *key)
-{
-	unsigned key_hash;
-	struct util_hash_table_item *item;
-
-	assert(ht);
-	if (!ht)
-		return NULL;
-
-	key_hash = ht->make_hash(key);
-
-	item = util_hash_table_find_item(ht, key, key_hash);
-	if(!item)
-		return NULL;
-
-	return item->value;
-}
-
-drm_private void util_hash_table_remove(struct util_hash_table *ht, void *key)
-{
-	unsigned key_hash;
-	struct util_hash_iter iter;
-	struct util_hash_table_item *item;
-
-	assert(ht);
-	if (!ht)
-		return;
-
-	key_hash = ht->make_hash(key);
-
-	iter = util_hash_table_find_iter(ht, key, key_hash);
-	if(util_hash_iter_is_null(iter))
-		return;
-
-	item = util_hash_table_item(iter);
-	assert(item);
-	free(item);
-
-	util_hash_erase(ht->head, iter);
-}
-
-drm_private void util_hash_table_clear(struct util_hash_table *ht)
-{
-	struct util_hash_iter iter;
-	struct util_hash_table_item *item;
-
-	assert(ht);
-	if (!ht)
-		return;
-
-	iter = util_hash_first_node(ht->head);
-	while (!util_hash_iter_is_null(iter)) {
-		item = (struct util_hash_table_item *)util_hash_take(ht->head, util_hash_iter_key(iter));
-		free(item);
-		iter = util_hash_first_node(ht->head);
-	}
-}
-
-drm_private void util_hash_table_foreach(struct util_hash_table *ht,
-			void (*callback)(void *key, void *value, void *data),
-			void *data)
-{
-	struct util_hash_iter iter;
-	struct util_hash_table_item *item;
-
-	assert(ht);
-	if (!ht)
-		return;
-
-	iter = util_hash_first_node(ht->head);
-	while (!util_hash_iter_is_null(iter)) {
-		item = (struct util_hash_table_item *)util_hash_iter_data(iter);
-		callback(item->key, item->value, data);
-		iter = util_hash_iter_next(iter);
-	}
-}
-
-static void util_hash_table_inc(void *k, void *v, void *d)
-{
-	++*(size_t *)d;
-}
-
-drm_private size_t util_hash_table_count(struct util_hash_table *ht)
-{
-	size_t count = 0;
-	util_hash_table_foreach(ht, util_hash_table_inc, &count);
-	return count;
-}
-
-drm_private void util_hash_table_destroy(struct util_hash_table *ht)
-{
-	struct util_hash_iter iter;
-	struct util_hash_table_item *item;
-
-	assert(ht);
-	if (!ht)
-		return;
-
-	iter = util_hash_first_node(ht->head);
-	while (!util_hash_iter_is_null(iter)) {
-		item = (struct util_hash_table_item *)util_hash_iter_data(iter);
-		free(item);
-		iter = util_hash_iter_next(iter);
-	}
-
-	util_hash_delete(ht->head);
-	free(ht);
-}
diff --git a/amdgpu/util_hash_table.h b/amdgpu/util_hash_table.h
deleted file mode 100644
index 3ab81a12..00000000
--- a/amdgpu/util_hash_table.h
+++ /dev/null
@@ -1,71 +0,0 @@
-/**************************************************************************
- *
- * Copyright 2008 VMware, Inc.
- * All Rights Reserved.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sub license, and/or sell copies of the Software, and to
- * permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- *
- * The above copyright notice and this permission notice (including the
- * next paragraph) shall be included in all copies or substantial portions
- * of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
- * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
- * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
- * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE LIABLE FOR
- * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
- * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
- * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
- *
- **************************************************************************/
-
-/**
- * General purpose hash table.
- *  
- * @author José Fonseca <jfonseca@vmware.com>
- */
-
-#ifndef U_HASH_TABLE_H_
-#define U_HASH_TABLE_H_
-
-#include "libdrm_macros.h"
-
-/**
- * Generic purpose hash table.
- */
-struct util_hash_table;
-
-/**
- * Create an hash table.
- * 
- * @param hash hash function
- * @param compare should return 0 for two equal keys.
- */
-drm_private struct util_hash_table *
-util_hash_table_create(unsigned (*hash)(void *key),
-		       int (*compare)(void *key1, void *key2));
-
-drm_private void
-util_hash_table_set(struct util_hash_table *ht, void *key, void *value);
-
-drm_private void *util_hash_table_get(struct util_hash_table *ht, void *key);
-
-drm_private void util_hash_table_remove(struct util_hash_table *ht, void *key);
-
-drm_private void util_hash_table_clear(struct util_hash_table *ht);
-
-drm_private void util_hash_table_foreach(struct util_hash_table *ht,
-			void (*callback)(void *key, void *value, void *data),
-			void *data);
-
-drm_private size_t util_hash_table_count(struct util_hash_table *ht);
-
-drm_private void util_hash_table_destroy(struct util_hash_table *ht);
-
-#endif /* U_HASH_TABLE_H_ */
-- 
2.14.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH libdrm 6/6] amdgpu: always add all BOs to lockup table

[Christian König]

This way we can always find a BO structure by its handle.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 amdgpu/amdgpu_bo.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
index 02592377..422c7c99 100644
--- a/amdgpu/amdgpu_bo.c
+++ b/amdgpu/amdgpu_bo.c
@@ -87,6 +87,10 @@ int amdgpu_bo_alloc(amdgpu_device_handle dev,
 
 	bo->handle = args.out.handle;
 
+	pthread_mutex_lock(&bo->dev->bo_table_mutex);
+	r = handle_table_insert(&bo->dev->bo_handles, bo->handle, bo);
+	pthread_mutex_unlock(&bo->dev->bo_table_mutex);
+
 	pthread_mutex_init(&bo->cpu_access_mutex, NULL);
 
 	if (r)
@@ -171,13 +175,6 @@ int amdgpu_bo_query_info(amdgpu_bo_handle bo,
 	return 0;
 }
 
-static void amdgpu_add_handle_to_table(amdgpu_bo_handle bo)
-{
-	pthread_mutex_lock(&bo->dev->bo_table_mutex);
-	handle_table_insert(&bo->dev->bo_handles, bo->handle, bo);
-	pthread_mutex_unlock(&bo->dev->bo_table_mutex);
-}
-
 static int amdgpu_bo_export_flink(amdgpu_bo_handle bo)
 {
 	struct drm_gem_flink flink;
@@ -240,14 +237,11 @@ int amdgpu_bo_export(amdgpu_bo_handle bo,
 		return 0;
 
 	case amdgpu_bo_handle_type_kms:
-		amdgpu_add_handle_to_table(bo);
-		/* fall through */
 	case amdgpu_bo_handle_type_kms_noimport:
 		*shared_handle = bo->handle;
 		return 0;
 
 	case amdgpu_bo_handle_type_dma_buf_fd:
-		amdgpu_add_handle_to_table(bo);
 		return drmPrimeHandleToFD(bo->dev->fd, bo->handle,
 					  DRM_CLOEXEC | DRM_RDWR,
 					  (int*)shared_handle);
-- 
2.14.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 6/6] amdgpu: always add all BOs to lockup table

[Michel Dänzer]

On 2018-08-03 01:34 PM, Christian König wrote:
> This way we can always find a BO structure by its handle.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

In the shortlog, should be "handle table" instead of "lockup table"?

With that fixed, the series is

Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>


Thanks for doing this!


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 6/6] amdgpu: always add all BOs to lockup table

[Christian König]

Am 03.08.2018 um 15:52 schrieb Michel Dänzer:
> On 2018-08-03 01:34 PM, Christian König wrote:
>> This way we can always find a BO structure by its handle.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
> In the shortlog, should be "handle table" instead of "lockup table"?
>
> With that fixed, the series is
>
> Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
>
>
> Thanks for doing this!
No, problem. Could you fix up the subject line and push it upstream?

I still don't have a working fdo account again :( Alternatively do you 
know an admin I can ping about that?

Thanks,
Christian.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 6/6] amdgpu: always add all BOs to lockup table

[Deucher, Alexander]

[-- Attachment #1.1: Type: text/plain, Size: 1321 bytes --]

Ping ajax or daniels or anholt on IRC.


Alex

________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Christian König <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Sent: Friday, August 3, 2018 9:54:42 AM
To: Michel Dänzer
Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH libdrm 6/6] amdgpu: always add all BOs to lockup table

Am 03.08.2018 um 15:52 schrieb Michel Dänzer:
> On 2018-08-03 01:34 PM, Christian König wrote:
>> This way we can always find a BO structure by its handle.
>>
>> Signed-off-by: Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org>
> In the shortlog, should be "handle table" instead of "lockup table"?
>
> With that fixed, the series is
>
> Reviewed-by: Michel Dänzer <michel.daenzer-5C7GfCeVMHo@public.gmane.org>
>
>
> Thanks for doing this!
No, problem. Could you fix up the subject line and push it upstream?

I still don't have a working fdo account again :( Alternatively do you
know an admin I can ping about that?

Thanks,
Christian.
_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[-- Attachment #1.2: Type: text/html, Size: 2418 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 6/6] amdgpu: always add all BOs to lockup table

[Marek Olšák]

I don't think this is a good idea. Can you please explain why this
won't cause performance regressions?

Thanks
Marek

On Fri, Aug 3, 2018 at 7:34 AM, Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
> This way we can always find a BO structure by its handle.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  amdgpu/amdgpu_bo.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
> index 02592377..422c7c99 100644
> --- a/amdgpu/amdgpu_bo.c
> +++ b/amdgpu/amdgpu_bo.c
> @@ -87,6 +87,10 @@ int amdgpu_bo_alloc(amdgpu_device_handle dev,
>
>         bo->handle = args.out.handle;
>
> +       pthread_mutex_lock(&bo->dev->bo_table_mutex);
> +       r = handle_table_insert(&bo->dev->bo_handles, bo->handle, bo);
> +       pthread_mutex_unlock(&bo->dev->bo_table_mutex);
> +
>         pthread_mutex_init(&bo->cpu_access_mutex, NULL);
>
>         if (r)
> @@ -171,13 +175,6 @@ int amdgpu_bo_query_info(amdgpu_bo_handle bo,
>         return 0;
>  }
>
> -static void amdgpu_add_handle_to_table(amdgpu_bo_handle bo)
> -{
> -       pthread_mutex_lock(&bo->dev->bo_table_mutex);
> -       handle_table_insert(&bo->dev->bo_handles, bo->handle, bo);
> -       pthread_mutex_unlock(&bo->dev->bo_table_mutex);
> -}
> -
>  static int amdgpu_bo_export_flink(amdgpu_bo_handle bo)
>  {
>         struct drm_gem_flink flink;
> @@ -240,14 +237,11 @@ int amdgpu_bo_export(amdgpu_bo_handle bo,
>                 return 0;
>
>         case amdgpu_bo_handle_type_kms:
> -               amdgpu_add_handle_to_table(bo);
> -               /* fall through */
>         case amdgpu_bo_handle_type_kms_noimport:
>                 *shared_handle = bo->handle;
>                 return 0;
>
>         case amdgpu_bo_handle_type_dma_buf_fd:
> -               amdgpu_add_handle_to_table(bo);
>                 return drmPrimeHandleToFD(bo->dev->fd, bo->handle,
>                                           DRM_CLOEXEC | DRM_RDWR,
>                                           (int*)shared_handle);
> --
> 2.14.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 6/6] amdgpu: always add all BOs to lockup table

[Christian König]

Why should it? Adding the handle is now not more than setting an array 
entry.

I've tested with allocating 250k BOs of 4k size each and there wasn't 
any measurable performance differences.

Christian.

Am 09.08.2018 um 18:56 schrieb Marek Olšák:
> I don't think this is a good idea. Can you please explain why this
> won't cause performance regressions?
>
> Thanks
> Marek
>
> On Fri, Aug 3, 2018 at 7:34 AM, Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>> This way we can always find a BO structure by its handle.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   amdgpu/amdgpu_bo.c | 14 ++++----------
>>   1 file changed, 4 insertions(+), 10 deletions(-)
>>
>> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
>> index 02592377..422c7c99 100644
>> --- a/amdgpu/amdgpu_bo.c
>> +++ b/amdgpu/amdgpu_bo.c
>> @@ -87,6 +87,10 @@ int amdgpu_bo_alloc(amdgpu_device_handle dev,
>>
>>          bo->handle = args.out.handle;
>>
>> +       pthread_mutex_lock(&bo->dev->bo_table_mutex);
>> +       r = handle_table_insert(&bo->dev->bo_handles, bo->handle, bo);
>> +       pthread_mutex_unlock(&bo->dev->bo_table_mutex);
>> +
>>          pthread_mutex_init(&bo->cpu_access_mutex, NULL);
>>
>>          if (r)
>> @@ -171,13 +175,6 @@ int amdgpu_bo_query_info(amdgpu_bo_handle bo,
>>          return 0;
>>   }
>>
>> -static void amdgpu_add_handle_to_table(amdgpu_bo_handle bo)
>> -{
>> -       pthread_mutex_lock(&bo->dev->bo_table_mutex);
>> -       handle_table_insert(&bo->dev->bo_handles, bo->handle, bo);
>> -       pthread_mutex_unlock(&bo->dev->bo_table_mutex);
>> -}
>> -
>>   static int amdgpu_bo_export_flink(amdgpu_bo_handle bo)
>>   {
>>          struct drm_gem_flink flink;
>> @@ -240,14 +237,11 @@ int amdgpu_bo_export(amdgpu_bo_handle bo,
>>                  return 0;
>>
>>          case amdgpu_bo_handle_type_kms:
>> -               amdgpu_add_handle_to_table(bo);
>> -               /* fall through */
>>          case amdgpu_bo_handle_type_kms_noimport:
>>                  *shared_handle = bo->handle;
>>                  return 0;
>>
>>          case amdgpu_bo_handle_type_dma_buf_fd:
>> -               amdgpu_add_handle_to_table(bo);
>>                  return drmPrimeHandleToFD(bo->dev->fd, bo->handle,
>>                                            DRM_CLOEXEC | DRM_RDWR,
>>                                            (int*)shared_handle);
>> --
>> 2.14.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH libdrm 6/6] amdgpu: always add all BOs to lockup table

[Marek Olšák]

OK. Thanks.

Marek

On Fri, Aug 10, 2018 at 9:06 AM, Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
> Why should it? Adding the handle is now not more than setting an array
> entry.
>
> I've tested with allocating 250k BOs of 4k size each and there wasn't any
> measurable performance differences.
>
> Christian.
>
>
> Am 09.08.2018 um 18:56 schrieb Marek Olšák:
>>
>> I don't think this is a good idea. Can you please explain why this
>> won't cause performance regressions?
>>
>> Thanks
>> Marek
>>
>> On Fri, Aug 3, 2018 at 7:34 AM, Christian König
>> <ckoenig.leichtzumerken@gmail.com> wrote:
>>>
>>> This way we can always find a BO structure by its handle.
>>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> ---
>>>   amdgpu/amdgpu_bo.c | 14 ++++----------
>>>   1 file changed, 4 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
>>> index 02592377..422c7c99 100644
>>> --- a/amdgpu/amdgpu_bo.c
>>> +++ b/amdgpu/amdgpu_bo.c
>>> @@ -87,6 +87,10 @@ int amdgpu_bo_alloc(amdgpu_device_handle dev,
>>>
>>>          bo->handle = args.out.handle;
>>>
>>> +       pthread_mutex_lock(&bo->dev->bo_table_mutex);
>>> +       r = handle_table_insert(&bo->dev->bo_handles, bo->handle, bo);
>>> +       pthread_mutex_unlock(&bo->dev->bo_table_mutex);
>>> +
>>>          pthread_mutex_init(&bo->cpu_access_mutex, NULL);
>>>
>>>          if (r)
>>> @@ -171,13 +175,6 @@ int amdgpu_bo_query_info(amdgpu_bo_handle bo,
>>>          return 0;
>>>   }
>>>
>>> -static void amdgpu_add_handle_to_table(amdgpu_bo_handle bo)
>>> -{
>>> -       pthread_mutex_lock(&bo->dev->bo_table_mutex);
>>> -       handle_table_insert(&bo->dev->bo_handles, bo->handle, bo);
>>> -       pthread_mutex_unlock(&bo->dev->bo_table_mutex);
>>> -}
>>> -
>>>   static int amdgpu_bo_export_flink(amdgpu_bo_handle bo)
>>>   {
>>>          struct drm_gem_flink flink;
>>> @@ -240,14 +237,11 @@ int amdgpu_bo_export(amdgpu_bo_handle bo,
>>>                  return 0;
>>>
>>>          case amdgpu_bo_handle_type_kms:
>>> -               amdgpu_add_handle_to_table(bo);
>>> -               /* fall through */
>>>          case amdgpu_bo_handle_type_kms_noimport:
>>>                  *shared_handle = bo->handle;
>>>                  return 0;
>>>
>>>          case amdgpu_bo_handle_type_dma_buf_fd:
>>> -               amdgpu_add_handle_to_table(bo);
>>>                  return drmPrimeHandleToFD(bo->dev->fd, bo->handle,
>>>                                            DRM_CLOEXEC | DRM_RDWR,
>>>                                            (int*)shared_handle);
>>> --
>>> 2.14.1
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Zhang, Jerry (Junwei)]

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