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

[PATCH libdrm 1/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 | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
index d29be244..39228620 100644
--- a/amdgpu/amdgpu_bo.c
+++ b/amdgpu/amdgpu_bo.c
@@ -88,6 +88,11 @@ int amdgpu_bo_alloc(amdgpu_device_handle dev,
 
 	bo->handle = args.out.handle;
 
+	pthread_mutex_lock(&bo->dev->bo_table_mutex);
+	util_hash_table_set(bo->dev->bo_handles,
+			    (void*)(uintptr_t)bo->handle, bo);
+	pthread_mutex_unlock(&bo->dev->bo_table_mutex);
+
 	pthread_mutex_init(&bo->cpu_access_mutex, NULL);
 
 	*buf_handle = bo;
@@ -168,14 +173,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);
-	util_hash_table_set(bo->dev->bo_handles,
-			    (void*)(uintptr_t)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

[PATCH libdrm 2/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 3/6] amdgpu: add handle table implementation

[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.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 amdgpu/Makefile.sources |  4 +++-
 amdgpu/handle_table.c   | 59 +++++++++++++++++++++++++++++++++++++++++++++++++
 amdgpu/handle_table.h   | 40 +++++++++++++++++++++++++++++++++
 3 files changed, 102 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..9acc44d5
--- /dev/null
+++ b/amdgpu/handle_table.c
@@ -0,0 +1,59 @@
+/*
+ * 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 "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 max_key = ALIGN(key, 512);
+		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)
+{
+	table->values[key] = NULL;
+}
+
+drm_private void *handle_table_lockup(struct handle_table *table, uint32_t key)
+{
+	return table->values[key];
+}
diff --git a/amdgpu/handle_table.h b/amdgpu/handle_table.h
new file mode 100644
index 00000000..9a170445
--- /dev/null
+++ b/amdgpu/handle_table.h
@@ -0,0 +1,40 @@
+/*
+ * 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_lockup(struct handle_table *table, uint32_t key);
+
+#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 3/6] amdgpu: add handle table implementation

[Michel Dänzer]

On 2018-08-02 04:04 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.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> 
> [...]
>  
> +drm_private int handle_table_insert(struct handle_table *table, uint32_t key,
> +				    void *value)
> +{
> +	if (key >= table->max_key) {
> +		uint32_t max_key = ALIGN(key, 512);

The idea here is to make the array page aligned, right? However, as is
it's only aligned to half the page size on 32-bit (and even worse on
systems where the minimum page size is e.g. 64K). Something like

	uint32_t max_key = ALIGN(key, sysconf(_SC_PAGESIZE) / sizeof(void*))

should do the trick.


Also, should handle_table_insert return an error if the table already
has a different non-NULL entry for the key?


> drm_private void handle_table_remove(struct handle_table *table, uint32_t key)
> +{
> +	table->values[key] = NULL;
> +}
> +
> +drm_private void *handle_table_lockup(struct handle_table *table, uint32_t key)
> +{
> +	return table->values[key];
> +}

Typo: Should be "lookup", not "lockup".

Maybe these should check that key < table->max_key, to prevent memory
outside of the table from getting corrupted by buggy callers.


-- 
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 3/6] amdgpu: add handle table implementation

[William Lewis]

Applies to patch #1 and patch #3, should lockup not be lookup?


On 08/02/2018 09:04 AM, 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.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   amdgpu/Makefile.sources |  4 +++-
>   amdgpu/handle_table.c   | 59 +++++++++++++++++++++++++++++++++++++++++++++++++
>   amdgpu/handle_table.h   | 40 +++++++++++++++++++++++++++++++++
>   3 files changed, 102 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..9acc44d5
> --- /dev/null
> +++ b/amdgpu/handle_table.c
> @@ -0,0 +1,59 @@
> +/*
> + * 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 "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 max_key = ALIGN(key, 512);
> +		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)
> +{
> +	table->values[key] = NULL;
> +}
> +
> +drm_private void *handle_table_lockup(struct handle_table *table, uint32_t key)
> +{
> +	return table->values[key];
> +}
> diff --git a/amdgpu/handle_table.h b/amdgpu/handle_table.h
> new file mode 100644
> index 00000000..9a170445
> --- /dev/null
> +++ b/amdgpu/handle_table.h
> @@ -0,0 +1,40 @@
> +/*
> + * 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_lockup(struct handle_table *table, uint32_t key);
> +
> +#endif /* _HANDLE_TABLE_H_ */

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

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

[Zhang, Jerry (Junwei)]

On 08/02/2018 10:04 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.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   amdgpu/Makefile.sources |  4 +++-
>   amdgpu/handle_table.c   | 59 +++++++++++++++++++++++++++++++++++++++++++++++++
>   amdgpu/handle_table.h   | 40 +++++++++++++++++++++++++++++++++
>   3 files changed, 102 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..9acc44d5
> --- /dev/null
> +++ b/amdgpu/handle_table.c
> @@ -0,0 +1,59 @@
> +/*
> + * 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 "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 max_key = ALIGN(key, 512);
> +		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)
> +{
> +	table->values[key] = NULL;
> +}

Not sure if there are too many handles for all devices, since we don't free the memory.
please confirm that.

Apart from that and typo, the series looks good for me.

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

When I try to re-implement the find bo API, the game has regression issue.
I have to verify it when it's ready.
Then send out the patch again.

Jerry

> +
> +drm_private void *handle_table_lockup(struct handle_table *table, uint32_t key)
> +{
> +	return table->values[key];
> +}
> diff --git a/amdgpu/handle_table.h b/amdgpu/handle_table.h
> new file mode 100644
> index 00000000..9a170445
> --- /dev/null
> +++ b/amdgpu/handle_table.h
> @@ -0,0 +1,40 @@
> +/*
> + * 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_lockup(struct handle_table *table, uint32_t key);
> +
> +#endif /* _HANDLE_TABLE_H_ */
>
_______________________________________________
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 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   |  2 --
 amdgpu/amdgpu_internal.h |  3 ++-
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
index 39228620..9a7d0b29 100644
--- a/amdgpu/amdgpu_bo.c
+++ b/amdgpu/amdgpu_bo.c
@@ -89,14 +89,17 @@ int amdgpu_bo_alloc(amdgpu_device_handle dev,
 	bo->handle = args.out.handle;
 
 	pthread_mutex_lock(&bo->dev->bo_table_mutex);
-	util_hash_table_set(bo->dev->bo_handles,
-			    (void*)(uintptr_t)bo->handle, bo);
+	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);
 
-	*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,
@@ -297,8 +300,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_lockup(&dev->bo_handles, shared_handle);
 		break;
 
 	case amdgpu_bo_handle_type_kms:
@@ -381,7 +383,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;
@@ -400,8 +402,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..6b07c064 100644
--- a/amdgpu/amdgpu_device.c
+++ b/amdgpu/amdgpu_device.c
@@ -123,7 +123,6 @@ static void amdgpu_device_free_internal(amdgpu_device_handle dev)
 	amdgpu_vamgr_deinit(&dev->vamgr_high_32);
 	amdgpu_vamgr_deinit(&dev->vamgr_high);
 	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 +229,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 5/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   | 14 --------------
 amdgpu/amdgpu_internal.h |  2 +-
 3 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/amdgpu/amdgpu_bo.c b/amdgpu/amdgpu_bo.c
index 9a7d0b29..14cccd9f 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,
@@ -216,12 +215,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,
@@ -295,8 +292,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_lockup(&dev->bo_flink_names, shared_handle);
 		break;
 
 	case amdgpu_bo_handle_type_dma_buf_fd:
@@ -364,8 +360,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:
@@ -404,10 +405,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 6b07c064..68ee19c9 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);
@@ -122,7 +111,6 @@ 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);
-	util_hash_table_destroy(dev->bo_flink_names);
 	pthread_mutex_destroy(&dev->bo_table_mutex);
 	free(dev->marketing_name);
 	free(dev);
@@ -227,8 +215,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 6/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

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

[Michel Dänzer]

On 2018-08-02 04:04 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>

Typo in the shortlog: should be "lookup" instead of "lockup".

Also, this patch should really be after patch 4.


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

This is a bit unfortunate, amdgpu_bo_handle_type_kms_noimport only just
landed this week for the 2.4.93 release, now it'll already be useless
noise... A bit more coordination would have been nice. :)


Anyway, with the typo in patch 3 fixed, patches 2 & 4-6 are

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


-- 
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 1/6] amdgpu: always add all BOs to lockup table

[Zhang, Jerry (Junwei)]

On 08/02/2018 10:55 PM, Michel Dänzer wrote:
> On 2018-08-02 04:04 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>
>
> Typo in the shortlog: should be "lookup" instead of "lockup".
>
> Also, this patch should really be after patch 4.
>
>
>> @@ -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;
>
> This is a bit unfortunate, amdgpu_bo_handle_type_kms_noimport only just
> landed this week for the 2.4.93 release, now it'll already be useless
> noise... A bit more coordination would have been nice. :)

BTW, there is a schedule for that release?

I mean if amdgpu_bo_handle_type_kms_noimport is merged in the release at last minute.
Generally we may wait for some verification or process for a release.
if the func has to go through a verification/process, that maybe not happen in a week.

Jerry

>
>
> Anyway, with the typo in patch 3 fixed, patches 2 & 4-6 are
>
> Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx