freedreno: add handle and name tracking

Due to the evil userspace buffer tracking we have to do, and hacks for
creating GEM buffer from fbdev/scanout, "evil-twin" fd_bo objects are
problematic.  So introduce hashtable tracking of bo's and dev's, to
avoid getting duplicate fd_bo ptrs for the same underlying gem object,
in particular when importing via flink name.

Signed-off-by: Rob Clark <robclark@freedesktop.org>

diff --git a/freedreno/freedreno_bo.c b/freedreno/freedreno_bo.c
index be386b4..f52ce5e 100644 (file)
--- a/freedreno/freedreno_bo.c
+++ b/freedreno/freedreno_bo.c
@@ -31,17 +31,46 @@
 
 #include <linux/fb.h>
 
+static pthread_mutex_t table_lock = PTHREAD_MUTEX_INITIALIZER;
+
+/* set buffer name, and add to table, call w/ table_lock held: */
+static void set_name(struct fd_bo *bo, uint32_t name)
+{
+       bo->name = name;
+       /* add ourself into the handle table: */
+       drmHashInsert(bo->dev->name_table, name, bo);
+}
+
+/* lookup a buffer, call w/ table_lock held: */
+static struct fd_bo * lookup_bo(void *tbl, uint32_t key)
+{
+       struct fd_bo *bo = NULL;
+       if (!drmHashLookup(tbl, key, (void **)&bo)) {
+               /* found, incr refcnt and return: */
+               bo = fd_bo_ref(bo);
+       }
+       return bo;
+}
+
+/* allocate a new buffer object, call w/ table_lock held */
 static struct fd_bo * bo_from_handle(struct fd_device *dev,
                uint32_t size, uint32_t handle)
 {
        unsigned i;
        struct fd_bo *bo = calloc(1, sizeof(*bo));
-       if (!bo)
+       if (!bo) {
+               struct drm_gem_close req = {
+                               .handle = handle,
+               };
+               drmIoctl(dev->fd, DRM_IOCTL_GEM_CLOSE, &req);
                return NULL;
-       bo->dev = dev;
+       }
+       bo->dev = fd_device_ref(dev);
        bo->size = size;
        bo->handle = handle;
        atomic_set(&bo->refcnt, 1);
+       /* add ourself into the handle table: */
+       drmHashInsert(dev->handle_table, handle, bo);
        for (i = 0; i < ARRAY_SIZE(bo->list); i++)
                list_inithead(&bo->list[i]);
        return bo;
@@ -95,7 +124,9 @@ struct fd_bo * fd_bo_new(struct fd_device *dev,
                return NULL;
        }
 
+       pthread_mutex_lock(&table_lock);
        bo = bo_from_handle(dev, size, req.handle);
+       pthread_mutex_unlock(&table_lock);
        if (!bo) {
                goto fail;
        }
@@ -127,6 +158,7 @@ struct fd_bo * fd_bo_from_fbdev(struct fd_pipe *pipe,
                return NULL;
        }
 
+       pthread_mutex_lock(&table_lock);
        bo = bo_from_handle(pipe->dev, size, req.handle);
 
        /* this is fugly, but works around a bug in the kernel..
@@ -152,9 +184,11 @@ struct fd_bo * fd_bo_from_fbdev(struct fd_pipe *pipe,
                bo->gpuaddr = req.gpuaddr;
                bo->map = fbmem;
        }
+       pthread_mutex_unlock(&table_lock);
 
        return bo;
 fail:
+       pthread_mutex_unlock(&table_lock);
        if (bo)
                fd_bo_del(bo);
        return NULL;
@@ -167,13 +201,28 @@ struct fd_bo * fd_bo_from_name(struct fd_device *dev, uint32_t name)
        };
        struct fd_bo *bo;
 
+       pthread_mutex_lock(&table_lock);
+
+       /* check name table first, to see if bo is already open: */
+       bo = lookup_bo(dev->name_table, name);
+       if (bo)
+               goto out_unlock;
+
        if (drmIoctl(dev->fd, DRM_IOCTL_GEM_OPEN, &req)) {
-               return NULL;
+               ERROR_MSG("gem-open failed: %s", strerror(errno));
+               goto out_unlock;
        }
 
+       bo = lookup_bo(dev->handle_table, req.handle);
+       if (bo)
+               goto out_unlock;
+
        bo = bo_from_handle(dev, req.size, req.handle);
        if (bo)
-               bo->name = name;
+               set_name(bo, name);
+
+out_unlock:
+       pthread_mutex_unlock(&table_lock);
 
        return bo;
 }
@@ -196,9 +245,13 @@ void fd_bo_del(struct fd_bo *bo)
                struct drm_gem_close req = {
                                .handle = bo->handle,
                };
+               pthread_mutex_lock(&table_lock);
+               drmHashDelete(bo->dev->handle_table, bo->handle);
                drmIoctl(bo->dev->fd, DRM_IOCTL_GEM_CLOSE, &req);
+               pthread_mutex_unlock(&table_lock);
        }
 
+       fd_device_del(bo->dev);
        free(bo);
 }
 
@@ -215,7 +268,9 @@ int fd_bo_get_name(struct fd_bo *bo, uint32_t *name)
                        return ret;
                }
 
-               bo->name = req.name;
+               pthread_mutex_lock(&table_lock);
+               set_name(bo, req.name);
+               pthread_mutex_unlock(&table_lock);
        }
 
        *name = bo->name;

diff --git a/freedreno/freedreno_device.c b/freedreno/freedreno_device.c
index 0a9ef9f..901d066 100644 (file)
--- a/freedreno/freedreno_device.c
+++ b/freedreno/freedreno_device.c
@@ -26,20 +26,77 @@
  *    Rob Clark <robclark@freedesktop.org>
  */
 
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
 #include "freedreno_drmif.h"
 #include "freedreno_priv.h"
 
-struct fd_device * fd_device_new(int fd)
+static pthread_mutex_t table_lock = PTHREAD_MUTEX_INITIALIZER;
+static void * dev_table;
+
+static struct fd_device * fd_device_new_impl(int fd)
 {
        struct fd_device *dev = calloc(1, sizeof(*dev));
        if (!dev)
                return NULL;
+       atomic_set(&dev->refcnt, 1);
        dev->fd = fd;
+       dev->handle_table = drmHashCreate();
+       dev->name_table = drmHashCreate();
+       return dev;
+}
+
+/* use inode for key into dev_table, rather than fd, to avoid getting
+ * confused by multiple-opens:
+ */
+static int devkey(int fd)
+{
+       struct stat s;
+       if (fstat(fd, &s)) {
+               ERROR_MSG("stat failed: %s", strerror(errno));
+               return -1;
+       }
+       return s.st_ino;
+}
+
+struct fd_device * fd_device_new(int fd)
+{
+       struct fd_device *dev = NULL;
+       int key = devkey(fd);
+
+       pthread_mutex_lock(&table_lock);
+
+       if (!dev_table)
+               dev_table = drmHashCreate();
+
+       if (drmHashLookup(dev_table, key, (void **)&dev)) {
+               dev = fd_device_new_impl(fd);
+               drmHashInsert(dev_table, key, dev);
+       } else {
+               dev = fd_device_ref(dev);
+       }
+
+       pthread_mutex_unlock(&table_lock);
+
+       return dev;
+}
+
+struct fd_device * fd_device_ref(struct fd_device *dev)
+{
+       atomic_inc(&dev->refcnt);
        return dev;
 }
 
 void fd_device_del(struct fd_device *dev)
 {
+       if (!atomic_dec_and_test(&dev->refcnt))
+               return;
+       pthread_mutex_lock(&table_lock);
+       drmHashDestroy(dev->handle_table);
+       drmHashDestroy(dev->name_table);
+       drmHashDelete(dev_table, devkey(dev->fd));
+       pthread_mutex_unlock(&table_lock);
        free(dev);
 }
-

diff --git a/freedreno/freedreno_drmif.h b/freedreno/freedreno_drmif.h
index ba99afd..54b900e 100644 (file)
--- a/freedreno/freedreno_drmif.h
+++ b/freedreno/freedreno_drmif.h
@@ -68,6 +68,7 @@ enum fd_param_id {
  */
 
 struct fd_device * fd_device_new(int fd);
+struct fd_device * fd_device_ref(struct fd_device *dev);
 void fd_device_del(struct fd_device *dev);
 
 

diff --git a/freedreno/freedreno_priv.h b/freedreno/freedreno_priv.h
index 0edca1d..433bd30 100644 (file)
--- a/freedreno/freedreno_priv.h
+++ b/freedreno/freedreno_priv.h
@@ -37,6 +37,7 @@
 #include <fcntl.h>
 #include <sys/ioctl.h>
 #include <sys/mman.h>
+#include <pthread.h>
 
 #include "xf86drm.h"
 #include "xf86atomic.h"
@@ -49,6 +50,18 @@
 
 struct fd_device {
        int fd;
+       atomic_t refcnt;
+
+       /* tables to keep track of bo's, to avoid "evil-twin" fd_bo objects:
+        *
+        *   handle_table: maps handle to fd_bo
+        *   name_table: maps flink name to fd_bo
+        *
+        * We end up needing two tables, because DRM_IOCTL_GEM_OPEN always
+        * returns a new handle.  So we need to figure out if the bo is already
+        * open in the process first, before calling gem-open.
+        */
+       void *handle_table, *name_table;
 };
 
 struct fd_pipe {