From: Emil Velikov <emil.velikov@xxxxxxxxxxxxx>
Currently drm_gem_prime_handle_to_fd() consists of illegible amount of
goto labels, for no obvious reason.
Split it in two (as below) making the code far simpler and obvious.
drm_gem_prime_handle_to_fd()
- prime mtx handling
- drm_gem_object lookup and refcounting
- creating an fd for the dmabuf
- hash table lookup
export_handle_to_buf()
- drm_gem_object export and locking
- adding the handle/fd to the hash table
Cc: Daniel Vetter <daniel@xxxxxxxx>
Signed-off-by: Emil Velikov <emil.velikov@xxxxxxxxxxxxx>
---
Currently we dma_buf_put() [in the error path] even for re-export of
original imported object and "self-export" - aka
obj->import_attach->dmabuf and obj->dmabuf.
Have not looked at it too closely, but gut suggests that may be off.
---
drivers/gpu/drm/drm_prime.c | 106 +++++++++++++++++++-----------------
1 file changed, 57 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 0d83b9bbf793..2b0b6e7a6a47 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -545,11 +545,54 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev,
* will clean it up.
*/
obj->dma_buf = dmabuf;
- get_dma_buf(obj->dma_buf);
return dmabuf;
}
+static struct dma_buf *export_handle_to_buf(struct drm_device *dev,
+ struct drm_file *file_priv,
+ struct drm_gem_object *obj,
+ uint32_t handle,
+ uint32_t flags)
+{
+ struct dma_buf *dmabuf = NULL;
+ int ret;
+
+ mutex_lock(&dev->object_name_lock);
+ /* re-export the original imported object */
+ if (obj->import_attach)
+ dmabuf = obj->import_attach->dmabuf;
+
+ if (!dmabuf)
+ dmabuf = obj->dma_buf;
+
+ if (!dmabuf)
+ dmabuf = export_and_register_object(dev, obj, flags);
+
+ if (IS_ERR(dmabuf)) {
+ /* normally the created dma-buf takes ownership of the ref,
+ * but if that fails then drop the ref
+ */
+ mutex_unlock(&dev->object_name_lock);
+ return dmabuf;
+ }
+
+ get_dma_buf(dmabuf);
+
+ /*
+ * If we've exported this buffer then cheat and add it to the import list
+ * so we get the correct handle back. We must do this under the
+ * protection of dev->object_name_lock to ensure that a racing gem close
+ * ioctl doesn't miss to remove this buffer handle from the cache.
+ */
+ ret = drm_prime_add_buf_handle(&file_priv->prime, dmabuf, handle);
+ mutex_unlock(&dev->object_name_lock);
+ if (ret) {
+ dma_buf_put(dmabuf);
+ dmabuf = ERR_PTR(ret);
+ }
+ return dmabuf;
+}
/**
* drm_gem_prime_handle_to_fd - PRIME export function for GEM drivers
* @dev: dev to export the buffer from
@@ -569,7 +612,7 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
int *prime_fd)
{
struct drm_gem_object *obj;
- int ret = 0;
+ int ret;
struct dma_buf *dmabuf;
mutex_lock(&file_priv->prime.lock);
@@ -580,49 +623,14 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
}
dmabuf = drm_prime_lookup_buf_by_handle(&file_priv->prime, handle);
- if (dmabuf) {
- get_dma_buf(dmabuf);
- goto out_have_handle;
- }
+ if (!dmabuf)
+ dmabuf = export_handle_to_buf(dev, file_priv, obj, handle, flags);
- mutex_lock(&dev->object_name_lock);
- /* re-export the original imported object */
- if (obj->import_attach) {
- dmabuf = obj->import_attach->dmabuf;
- get_dma_buf(dmabuf);
- goto out_have_obj;
- }
-
- if (obj->dma_buf) {
- get_dma_buf(obj->dma_buf);
- dmabuf = obj->dma_buf;
- goto out_have_obj;
- }
-
- dmabuf = export_and_register_object(dev, obj, flags);
if (IS_ERR(dmabuf)) {
- /* normally the created dma-buf takes ownership of the ref,
- * but if that fails then drop the ref
- */
ret = PTR_ERR(dmabuf);
- mutex_unlock(&dev->object_name_lock);
- goto out;
+ goto out_object_put;
}
-out_have_obj:
- /*
- * If we've exported this buffer then cheat and add it to the import list
- * so we get the correct handle back. We must do this under the
- * protection of dev->object_name_lock to ensure that a racing gem close
- * ioctl doesn't miss to remove this buffer handle from the cache.
- */
- ret = drm_prime_add_buf_handle(&file_priv->prime,
- dmabuf, handle);
- mutex_unlock(&dev->object_name_lock);
- if (ret)
- goto fail_put_dmabuf;
-
-out_have_handle:
ret = dma_buf_fd(dmabuf, flags);
/*
* We must _not_ remove the buffer from the handle cache since the newly
@@ -630,18 +638,18 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
* and that is invariant as long as a userspace gem handle exists.
* Closing the handle will clean out the cache anyway, so we don't leak.
*/
- if (ret < 0) {
- goto fail_put_dmabuf;
- } else {
- *prime_fd = ret;
- ret = 0;
- }
+ if (ret < 0)
+ goto out_dmabuf_put;
- goto out;
+ *prime_fd = ret;
-fail_put_dmabuf:
+ drm_gem_object_put_unlocked(obj);
+ mutex_unlock(&file_priv->prime.lock);
+ return 0;
+
+out_dmabuf_put:
dma_buf_put(dmabuf);
-out:
+out_object_put:
drm_gem_object_put_unlocked(obj);
out_unlock:
mutex_unlock(&file_priv->prime.lock);
--
2.21.0
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
