[PATCH 3/4] drm/gem: cleanup prime lock issue with drm_gem_handle_create_tail

[Dave Airlie <airlied@gmail.com>]

From: Dave Airlie <airlied@redhat.com>

This patch cleans up one locking order issue with a deadlock on
the prime lock.

It avoids calling the prime delete function from drm_gem_handle_create_tail,
and instead open codes the exit paths, it also splits the common code
out for the exit path which can use it.

This means the callee has to call the prime cleanup paths, which is fine
since this is only the prime code and it holds the lock already.

This is based on an patch/idea by Daniel Vetter, but I've cleaned it up
and reworked it.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/drm_gem.c   | 55 +++++++++++++++++++++++++++------------------
 drivers/gpu/drm/drm_prime.c |  3 +++
 2 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 12d0dc7..49354d7 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -239,14 +239,12 @@ drm_gem_object_handle_unreference_unlocked(struct drm_gem_object *obj)
 	drm_gem_object_unreference_unlocked(obj);
 }
 
+/* use for deletion and failure in allocation */
 static void
-drm_gem_handle_delete_tail(struct drm_file *filp,
-			   struct drm_gem_object *obj)
+drm_gem_handle_cleanup(struct drm_file *filp,
+		       struct drm_gem_object *obj)
 {
 	struct drm_device *dev = obj->dev;
-
-	if (drm_core_check_feature(dev, DRIVER_PRIME))
-		drm_gem_remove_prime_handles(obj, filp);
 	drm_vma_node_revoke(&obj->vma_node, filp->filp);
 
 	if (dev->driver->gem_close_object)
@@ -254,6 +252,17 @@ drm_gem_handle_delete_tail(struct drm_file *filp,
 	drm_gem_object_handle_unreference_unlocked(obj);
 }
 
+static void
+drm_gem_handle_delete_tail_prime(struct drm_file *filp,
+				 struct drm_gem_object *obj)
+{
+	struct drm_device *dev = obj->dev;
+
+	if (drm_core_check_feature(dev, DRIVER_PRIME))
+		drm_gem_remove_prime_handles(obj, filp);
+	drm_gem_handle_cleanup(filp, obj);
+}
+
 /**
  * drm_gem_handle_delete - deletes the given file-private handle
  * @filp: drm file-private structure to use for the handle look up
@@ -291,7 +300,7 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle)
 	idr_remove(&filp->object_idr, handle);
 	spin_unlock(&filp->table_lock);
 
-	drm_gem_handle_delete_tail(filp, obj);
+	drm_gem_handle_delete_tail_prime(filp, obj);
 	return 0;
 }
 EXPORT_SYMBOL(drm_gem_handle_delete);
@@ -333,6 +342,17 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
 
 	WARN_ON(!mutex_is_locked(&dev->object_name_lock));
 
+	ret = drm_vma_node_allow(&obj->vma_node, file_priv->filp);
+	if (ret) {
+		goto out_unlock;
+	}
+
+	if (dev->driver->gem_open_object) {
+		ret = dev->driver->gem_open_object(obj, file_priv);
+		if (ret)
+			goto out_vma;
+	}
+
 	/*
 	 * Get the user-visible handle using idr.  Preload and perform
 	 * allocation under our spinlock.
@@ -347,26 +367,17 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
 	idr_preload_end();
 	mutex_unlock(&dev->object_name_lock);
 	if (ret < 0) {
-		drm_gem_object_handle_unreference_unlocked(obj);
+		drm_gem_handle_cleanup(file_priv, obj);
 		return ret;
 	}
 	*handlep = ret;
 
-	ret = drm_vma_node_allow(&obj->vma_node, file_priv->filp);
-	if (ret) {
-		drm_gem_handle_delete(file_priv, *handlep);
-		return ret;
-	}
-
-	if (dev->driver->gem_open_object) {
-		ret = dev->driver->gem_open_object(obj, file_priv);
-		if (ret) {
-			drm_gem_handle_delete(file_priv, *handlep);
-			return ret;
-		}
-	}
-
 	return 0;
+out_vma:
+	drm_vma_node_revoke(&obj->vma_node, file_priv->filp);
+out_unlock:
+	mutex_unlock(&dev->object_name_lock);
+	return ret;
 }
 
 /**
@@ -717,7 +728,7 @@ drm_gem_object_release_handle(int id, void *ptr, void *data)
 	struct drm_file *file_priv = data;
 	struct drm_gem_object *obj = ptr;
 
-	drm_gem_handle_delete_tail(file_priv, obj);
+	drm_gem_handle_delete_tail_prime(file_priv, obj);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index d22ce83..88c004e 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -590,6 +590,9 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
 
 	/* drm_gem_handle_create_tail unlocks dev->object_name_lock. */
 	ret = drm_gem_handle_create_tail(file_priv, obj, handle);
+	if (ret)
+		drm_prime_remove_buf_handle_locked(&file_priv->prime,
+						   obj->dma_buf);
 	drm_gem_object_unreference_unlocked(obj);
 	if (ret)
 		goto out_put;
-- 
2.5.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel