diff mbox

drm/prime: fix lifetime issue with storing dma_buf on import list for exports

Message ID 1355808151-31462-1-git-send-email-airlied@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dave Airlie Dec. 18, 2012, 5:22 a.m. UTC 
From: Dave Airlie <airlied@redhat.com>

So when we export an dma buf we store it on the import list for that file
priv, so we can trap later imports of a buffer we exported from ourselves.

However there is nothing to clean that pointer up if the dma-buf gets
released internally, since we don't hold a reference to it. Holding
a reference to it could cause circular references so is probably best
avoided.

However if we store the gem objects in the file priv, and when we clean
up the gem object handles we clean up these references as well, then
it should be safe. We can then search for dma bufs referenced from the
gem objects themselves.

This seems to point out that obj->export_dma_buf and obj->import_attach
may need a bit of a lock safety review.

This fixes some intel/radeon tests from ages ago that started to fail
along with appearing to fix prime on my GM45/r600 laptop in F18.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/drm_gem.c   |  9 +--------
 drivers/gpu/drm/drm_prime.c | 48 +++++++++++++++++++++++++++++++++++----------
 include/drm/drmP.h          |  7 ++++---
 3 files changed, 43 insertions(+), 21 deletions(-)
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 24efae4..4252f61 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -204,14 +204,7 @@  EXPORT_SYMBOL(drm_gem_object_alloc);
 static void
 drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct drm_file *filp)
 {
-	if (obj->import_attach) {
-		drm_prime_remove_imported_buf_handle(&filp->prime,
-				obj->import_attach->dmabuf);
-	}
-	if (obj->export_dma_buf) {
-		drm_prime_remove_imported_buf_handle(&filp->prime,
-				obj->export_dma_buf);
-	}
+	drm_prime_remove_imported_buf_handle(&filp->prime, obj);
 }
 
 /**
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 7f12573..1f31df1 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -56,10 +56,14 @@ 
  * from the dma-buf private.
  */
 
+#define PRIME_IMPORT 1
+#define PRIME_EXPORT 2
+
 struct drm_prime_member {
 	struct list_head entry;
-	struct dma_buf *dma_buf;
+	struct drm_gem_object *obj;
 	uint32_t handle;
+	int type;
 };
 
 int drm_gem_prime_handle_to_fd(struct drm_device *dev,
@@ -104,8 +108,8 @@  int drm_gem_prime_handle_to_fd(struct drm_device *dev,
 	/* 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,
+						handle);
 	if (ret) {
 		drm_gem_object_unreference_unlocked(obj);
 		mutex_unlock(&file_priv->prime.lock);
@@ -149,8 +153,8 @@  int drm_gem_prime_fd_to_handle(struct drm_device *dev,
 	if (ret)
 		goto out_put;
 
-	ret = drm_prime_add_imported_buf_handle(&file_priv->prime,
-			dma_buf, *handle);
+	ret = drm_prime_add_imported_buf_handle(&file_priv->prime, obj,
+						*handle);
 	if (ret)
 		goto fail;
 
@@ -307,7 +311,9 @@  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)
+static int drm_prime_add_buf_handle(struct drm_prime_file_private *prime_fpriv,
+				    struct drm_gem_object *obj,
+				    uint32_t handle, int type)
 {
 	struct drm_prime_member *member;
 
@@ -315,11 +321,23 @@  int drm_prime_add_imported_buf_handle(struct drm_prime_file_private *prime_fpriv
 	if (!member)
 		return -ENOMEM;
 
-	member->dma_buf = dma_buf;
+	member->obj = obj;
 	member->handle = handle;
+	member->type = type;
 	list_add(&member->entry, &prime_fpriv->head);
 	return 0;
 }
+
+int drm_prime_add_exported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct drm_gem_object *obj, uint32_t handle)
+{
+	return drm_prime_add_buf_handle(prime_fpriv, obj, handle, PRIME_EXPORT);
+}
+EXPORT_SYMBOL(drm_prime_add_exported_buf_handle);
+
+int drm_prime_add_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct drm_gem_object *obj, uint32_t handle)
+{
+	return drm_prime_add_buf_handle(prime_fpriv, obj, handle, PRIME_IMPORT);
+}
 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)
@@ -327,7 +345,17 @@  int drm_prime_lookup_imported_buf_handle(struct drm_prime_file_private *prime_fp
 	struct drm_prime_member *member;
 
 	list_for_each_entry(member, &prime_fpriv->head, entry) {
-		if (member->dma_buf == dma_buf) {
+		struct dma_buf *objbuf = NULL;
+
+		if (member->type == PRIME_IMPORT && member->obj->import_attach)
+			objbuf = member->obj->import_attach->dmabuf;
+		else if (member->type == PRIME_EXPORT && member->obj->export_dma_buf)
+			objbuf = member->obj->export_dma_buf;
+		       
+		if (!objbuf)
+			continue;
+
+		if (objbuf == dma_buf) {
 			*handle = member->handle;
 			return 0;
 		}
@@ -336,13 +364,13 @@  int drm_prime_lookup_imported_buf_handle(struct drm_prime_file_private *prime_fp
 }
 EXPORT_SYMBOL(drm_prime_lookup_imported_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_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct drm_gem_object *obj)
 {
 	struct drm_prime_member *member, *safe;
 
 	mutex_lock(&prime_fpriv->lock);
 	list_for_each_entry_safe(member, safe, &prime_fpriv->head, entry) {
-		if (member->dma_buf == dma_buf) {
+		if (member->obj == obj) {
 			list_del(&member->entry);
 			kfree(member);
 		}
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index fad21c9..63fb686 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1559,9 +1559,10 @@  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);
-void drm_prime_remove_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf);
+int drm_prime_add_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct drm_gem_object *obj, uint32_t handle);
+int drm_prime_add_exported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct drm_gem_object *obj, uint32_t handle);
+int drm_prime_lookup_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *, uint32_t *handle);
+void drm_prime_remove_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct drm_gem_object *obj);
 
 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,