[libdrm,6/9] amdgpu: Re-use an existing amdgpu_device when possible

Submitted by Michel Dänzer on June 24, 2019, 4:54 p.m.

Details

Message ID 20190624165406.13682-7-michel@daenzer.net
State Rejected
Headers show
Series "amdgpu:" ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Michel Dänzer June 24, 2019, 4:54 p.m.
From: Michel Dänzer <michel.daenzer@amd.com>

It's possible if amdgpu_device's user_fd references the same file
description as the fd passed to amdgpu_device_initialize.

Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 amdgpu/amdgpu_device.c   | 76 +++++++++++++++++++++++++++++-----------
 amdgpu/amdgpu_internal.h |  3 +-
 2 files changed, 58 insertions(+), 21 deletions(-)

Patch hide | download patch | download mbox

diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c
index abf5f942..8d9a85c2 100644
--- a/amdgpu/amdgpu_device.c
+++ b/amdgpu/amdgpu_device.c
@@ -28,6 +28,11 @@ 
  *
  */
 
+#ifdef __linux__
+#include <linux/kcmp.h>
+#include <sys/syscall.h>
+#endif
+
 #include <sys/stat.h>
 #include <errno.h>
 #include <string.h>
@@ -44,7 +49,7 @@ 
 #define PTR_TO_UINT(x) ((unsigned)((intptr_t)(x)))
 
 static pthread_mutex_t dev_mutex = PTHREAD_MUTEX_INITIALIZER;
-static struct amdgpu_core_device *dev_list;
+static amdgpu_device_handle dev_list;
 
 static int fd_compare(int fd1, int fd2)
 {
@@ -67,12 +72,6 @@  static int fd_compare(int fd1, int fd2)
 
 static void amdgpu_device_free(struct amdgpu_core_device *dev)
 {
-	struct amdgpu_core_device **node = &dev_list;
-
-	while (*node != dev && (*node)->next)
-		node = &(*node)->next;
-	*node = (*node)->next;
-
 	close(dev->fd);
 
 	amdgpu_vamgr_deinit(&dev->vamgr_32);
@@ -86,20 +85,35 @@  static void amdgpu_device_free(struct amdgpu_core_device *dev)
 	free(dev);
 }
 
+static bool same_file_description(int fd1, int fd2)
+{
+#ifdef __linux__
+	pid_t pid = getpid();
+
+	return syscall(SYS_kcmp, pid, pid, KCMP_FILE, fd1, fd2) == 0;
+#endif
+
+	/* NOTE: This is never true at this point, since we always duplicate the
+	 * fd passed to amdgpu_device_initialize
+	 */
+	return fd1 == fd2;
+}
+
 static int amdgpu_device_init(amdgpu_device_handle user_dev)
 {
+	struct amdgpu_device *dev_iter;
 	struct amdgpu_core_device *dev;
 	drmVersionPtr version;
 	uint64_t start, max;
 	int r;
 
-	for (dev = dev_list; dev; dev = dev->next)
-		if (fd_compare(dev->fd, user_dev->user_fd) == 0)
+	for (dev_iter = dev_list; dev_iter; dev_iter = dev_iter->next)
+		if (fd_compare(dev_iter->core->fd, user_dev->user_fd) == 0)
 			break;
 
-	if (dev) {
-		atomic_inc(&dev->refcount);
-		user_dev->core = dev;
+	if (dev_iter) {
+		atomic_inc(&dev_iter->core->refcount);
+		user_dev->core = dev_iter->core;
 		return 0;
 	}
 
@@ -115,9 +129,6 @@  static int amdgpu_device_init(amdgpu_device_handle user_dev)
 	dev->fd = user_dev->user_fd;
 	user_dev->core = dev;
 
-	dev->next = dev_list;
-	dev_list = dev;
-
 	version = drmGetVersion(dev->fd);
 	if (version->version_major != 3) {
 		fprintf(stderr, "%s: DRM version is %d.%d.%d but this driver is "
@@ -184,6 +195,17 @@  drm_public int amdgpu_device_initialize(int fd,
 
 	*device_handle = NULL;
 
+	pthread_mutex_lock(&dev_mutex);
+
+	for (user_dev = dev_list; user_dev; user_dev = user_dev->next) {
+		if (same_file_description(user_dev->user_fd, fd)) {
+			atomic_inc(&user_dev->refcount);
+			goto out;
+		}
+	}
+
+	pthread_mutex_unlock(&dev_mutex);
+
 	user_dev = calloc(1, sizeof(struct amdgpu_device));
 	if (!user_dev) {
 		fprintf(stderr, "%s: calloc failed\n", __func__);
@@ -211,6 +233,11 @@  drm_public int amdgpu_device_initialize(int fd,
 		goto cleanup;
 	}
 
+	atomic_set(&user_dev->refcount, 1);
+	user_dev->next = dev_list;
+	dev_list = user_dev;
+
+out:
 	*major_version = user_dev->core->major_version;
 	*minor_version = user_dev->core->minor_version;
 	*device_handle = user_dev;
@@ -234,14 +261,23 @@  drm_public int amdgpu_device_deinitialize(amdgpu_device_handle user_dev)
 
 	pthread_mutex_lock(&dev_mutex);
 
-	if (user_dev->user_fd != dev->fd)
-		close(user_dev->user_fd);
+	if (update_references(&user_dev->refcount, NULL)) {
+		struct amdgpu_device **node = &dev_list;
 
-	if (update_references(&dev->refcount, NULL))
-		amdgpu_device_free(dev);
+		while (*node != user_dev && (*node)->next)
+			node = &(*node)->next;
+		*node = (*node)->next;
+
+		if (user_dev->user_fd != dev->fd)
+			close(user_dev->user_fd);
+
+		if (update_references(&dev->refcount, NULL))
+			amdgpu_device_free(dev);
+
+		free(user_dev);
+	}
 
 	pthread_mutex_unlock(&dev_mutex);
-	free(user_dev);
 	return 0;
 }
 
diff --git a/amdgpu/amdgpu_internal.h b/amdgpu/amdgpu_internal.h
index a08a4ae8..686d50ec 100644
--- a/amdgpu/amdgpu_internal.h
+++ b/amdgpu/amdgpu_internal.h
@@ -70,7 +70,6 @@  struct amdgpu_core_device {
 	unsigned major_version;
 	unsigned minor_version;
 
-	struct amdgpu_core_device *next;
 	char *marketing_name;
 	/** List of buffer handles. Protected by bo_table_mutex. */
 	struct handle_table bo_handles;
@@ -91,8 +90,10 @@  struct amdgpu_core_device {
 };
 
 struct amdgpu_device {
+	atomic_t refcount;
 	int user_fd;
 	struct amdgpu_core_device *core;
+	struct amdgpu_device *next;
 };
 
 struct amdgpu_core_bo {