Mesa (staging/20.0): iris: fix export of GEM handles
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Tue Jun 9 18:01:50 UTC 2020
Module: Mesa
Branch: staging/20.0
Commit: 05ee65801c04bfa80b267375ab4b6823c18035b3
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=05ee65801c04bfa80b267375ab4b6823c18035b3
Author: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
Date: Sat May 2 16:46:47 2020 +0300
iris: fix export of GEM handles
We reuse DRM file descriptors internally. Therefore when we export a
GEM handle we must do so in the file descriptor used externally.
This change also fixes a file descriptor leak of the FD given at
screen creation.
v2: Don't bother checking fd equals, they're always different
Fix dmabuf leak
Fix GEM handle leaks by tracking exported handles
v3: Check os_same_file_description error (Michel)
Don't create multiple exports for a given GEM table
v4: Add WARN_ONCE (Ken)
Rename external_fd to winsys_fd
v5: Remove export lock in favor of bufmgr's
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/2882
Fixes: 7557f1605968 ("iris: share buffer managers accross screens")
Tested-by: Eric Engestrom <eric at engestrom.ch>
Tested-by: Tapani Pälli <tapani.palli at intel.com>
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4861>
(cherry picked from commit aba3aed96e4394a213e188f2f71ef045803a27c5)
---
.pick_status.json | 2 +-
src/gallium/drivers/iris/iris_bufmgr.c | 106 ++++++++++++++++++++++++++++++-
src/gallium/drivers/iris/iris_bufmgr.h | 16 +++++
src/gallium/drivers/iris/iris_resource.c | 33 ++++++++--
src/gallium/drivers/iris/iris_screen.c | 2 +
src/gallium/drivers/iris/iris_screen.h | 8 ++-
6 files changed, 156 insertions(+), 11 deletions(-)
diff --git a/.pick_status.json b/.pick_status.json
index 7cbf41dfeae..14baf637647 100644
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -985,7 +985,7 @@
"description": "iris: fix export of GEM handles",
"nominated": true,
"nomination_type": 1,
- "resolution": 0,
+ "resolution": 1,
"master_sha": null,
"because_sha": "7557f1605968c39d680545d5b8457d17eea3b922"
},
diff --git a/src/gallium/drivers/iris/iris_bufmgr.c b/src/gallium/drivers/iris/iris_bufmgr.c
index 825553b3e31..3ed2204d437 100644
--- a/src/gallium/drivers/iris/iris_bufmgr.c
+++ b/src/gallium/drivers/iris/iris_bufmgr.c
@@ -63,6 +63,7 @@
#include "util/macros.h"
#include "util/hash_table.h"
#include "util/list.h"
+#include "util/os_file.h"
#include "util/u_dynarray.h"
#include "util/vma.h"
#include "iris_bufmgr.h"
@@ -91,6 +92,17 @@
#define PAGE_SIZE 4096
+#define WARN_ONCE(cond, fmt...) do { \
+ if (unlikely(cond)) { \
+ static bool _warned = false; \
+ if (!_warned) { \
+ fprintf(stderr, "WARNING: "); \
+ fprintf(stderr, fmt); \
+ _warned = true; \
+ } \
+ } \
+} while (0)
+
#define FILE_DEBUG_FLAG DEBUG_BUFMGR
static inline int
@@ -126,6 +138,16 @@ struct bo_cache_bucket {
uint64_t size;
};
+struct bo_export {
+ /** File descriptor associated with a handle export. */
+ int drm_fd;
+
+ /** GEM handle in drm_fd */
+ uint32_t gem_handle;
+
+ struct list_head link;
+};
+
struct iris_bufmgr {
/**
* List into the list of bufmgr.
@@ -352,9 +374,13 @@ static struct iris_bo *
bo_calloc(void)
{
struct iris_bo *bo = calloc(1, sizeof(*bo));
- if (bo) {
- bo->hash = _mesa_hash_pointer(bo);
- }
+ if (!bo)
+ return NULL;
+
+ list_inithead(&bo->exports);
+
+ bo->hash = _mesa_hash_pointer(bo);
+
return bo;
}
@@ -711,6 +737,7 @@ iris_bo_gem_create_from_name(struct iris_bufmgr *bufmgr,
bo->tiling_mode = get_tiling.tiling_mode;
bo->swizzle_mode = get_tiling.swizzle_mode;
+
/* XXX stride is unknown */
DBG("bo_create_from_handle: %d (%s)\n", handle, bo->name);
@@ -739,6 +766,16 @@ bo_close(struct iris_bo *bo)
entry = _mesa_hash_table_search(bufmgr->handle_table, &bo->gem_handle);
_mesa_hash_table_remove(bufmgr->handle_table, entry);
+
+ list_for_each_entry_safe(struct bo_export, export, &bo->exports, link) {
+ struct drm_gem_close close = { .handle = export->gem_handle };
+ gen_ioctl(export->drm_fd, DRM_IOCTL_GEM_CLOSE, &close);
+
+ list_del(&export->link);
+ free(export);
+ }
+ } else {
+ assert(list_is_empty(&bo->exports));
}
/* Close this object */
@@ -1449,6 +1486,69 @@ iris_bo_flink(struct iris_bo *bo, uint32_t *name)
return 0;
}
+int
+iris_bo_export_gem_handle_for_device(struct iris_bo *bo, int drm_fd,
+ uint32_t *out_handle)
+{
+ /* Only add the new GEM handle to the list of export if it belongs to a
+ * different GEM device. Otherwise we might close the same buffer multiple
+ * times.
+ */
+ struct iris_bufmgr *bufmgr = bo->bufmgr;
+ int ret = os_same_file_description(drm_fd, bufmgr->fd);
+ WARN_ONCE(ret < 0,
+ "Kernel has no file descriptor comparison support: %s\n",
+ strerror(errno));
+ if (ret == 0) {
+ *out_handle = iris_bo_export_gem_handle(bo);
+ return 0;
+ }
+
+ struct bo_export *export = calloc(1, sizeof(*export));
+ if (!export)
+ return -ENOMEM;
+
+ export->drm_fd = drm_fd;
+
+ int dmabuf_fd = -1;
+ int err = iris_bo_export_dmabuf(bo, &dmabuf_fd);
+ if (err) {
+ free(export);
+ return err;
+ }
+
+ mtx_lock(&bufmgr->lock);
+ err = drmPrimeFDToHandle(drm_fd, dmabuf_fd, &export->gem_handle);
+ close(dmabuf_fd);
+ if (err) {
+ mtx_unlock(&bufmgr->lock);
+ free(export);
+ return err;
+ }
+
+ bool found = false;
+ list_for_each_entry(struct bo_export, iter, &bo->exports, link) {
+ if (iter->drm_fd != drm_fd)
+ continue;
+ /* Here we assume that for a given DRM fd, we'll always get back the
+ * same GEM handle for a given buffer.
+ */
+ assert(iter->gem_handle == export->gem_handle);
+ free(export);
+ export = iter;
+ found = true;
+ break;
+ }
+ if (!found)
+ list_addtail(&export->link, &bo->exports);
+
+ mtx_unlock(&bufmgr->lock);
+
+ *out_handle = export->gem_handle;
+
+ return 0;
+}
+
static void
add_bucket(struct iris_bufmgr *bufmgr, int size)
{
diff --git a/src/gallium/drivers/iris/iris_bufmgr.h b/src/gallium/drivers/iris/iris_bufmgr.h
index 4aa19350f11..831f7adceb9 100644
--- a/src/gallium/drivers/iris/iris_bufmgr.h
+++ b/src/gallium/drivers/iris/iris_bufmgr.h
@@ -28,6 +28,7 @@
#include <stdint.h>
#include <stdio.h>
#include <sys/types.h>
+#include "c11/threads.h"
#include "util/macros.h"
#include "util/u_atomic.h"
#include "util/list.h"
@@ -172,6 +173,9 @@ struct iris_bo {
/** BO cache list */
struct list_head head;
+ /** List of GEM handle exports of this buffer (bo_export) */
+ struct list_head exports;
+
/**
* Boolean of whether this buffer can be re-used
*/
@@ -365,6 +369,18 @@ int iris_bo_export_dmabuf(struct iris_bo *bo, int *prime_fd);
struct iris_bo *iris_bo_import_dmabuf(struct iris_bufmgr *bufmgr, int prime_fd,
uint32_t tiling, uint32_t stride);
+/**
+ * Exports a bo as a GEM handle into a given DRM file descriptor
+ * \param bo Buffer to export
+ * \param drm_fd File descriptor where the new handle is created
+ * \param out_handle Pointer to store the new handle
+ *
+ * Returns 0 if the buffer was successfully exported, a non zero error code
+ * otherwise.
+ */
+int iris_bo_export_gem_handle_for_device(struct iris_bo *bo, int drm_fd,
+ uint32_t *out_handle);
+
uint32_t iris_bo_export_gem_handle(struct iris_bo *bo);
int iris_reg_read(struct iris_bufmgr *bufmgr, uint32_t offset, uint64_t *out);
diff --git a/src/gallium/drivers/iris/iris_resource.c b/src/gallium/drivers/iris/iris_resource.c
index ecc7f9e8872..e6c049c1fae 100644
--- a/src/gallium/drivers/iris/iris_resource.c
+++ b/src/gallium/drivers/iris/iris_resource.c
@@ -1112,7 +1112,7 @@ iris_resource_disable_aux_on_first_query(struct pipe_resource *resource,
}
static bool
-iris_resource_get_param(struct pipe_screen *screen,
+iris_resource_get_param(struct pipe_screen *pscreen,
struct pipe_context *context,
struct pipe_resource *resource,
unsigned plane,
@@ -1121,6 +1121,7 @@ iris_resource_get_param(struct pipe_screen *screen,
unsigned handle_usage,
uint64_t *value)
{
+ struct iris_screen *screen = (struct iris_screen *)pscreen;
struct iris_resource *res = (struct iris_resource *)resource;
bool mod_with_aux =
res->mod_info && res->mod_info->aux_usage != ISL_AUX_USAGE_NONE;
@@ -1129,7 +1130,7 @@ iris_resource_get_param(struct pipe_screen *screen,
unsigned handle;
if (iris_resource_unfinished_aux_import(res))
- iris_resource_finish_aux_import(screen, res);
+ iris_resource_finish_aux_import(pscreen, res);
struct iris_bo *bo = wants_aux ? res->aux.bo : res->bo;
@@ -1161,9 +1162,19 @@ iris_resource_get_param(struct pipe_screen *screen,
if (result)
*value = handle;
return result;
- case PIPE_RESOURCE_PARAM_HANDLE_TYPE_KMS:
- *value = iris_bo_export_gem_handle(bo);
+ case PIPE_RESOURCE_PARAM_HANDLE_TYPE_KMS: {
+ /* Because we share the same drm file across multiple iris_screen, when
+ * we export a GEM handle we must make sure it is valid in the DRM file
+ * descriptor the caller is using (this is the FD given at screen
+ * creation).
+ */
+ uint32_t handle;
+ if (iris_bo_export_gem_handle_for_device(bo, screen->winsys_fd, &handle))
+ return false;
+ *value = handle;
return true;
+ }
+
case PIPE_RESOURCE_PARAM_HANDLE_TYPE_FD:
result = iris_bo_export_dmabuf(bo, (int *) &handle) == 0;
if (result)
@@ -1181,6 +1192,7 @@ iris_resource_get_handle(struct pipe_screen *pscreen,
struct winsys_handle *whandle,
unsigned usage)
{
+ struct iris_screen *screen = (struct iris_screen *) pscreen;
struct iris_resource *res = (struct iris_resource *)resource;
bool mod_with_aux =
res->mod_info && res->mod_info->aux_usage != ISL_AUX_USAGE_NONE;
@@ -1218,9 +1230,18 @@ iris_resource_get_handle(struct pipe_screen *pscreen,
switch (whandle->type) {
case WINSYS_HANDLE_TYPE_SHARED:
return iris_bo_flink(bo, &whandle->handle) == 0;
- case WINSYS_HANDLE_TYPE_KMS:
- whandle->handle = iris_bo_export_gem_handle(bo);
+ case WINSYS_HANDLE_TYPE_KMS: {
+ /* Because we share the same drm file across multiple iris_screen, when
+ * we export a GEM handle we must make sure it is valid in the DRM file
+ * descriptor the caller is using (this is the FD given at screen
+ * creation).
+ */
+ uint32_t handle;
+ if (iris_bo_export_gem_handle_for_device(bo, screen->winsys_fd, &handle))
+ return false;
+ whandle->handle = handle;
return true;
+ }
case WINSYS_HANDLE_TYPE_FD:
return iris_bo_export_dmabuf(bo, (int *) &whandle->handle) == 0;
}
diff --git a/src/gallium/drivers/iris/iris_screen.c b/src/gallium/drivers/iris/iris_screen.c
index 6ba21451a62..78d49848f12 100644
--- a/src/gallium/drivers/iris/iris_screen.c
+++ b/src/gallium/drivers/iris/iris_screen.c
@@ -524,6 +524,7 @@ iris_screen_destroy(struct iris_screen *screen)
u_transfer_helper_destroy(screen->base.transfer_helper);
iris_bufmgr_unref(screen->bufmgr);
disk_cache_destroy(screen->disk_cache);
+ close(screen->winsys_fd);
ralloc_free(screen);
}
@@ -674,6 +675,7 @@ iris_screen_create(int fd, const struct pipe_screen_config *config)
return NULL;
screen->fd = iris_bufmgr_get_fd(screen->bufmgr);
+ screen->winsys_fd = fd;
screen->aperture_bytes = get_aperture_size(fd);
diff --git a/src/gallium/drivers/iris/iris_screen.h b/src/gallium/drivers/iris/iris_screen.h
index 58864f7625d..4144d48d27d 100644
--- a/src/gallium/drivers/iris/iris_screen.h
+++ b/src/gallium/drivers/iris/iris_screen.h
@@ -51,9 +51,15 @@ struct iris_screen {
/** Global slab allocator for iris_transfer_map objects */
struct slab_parent_pool transfer_pool;
- /** drm device file descriptor, on shared with bufmgr, do not close. */
+ /** drm device file descriptor, shared with bufmgr, do not close. */
int fd;
+ /**
+ * drm device file descriptor to used for window system integration, owned
+ * by iris_screen, can be a different DRM instance than fd.
+ */
+ int winsys_fd;
+
/** PCI ID for our GPU device */
int pci_id;
More information about the mesa-commit
mailing list