From 80761a42e0e6a79728b5f9b3d6df0616d64fe45e Mon Sep 17 00:00:00 2001 From: Kenneth Graunke Date: Fri, 31 Mar 2017 09:44:54 -0700 Subject: [PATCH] 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 Acked-by: Jason Ekstrand --- src/mesa/drivers/dri/i965/intel_bufmgr_gem.c | 66 ++++---------------- 1 file changed, 13 insertions(+), 53 deletions(-) diff --git a/src/mesa/drivers/dri/i965/intel_bufmgr_gem.c b/src/mesa/drivers/dri/i965/intel_bufmgr_gem.c index eb6a5e742b1..dc8129e0b25 100644 --- 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; } -- 2.30.2