Currently we have a problem with this:
1. i915: create gem object
2. i915: export gem object to prime
3. radeon: import gem object
4. close prime fd
5. radeon: unref object
6. i915: unref object
i915 has an imported object reference in its file priv, that isn't
cleaned up properly until fd close. The reference gets added at step 2,
but at step 6 we don't have enough info to clean it up.
The solution is to take a reference on the dma-buf when we export it,
and drop the reference when the gem handle goes away.
So when we export a dma_buf from a gem object, we keep track of it
with the handle, we take a reference to the dma_buf. When we close
the handle (i.e. userspace is finished with the buffer), we drop
the reference to the dma_buf, and it gets collected.
This patch isn't meant to fix any other problem or bikesheds, and it doesn't
fix any races with other scenarios.
v1.1: move export symbol line back up.
v2: okay I had to do a bit more, as the first patch showed a leak
on one of my tests, that I found using the dma-buf debugfs support,
the problem case is exporting a buffer twice with the same handle,
we'd add another export handle for it unnecessarily, however
we now fail if we try to export the same object with a different gem handle,
however I'm not sure if that is a case I want to support, and I've
gotten the code to WARN_ON if we hit something like that.
v2.1: rebase this patch, write better commit msg.
Signed-off-by: Dave Airlie <airlied@xxxxxxxxxx>
---
drivers/gpu/drm/drm_gem.c | 2 +-
drivers/gpu/drm/drm_prime.c | 94 ++++++++++++++++++++++++++++++++-------------
include/drm/drmP.h | 3 +-
3 files changed, 71 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index af779ae..f0f7a86 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -209,7 +209,7 @@ drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct drm_file *filp)
obj->import_attach->dmabuf);
}
if (obj->export_dma_buf) {
- drm_prime_remove_imported_buf_handle(&filp->prime,
+ drm_prime_remove_exported_buf_handle(&filp->prime,
obj->export_dma_buf);
}
}
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 366910d..05ee250 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -157,6 +157,8 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = {
.vunmap = drm_gem_dmabuf_vunmap,
};
+static int drm_prime_add_exported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle);
+
/**
* DOC: PRIME Helpers
*
@@ -201,6 +203,8 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
struct drm_gem_object *obj;
void *buf;
int ret;
+ struct dma_buf *dmabuf;
+ uint32_t exp_handle;
obj = drm_gem_object_lookup(dev, file_priv, handle);
if (!obj)
@@ -209,41 +213,52 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
mutex_lock(&file_priv->prime.lock);
/* re-export the original imported object */
if (obj->import_attach) {
- get_dma_buf(obj->import_attach->dmabuf);
- *prime_fd = dma_buf_fd(obj->import_attach->dmabuf, flags);
- drm_gem_object_unreference_unlocked(obj);
- mutex_unlock(&file_priv->prime.lock);
- return 0;
+ dmabuf = obj->import_attach->dmabuf;
+ goto out_have_obj;
}
if (obj->export_dma_buf) {
- get_dma_buf(obj->export_dma_buf);
- *prime_fd = dma_buf_fd(obj->export_dma_buf, flags);
+ dmabuf = obj->export_dma_buf;
+ goto out_have_obj;
+ }
+
+ buf = dev->driver->gem_prime_export(dev, obj, flags);
+ if (IS_ERR(buf)) {
+ /* normally the created dma-buf takes ownership of the ref,
+ * but if that fails then drop the ref
+ */
drm_gem_object_unreference_unlocked(obj);
- } else {
- buf = dev->driver->gem_prime_export(dev, obj, flags);
- if (IS_ERR(buf)) {
- /* normally the created dma-buf takes ownership of the ref,
- * but if that fails then drop the ref
- */
- drm_gem_object_unreference_unlocked(obj);
- mutex_unlock(&file_priv->prime.lock);
- return PTR_ERR(buf);
- }
- obj->export_dma_buf = buf;
- *prime_fd = dma_buf_fd(buf, flags);
+ mutex_unlock(&file_priv->prime.lock);
+ return PTR_ERR(buf);
}
+ obj->export_dma_buf = buf;
+
/* if we've exported this buffer the cheat and add it to the import list
* so we get the correct handle back
*/
- ret = drm_prime_add_imported_buf_handle(&file_priv->prime,
- obj->export_dma_buf, handle);
+ ret = drm_prime_add_exported_buf_handle(&file_priv->prime,
+ obj->export_dma_buf, handle);
if (ret) {
drm_gem_object_unreference_unlocked(obj);
mutex_unlock(&file_priv->prime.lock);
return ret;
}
+ *prime_fd = dma_buf_fd(buf, flags);
+ mutex_unlock(&file_priv->prime.lock);
+ return 0;
+out_have_obj:
+ get_dma_buf(dmabuf);
+ ret = drm_prime_lookup_buf_handle(&file_priv->prime,
+ dmabuf, &exp_handle);
+ if (WARN_ON(ret == -ENOENT || exp_handle != handle)) {
+ dma_buf_put(dmabuf);
+ drm_gem_object_unreference_unlocked(obj);
+ mutex_unlock(&file_priv->prime.lock);
+ return -EINVAL;
+ }
+ *prime_fd = dma_buf_fd(dmabuf, flags);
+ drm_gem_object_unreference_unlocked(obj);
mutex_unlock(&file_priv->prime.lock);
return 0;
}
@@ -314,7 +329,7 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
mutex_lock(&file_priv->prime.lock);
- ret = drm_prime_lookup_imported_buf_handle(&file_priv->prime,
+ ret = drm_prime_lookup_buf_handle(&file_priv->prime,
dma_buf, handle);
if (!ret) {
ret = 0;
@@ -491,7 +506,7 @@ void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv)
}
EXPORT_SYMBOL(drm_prime_destroy_file_private);
-int drm_prime_add_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle)
+int drm_prime_add_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle)
{
struct drm_prime_member *member;
@@ -504,9 +519,21 @@ int drm_prime_add_imported_buf_handle(struct drm_prime_file_private *prime_fpriv
list_add(&member->entry, &prime_fpriv->head);
return 0;
}
+
+int drm_prime_add_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle)
+{
+ return drm_prime_add_buf_handle(prime_fpriv, dma_buf, handle);
+}
EXPORT_SYMBOL(drm_prime_add_imported_buf_handle);
-int drm_prime_lookup_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t *handle)
+static int drm_prime_add_exported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle)
+{
+ /* take a reference to the buf handle for this case */
+ get_dma_buf(dma_buf);
+ return drm_prime_add_buf_handle(prime_fpriv, dma_buf, handle);
+}
+
+int drm_prime_lookup_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t *handle)
{
struct drm_prime_member *member;
@@ -518,9 +545,9 @@ int drm_prime_lookup_imported_buf_handle(struct drm_prime_file_private *prime_fp
}
return -ENOENT;
}
-EXPORT_SYMBOL(drm_prime_lookup_imported_buf_handle);
+EXPORT_SYMBOL(drm_prime_lookup_buf_handle);
-void drm_prime_remove_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf)
+void drm_prime_remove_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf)
{
struct drm_prime_member *member, *safe;
@@ -533,4 +560,19 @@ void drm_prime_remove_imported_buf_handle(struct drm_prime_file_private *prime_f
}
mutex_unlock(&prime_fpriv->lock);
}
+
+void drm_prime_remove_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf)
+{
+ return drm_prime_remove_buf_handle(prime_fpriv, dma_buf);
+}
EXPORT_SYMBOL(drm_prime_remove_imported_buf_handle);
+
+void drm_prime_remove_exported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf)
+{
+ drm_prime_remove_buf_handle(prime_fpriv, dma_buf);
+ dma_buf_put(dma_buf);
+}
+EXPORT_SYMBOL(drm_prime_remove_exported_buf_handle);
+
+
+
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 2d94d74..0466808 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1594,8 +1594,9 @@ extern void drm_prime_gem_destroy(struct drm_gem_object *obj, struct sg_table *s
void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv);
void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv);
int drm_prime_add_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle);
-int drm_prime_lookup_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t *handle);
+int drm_prime_lookup_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t *handle);
void drm_prime_remove_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf);
+void drm_prime_remove_exported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf);
int drm_prime_add_dma_buf(struct drm_device *dev, struct drm_gem_object *obj);
int drm_prime_lookup_obj(struct drm_device *dev, struct dma_buf *buf,
--
1.8.2
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
