Walking a linear list to find a matching PRIME handle or flinked name
does not scale and becomes a major burden with just a few objects.

References: https://bugs.freedesktop.org/show_bug.cgi?id=94631
Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
---
 intel/intel_bufmgr_gem.c | 127 +++++++++++++++++++++++++++--------------------
 xf86drm.h                |   1 +
 xf86drmHash.c            |  11 ++++
 3 files changed, 85 insertions(+), 54 deletions(-)

diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index 0f212e9..6c9276c 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -39,6 +39,7 @@
 #endif
 
 #include <xf86drm.h>
+#include <xf86drmHash.h>
 #include <xf86atomic.h>
 #include <fcntl.h>
 #include <stdio.h>
@@ -130,7 +131,9 @@ typedef struct _drm_intel_bufmgr_gem {
 
        drmMMListHead managers;
 
-       drmMMListHead named;
+       HashTable *name_table;
+       HashTable *handle_table;
+
        drmMMListHead vma_cache;
        int vma_count, vma_open, vma_max;
 
@@ -175,7 +178,6 @@ struct _drm_intel_bo_gem {
          * List contains both flink named and prime fd'd objects
         */
        unsigned int global_name;
-       drmMMListHead name_list;
 
        /**
         * Index of the buffer within the validation list while preparing a
@@ -808,6 +810,10 @@ retry:
                if (!bo_gem)
                        return NULL;
 
+               /* drm_intel_gem_bo_free calls DRMLISTDEL() for an uninitialized
+                  list (vma_list), so better set the list head here */
+               DRMINITLISTHEAD(&bo_gem->vma_list);
+
                bo_gem->bo.size = bo_size;
 
                memclear(create);
@@ -816,23 +822,26 @@ retry:
                ret = drmIoctl(bufmgr_gem->fd,
                               DRM_IOCTL_I915_GEM_CREATE,
                               &create);
-               bo_gem->gem_handle = create.handle;
-               bo_gem->bo.handle = bo_gem->gem_handle;
                if (ret != 0) {
                        free(bo_gem);
                        return NULL;
                }
+
+               bo_gem->gem_handle = create.handle;
+               bo_gem->bo.handle = bo_gem->gem_handle;
                bo_gem->bo.bufmgr = bufmgr;
                bo_gem->bo.align = alignment;
 
+               if (drmHashInsert(bufmgr_gem->handle_table,
+                                 bo_gem->gem_handle, bo_gem)) {
+                       drm_intel_gem_bo_free(&bo_gem->bo);
+                       return NULL;
+               }
+
                bo_gem->tiling_mode = I915_TILING_NONE;
                bo_gem->swizzle_mode = I915_BIT_6_SWIZZLE_NONE;
                bo_gem->stride = 0;
 
-               /* drm_intel_gem_bo_free calls DRMLISTDEL() for an uninitialized
-                  list (vma_list), so better set the list head here */
-               DRMINITLISTHEAD(&bo_gem->name_list);
-               DRMINITLISTHEAD(&bo_gem->vma_list);
                if (drm_intel_gem_bo_set_tiling_internal(&bo_gem->bo,
                                                         tiling_mode,
                                                         stride)) {
@@ -956,6 +965,8 @@ drm_intel_gem_bo_alloc_userptr(drm_intel_bufmgr *bufmgr,
        if (!bo_gem)
                return NULL;
 
+       DRMINITLISTHEAD(&bo_gem->vma_list);
+
        bo_gem->bo.size = size;
 
        memclear(userptr);
@@ -985,8 +996,11 @@ drm_intel_gem_bo_alloc_userptr(drm_intel_bufmgr *bufmgr,
        bo_gem->swizzle_mode = I915_BIT_6_SWIZZLE_NONE;
        bo_gem->stride       = 0;
 
-       DRMINITLISTHEAD(&bo_gem->name_list);
-       DRMINITLISTHEAD(&bo_gem->vma_list);
+       if (drmHashInsert(bufmgr_gem->handle_table,
+                         bo_gem->gem_handle, bo_gem)) {
+               drm_intel_gem_bo_free(&bo_gem->bo);
+               return NULL;
+       }
 
        bo_gem->name = name;
        atomic_set(&bo_gem->refcount, 1);
@@ -1087,7 +1101,6 @@ drm_intel_bo_gem_create_from_name(drm_intel_bufmgr 
*bufmgr,
        int ret;
        struct drm_gem_open open_arg;
        struct drm_i915_gem_get_tiling get_tiling;
-       drmMMListHead *list;
 
        /* At the moment most applications only have a few named bo.
         * For instance, in a DRI client only the render buffers passed
@@ -1096,15 +1109,11 @@ drm_intel_bo_gem_create_from_name(drm_intel_bufmgr 
*bufmgr,
         * provides a sufficiently fast match.
         */
        pthread_mutex_lock(&bufmgr_gem->lock);
-       for (list = bufmgr_gem->named.next;
-            list != &bufmgr_gem->named;
-            list = list->next) {
-               bo_gem = DRMLISTENTRY(drm_intel_bo_gem, list, name_list);
-               if (bo_gem->global_name == handle) {
-                       drm_intel_gem_bo_reference(&bo_gem->bo);
-                       pthread_mutex_unlock(&bufmgr_gem->lock);
-                       return &bo_gem->bo;
-               }
+       bo_gem = drmHashLookupValue(bufmgr_gem->name_table, handle);
+       if (bo_gem) {
+               drm_intel_gem_bo_reference(&bo_gem->bo);
+               pthread_mutex_unlock(&bufmgr_gem->lock);
+               return &bo_gem->bo;
        }
 
        memclear(open_arg);
@@ -1122,15 +1131,11 @@ drm_intel_bo_gem_create_from_name(drm_intel_bufmgr 
*bufmgr,
          * object from the kernel before by looking through the list
          * again for a matching gem_handle
          */
-       for (list = bufmgr_gem->named.next;
-            list != &bufmgr_gem->named;
-            list = list->next) {
-               bo_gem = DRMLISTENTRY(drm_intel_bo_gem, list, name_list);
-               if (bo_gem->gem_handle == open_arg.handle) {
-                       drm_intel_gem_bo_reference(&bo_gem->bo);
-                       pthread_mutex_unlock(&bufmgr_gem->lock);
-                       return &bo_gem->bo;
-               }
+       bo_gem = drmHashLookupValue(bufmgr_gem->handle_table, open_arg.handle);
+       if (bo_gem) {
+               drm_intel_gem_bo_reference(&bo_gem->bo);
+               pthread_mutex_unlock(&bufmgr_gem->lock);
+               return &bo_gem->bo;
        }
 
        bo_gem = calloc(1, sizeof(*bo_gem));
@@ -1139,6 +1144,8 @@ drm_intel_bo_gem_create_from_name(drm_intel_bufmgr 
*bufmgr,
                return NULL;
        }
 
+       DRMINITLISTHEAD(&bo_gem->vma_list);
+
        bo_gem->bo.size = open_arg.size;
        bo_gem->bo.offset = 0;
        bo_gem->bo.offset64 = 0;
@@ -1153,14 +1160,23 @@ drm_intel_bo_gem_create_from_name(drm_intel_bufmgr 
*bufmgr,
        bo_gem->reusable = false;
        bo_gem->use_48b_address_range = false;
 
+       if (drmHashInsert(bufmgr_gem->handle_table,
+                         bo_gem->gem_handle, bo_gem) ||
+           drmHashInsert(bufmgr_gem->name_table,
+                         bo_gem->global_name, bo_gem)) {
+               pthread_mutex_unlock(&bufmgr_gem->lock);
+               drm_intel_gem_bo_unreference(&bo_gem->bo);
+               return NULL;
+       }
+
        memclear(get_tiling);
        get_tiling.handle = bo_gem->gem_handle;
        ret = drmIoctl(bufmgr_gem->fd,
                       DRM_IOCTL_I915_GEM_GET_TILING,
                       &get_tiling);
        if (ret != 0) {
-               drm_intel_gem_bo_unreference(&bo_gem->bo);
                pthread_mutex_unlock(&bufmgr_gem->lock);
+               drm_intel_gem_bo_unreference(&bo_gem->bo);
                return NULL;
        }
        bo_gem->tiling_mode = get_tiling.tiling_mode;
@@ -1168,8 +1184,6 @@ drm_intel_bo_gem_create_from_name(drm_intel_bufmgr 
*bufmgr,
        /* XXX stride is unknown */
        drm_intel_bo_gem_set_in_aperture_size(bufmgr_gem, bo_gem, 0);
 
-       DRMINITLISTHEAD(&bo_gem->vma_list);
-       DRMLISTADDTAIL(&bo_gem->name_list, &bufmgr_gem->named);
        pthread_mutex_unlock(&bufmgr_gem->lock);
        DBG("bo_create_from_handle: %d (%s)\n", handle, bo_gem->name);
 
@@ -1200,6 +1214,8 @@ drm_intel_gem_bo_free(drm_intel_bo *bo)
                bufmgr_gem->vma_count--;
        }
 
+       drmHashDelete(bufmgr_gem->handle_table, bo_gem->gem_handle);
+
        /* Close this object */
        memclear(close);
        close.handle = bo_gem->gem_handle;
@@ -1377,7 +1393,8 @@ drm_intel_gem_bo_unreference_final(drm_intel_bo *bo, 
time_t time)
                drm_intel_gem_bo_mark_mmaps_incoherent(bo);
        }
 
-       DRMLISTDEL(&bo_gem->name_list);
+       if (bo_gem->global_name)
+               drmHashDelete(bufmgr_gem->name_table, bo_gem->global_name);
 
        bucket = drm_intel_gem_bo_bucket_for_size(bufmgr_gem, bo->size);
        /* Put the buffer into our internal cache for reuse if we can. */
@@ -2610,7 +2627,6 @@ drm_intel_bo_gem_create_from_prime(drm_intel_bufmgr 
*bufmgr, int prime_fd, int s
        uint32_t handle;
        drm_intel_bo_gem *bo_gem;
        struct drm_i915_gem_get_tiling get_tiling;
-       drmMMListHead *list;
 
        pthread_mutex_lock(&bufmgr_gem->lock);
        ret = drmPrimeFDToHandle(bufmgr_gem->fd, prime_fd, &handle);
@@ -2625,15 +2641,11 @@ drm_intel_bo_gem_create_from_prime(drm_intel_bufmgr 
*bufmgr, int prime_fd, int s
         * for named buffers, we must not create two bo's pointing at the same
         * kernel object
         */
-       for (list = bufmgr_gem->named.next;
-            list != &bufmgr_gem->named;
-            list = list->next) {
-               bo_gem = DRMLISTENTRY(drm_intel_bo_gem, list, name_list);
-               if (bo_gem->gem_handle == handle) {
-                       drm_intel_gem_bo_reference(&bo_gem->bo);
-                       pthread_mutex_unlock(&bufmgr_gem->lock);
-                       return &bo_gem->bo;
-               }
+       bo_gem = drmHashLookupValue(bufmgr_gem->name_table, handle);
+       if (bo_gem) {
+               drm_intel_gem_bo_reference(&bo_gem->bo);
+               pthread_mutex_unlock(&bufmgr_gem->lock);
+               return &bo_gem->bo;
        }
 
        bo_gem = calloc(1, sizeof(*bo_gem));
@@ -2641,6 +2653,9 @@ drm_intel_bo_gem_create_from_prime(drm_intel_bufmgr 
*bufmgr, int prime_fd, int s
                pthread_mutex_unlock(&bufmgr_gem->lock);
                return NULL;
        }
+
+       DRMINITLISTHEAD(&bo_gem->vma_list);
+
        /* Determine size of bo.  The fd-to-handle ioctl really should
         * return the size, but it doesn't.  If we have kernel 3.12 or
         * later, we can lseek on the prime fd to get the size.  Older
@@ -2656,6 +2671,12 @@ drm_intel_bo_gem_create_from_prime(drm_intel_bufmgr 
*bufmgr, int prime_fd, int s
        bo_gem->bo.bufmgr = bufmgr;
 
        bo_gem->gem_handle = handle;
+       if (drmHashInsert(bufmgr_gem->handle_table,
+                         bo_gem->gem_handle, bo_gem)) {
+               drm_intel_gem_bo_free(&bo_gem->bo);
+               pthread_mutex_unlock(&bufmgr_gem->lock);
+               return NULL;
+       }
 
        atomic_set(&bo_gem->refcount, 1);
 
@@ -2667,8 +2688,6 @@ drm_intel_bo_gem_create_from_prime(drm_intel_bufmgr 
*bufmgr, int prime_fd, int s
        bo_gem->reusable = false;
        bo_gem->use_48b_address_range = false;
 
-       DRMINITLISTHEAD(&bo_gem->vma_list);
-       DRMLISTADDTAIL(&bo_gem->name_list, &bufmgr_gem->named);
        pthread_mutex_unlock(&bufmgr_gem->lock);
 
        memclear(get_tiling);
@@ -2695,11 +2714,6 @@ drm_intel_bo_gem_export_to_prime(drm_intel_bo *bo, int 
*prime_fd)
        drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bo->bufmgr;
        drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo;
 
-       pthread_mutex_lock(&bufmgr_gem->lock);
-        if (DRMLISTEMPTY(&bo_gem->name_list))
-                DRMLISTADDTAIL(&bo_gem->name_list, &bufmgr_gem->named);
-       pthread_mutex_unlock(&bufmgr_gem->lock);
-
        if (drmPrimeHandleToFD(bufmgr_gem->fd, bo_gem->gem_handle,
                               DRM_CLOEXEC, prime_fd) != 0)
                return -errno;
@@ -2725,16 +2739,20 @@ drm_intel_gem_bo_flink(drm_intel_bo *bo, uint32_t * 
name)
                pthread_mutex_lock(&bufmgr_gem->lock);
 
                ret = drmIoctl(bufmgr_gem->fd, DRM_IOCTL_GEM_FLINK, &flink);
-               if (ret != 0) {
+               if (ret) {
                        pthread_mutex_unlock(&bufmgr_gem->lock);
                        return -errno;
                }
 
+               if (drmHashInsert(bufmgr_gem->name_table,
+                                 bo_gem->global_name, bo_gem)) {
+                       pthread_mutex_unlock(&bufmgr_gem->lock);
+                       return -ENOMEM;
+               }
+
                bo_gem->global_name = flink.name;
                bo_gem->reusable = false;
 
-                if (DRMLISTEMPTY(&bo_gem->name_list))
-                        DRMLISTADDTAIL(&bo_gem->name_list, &bufmgr_gem->named);
                pthread_mutex_unlock(&bufmgr_gem->lock);
        }
 
@@ -3691,7 +3709,8 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size)
            drm_intel_gem_get_pipe_from_crtc_id;
        bufmgr_gem->bufmgr.bo_references = drm_intel_gem_bo_references;
 
-       DRMINITLISTHEAD(&bufmgr_gem->named);
+       bufmgr_gem->name_table = drmHashCreate();
+       bufmgr_gem->handle_table = drmHashCreate();
        init_cache_buckets(bufmgr_gem);
 
        DRMINITLISTHEAD(&bufmgr_gem->vma_cache);
diff --git a/xf86drm.h b/xf86drm.h
index 481d882..df1885e 100644
--- a/xf86drm.h
+++ b/xf86drm.h
@@ -695,6 +695,7 @@ extern void          drmFree(void *pt);
 extern void *drmHashCreate(void);
 extern int  drmHashDestroy(void *t);
 extern int  drmHashLookup(void *t, unsigned long key, void **value);
+extern void *drmHashLookupValue(void *t, unsigned long key);
 extern int  drmHashInsert(void *t, unsigned long key, void *value);
 extern int  drmHashDelete(void *t, unsigned long key);
 extern int  drmHashFirst(void *t, unsigned long *key, void **value);
diff --git a/xf86drmHash.c b/xf86drmHash.c
index f287e61..f689240 100644
--- a/xf86drmHash.c
+++ b/xf86drmHash.c
@@ -185,6 +185,17 @@ int drmHashLookup(void *t, unsigned long key, void **value)
     return 0;                  /* Found */
 }
 
+void *drmHashLookupValue(void *t, unsigned long key)
+{
+    HashBucketPtr bucket;
+
+    bucket = HashFind(t, key, NULL);
+    if (!bucket)
+           return NULL;
+
+    return bucket->value;
+}
+
 int drmHashInsert(void *t, unsigned long key, void *value)
 {
     HashTablePtr  table = (HashTablePtr)t;
-- 
2.9.3

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

Reply via email to