i965/drm: Drop code to search for an existing bufmgr.

This functionality was added by libdrm commit
743af59669386cb6e063fa4bd85f0a0b2da86295 (intel: make bufmgr_gem
shareable from different API) in an attempt to solve libva/mesa buffer
sharing problems.  Specifically, this was working around an issue hit
by Chromium, which used the same drm_fd for multiple APIs, and shared
buffers between them.

This code attempted to work around that issue by using the same bufmgr
for both libva and Mesa.  It worked because libdrm_intel was loaded by
both libraries.  However, now that Mesa has forked, we don't have a
common library, and this code cannot work.

The correct solution is to have each API open its own file descriptor
(and get a corresponding buffer manager), and then use PRIME export
and import to share BOs across those APIs.  Then the kernel can manage
those shared resources.  According to Chris, the kernel will pass back
the same handle for a prime FD if the lookup is from the same device FD.

We believe Chromium has since moved to this model.

In Mesa, there is already only one screen per FD, and so there will
only be one bufmgr per FD.  We don't need any of this code.

v2: Add a big warning comment written by Chris Wilson.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Jason Ekstrand <jason@jlekstrand.net>

diff --git a/src/mesa/drivers/dri/i965/intel_bufmgr_gem.c b/src/mesa/drivers/dri/i965/intel_bufmgr_gem.c
index eb6a5e742b1d8e864e11c554c1834fb4750e51e2..dc8129e0b25e039d61889132e83ab385151c2375 100644 (file)
--- a/src/mesa/drivers/dri/i965/intel_bufmgr_gem.c
+++ b/src/mesa/drivers/dri/i965/intel_bufmgr_gem.c
@@ -119,8 +119,6 @@ struct drm_bacon_gem_bo_bucket {
 };
 
 typedef struct _drm_bacon_bufmgr {
-       int refcount;
-
        int fd;
 
        int max_relocs;
@@ -137,8 +135,6 @@ typedef struct _drm_bacon_bufmgr {
        int num_buckets;
        time_t time;
 
-       struct list_head managers;
-
        struct hash_table *name_table;
        struct hash_table *handle_table;
 
@@ -1514,8 +1510,8 @@ drm_bacon_gem_bo_start_gtt_access(drm_bacon_bo *bo, int write_enable)
        }
 }
 
-static void
-drm_bacon_bufmgr_gem_destroy(drm_bacon_bufmgr *bufmgr)
+void
+drm_bacon_bufmgr_destroy(drm_bacon_bufmgr *bufmgr)
 {
        free(bufmgr->exec2_objects);
        free(bufmgr->exec_bos);
@@ -2369,38 +2365,6 @@ drm_bacon_reg_read(drm_bacon_bufmgr *bufmgr,
        return ret;
 }
 
-static pthread_mutex_t bufmgr_list_mutex = PTHREAD_MUTEX_INITIALIZER;
-static struct list_head bufmgr_list = { &bufmgr_list, &bufmgr_list };
-
-static drm_bacon_bufmgr *
-drm_bacon_bufmgr_gem_find(int fd)
-{
-       list_for_each_entry(drm_bacon_bufmgr,
-                            bufmgr, &bufmgr_list, managers) {
-               if (bufmgr->fd == fd) {
-                       p_atomic_inc(&bufmgr->refcount);
-                       return bufmgr;
-               }
-       }
-
-       return NULL;
-}
-
-void
-drm_bacon_bufmgr_destroy(drm_bacon_bufmgr *bufmgr)
-{
-       if (atomic_add_unless(&bufmgr->refcount, -1, 1)) {
-               pthread_mutex_lock(&bufmgr_list_mutex);
-
-               if (p_atomic_dec_zero(&bufmgr->refcount)) {
-                       list_del(&bufmgr->managers);
-                       drm_bacon_bufmgr_gem_destroy(bufmgr);
-               }
-
-               pthread_mutex_unlock(&bufmgr_list_mutex);
-       }
-}
-
 void *drm_bacon_gem_bo_map__gtt(drm_bacon_bo *bo)
 {
        drm_bacon_bufmgr *bufmgr = bo->bufmgr;
@@ -2538,23 +2502,24 @@ drm_bacon_bufmgr_gem_init(struct gen_device_info *devinfo,
        drm_bacon_bufmgr *bufmgr;
        struct drm_i915_gem_get_aperture aperture;
 
-       pthread_mutex_lock(&bufmgr_list_mutex);
-
-       bufmgr = drm_bacon_bufmgr_gem_find(fd);
-       if (bufmgr)
-               goto exit;
-
        bufmgr = calloc(1, sizeof(*bufmgr));
        if (bufmgr == NULL)
-               goto exit;
+               return NULL;
 
+       /* Handles to buffer objects belong to the device fd and are not
+        * reference counted by the kernel.  If the same fd is used by
+        * multiple parties (threads sharing the same screen bufmgr, or
+        * even worse the same device fd passed to multiple libraries)
+        * ownership of those handles is shared by those independent parties.
+        *
+        * Don't do this! Ensure that each library/bufmgr has its own device
+        * fd so that its namespace does not clash with another.
+        */
        bufmgr->fd = fd;
-       p_atomic_set(&bufmgr->refcount, 1);
 
        if (pthread_mutex_init(&bufmgr->lock, NULL) != 0) {
                free(bufmgr);
-               bufmgr = NULL;
-               goto exit;
+               return NULL;
        }
 
        memclear(aperture);
@@ -2576,15 +2541,10 @@ drm_bacon_bufmgr_gem_init(struct gen_device_info *devinfo,
        list_inithead(&bufmgr->vma_cache);
        bufmgr->vma_max = -1; /* unlimited by default */
 
-       list_add(&bufmgr->managers, &bufmgr_list);
-
        bufmgr->name_table =
                _mesa_hash_table_create(NULL, key_hash_uint, key_uint_equal);
        bufmgr->handle_table =
                _mesa_hash_table_create(NULL, key_hash_uint, key_uint_equal);
 
-exit:
-       pthread_mutex_unlock(&bufmgr_list_mutex);
-
        return bufmgr;
 }