Skip to content

Commit b2d4da3

Browse files
Al Viroalexdeucher
authored andcommitted
drm: new helper: drm_gem_prime_handle_to_dmabuf()
Once something had been put into descriptor table, the only thing you can do with it is returning descriptor to userland - you can't withdraw it on subsequent failure exit, etc. You certainly can't count upon it staying in the same slot of descriptor table - another thread could've played with close(2)/dup2(2)/whatnot. drm_gem_prime_handle_to_fd() creates a dmabuf, allocates a descriptor and attaches dmabuf's file to it (the last two steps are done in dma_buf_fd()). That's nice when all you are going to do is passing a descriptor to userland. If you just need to work with the resulting object or have something else to be done that might fail, drm_gem_prime_handle_to_fd() is racy. The problem is analogous to one with anon_inode_getfd(), and solution is similar to what anon_inode_getfile() provides. Add drm_gem_prime_handle_to_dmabuf() - the "set dmabuf up" parts of drm_gem_prime_handle_to_fd() without the descriptor-related ones. Instead of inserting into descriptor table and returning the file descriptor it just returns the struct file. drm_gem_prime_handle_to_fd() becomes a wrapper for it. Other users will be introduced in the next commit. Acked-by: Thomas Zimmermann <[email protected]> Signed-off-by: Al Viro <[email protected]> Signed-off-by: Alex Deucher <[email protected]>
1 parent 81f7804 commit b2d4da3

File tree

2 files changed

+57
-30
lines changed

2 files changed

+57
-30
lines changed

drivers/gpu/drm/drm_prime.c

Lines changed: 54 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -410,22 +410,30 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev,
410410
}
411411

412412
/**
413-
* drm_gem_prime_handle_to_fd - PRIME export function for GEM drivers
413+
* drm_gem_prime_handle_to_dmabuf - PRIME export function for GEM drivers
414414
* @dev: dev to export the buffer from
415415
* @file_priv: drm file-private structure
416416
* @handle: buffer handle to export
417417
* @flags: flags like DRM_CLOEXEC
418-
* @prime_fd: pointer to storage for the fd id of the create dma-buf
419418
*
420419
* This is the PRIME export function which must be used mandatorily by GEM
421420
* drivers to ensure correct lifetime management of the underlying GEM object.
422421
* The actual exporting from GEM object to a dma-buf is done through the
423422
* &drm_gem_object_funcs.export callback.
423+
*
424+
* Unlike drm_gem_prime_handle_to_fd(), it returns the struct dma_buf it
425+
* has created, without attaching it to any file descriptors. The difference
426+
* between those two is similar to that between anon_inode_getfile() and
427+
* anon_inode_getfd(); insertion into descriptor table is something you
428+
* can not revert if any cleanup is needed, so the descriptor-returning
429+
* variants should only be used when you are past the last failure exit
430+
* and the only thing left is passing the new file descriptor to userland.
431+
* When all you need is the object itself or when you need to do something
432+
* else that might fail, use that one instead.
424433
*/
425-
int drm_gem_prime_handle_to_fd(struct drm_device *dev,
434+
struct dma_buf *drm_gem_prime_handle_to_dmabuf(struct drm_device *dev,
426435
struct drm_file *file_priv, uint32_t handle,
427-
uint32_t flags,
428-
int *prime_fd)
436+
uint32_t flags)
429437
{
430438
struct drm_gem_object *obj;
431439
int ret = 0;
@@ -434,14 +442,14 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
434442
mutex_lock(&file_priv->prime.lock);
435443
obj = drm_gem_object_lookup(file_priv, handle);
436444
if (!obj) {
437-
ret = -ENOENT;
445+
dmabuf = ERR_PTR(-ENOENT);
438446
goto out_unlock;
439447
}
440448

441449
dmabuf = drm_prime_lookup_buf_by_handle(&file_priv->prime, handle);
442450
if (dmabuf) {
443451
get_dma_buf(dmabuf);
444-
goto out_have_handle;
452+
goto out;
445453
}
446454

447455
mutex_lock(&dev->object_name_lock);
@@ -463,7 +471,6 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
463471
/* normally the created dma-buf takes ownership of the ref,
464472
* but if that fails then drop the ref
465473
*/
466-
ret = PTR_ERR(dmabuf);
467474
mutex_unlock(&dev->object_name_lock);
468475
goto out;
469476
}
@@ -478,34 +485,51 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
478485
ret = drm_prime_add_buf_handle(&file_priv->prime,
479486
dmabuf, handle);
480487
mutex_unlock(&dev->object_name_lock);
481-
if (ret)
482-
goto fail_put_dmabuf;
483-
484-
out_have_handle:
485-
ret = dma_buf_fd(dmabuf, flags);
486-
/*
487-
* We must _not_ remove the buffer from the handle cache since the newly
488-
* created dma buf is already linked in the global obj->dma_buf pointer,
489-
* and that is invariant as long as a userspace gem handle exists.
490-
* Closing the handle will clean out the cache anyway, so we don't leak.
491-
*/
492-
if (ret < 0) {
493-
goto fail_put_dmabuf;
494-
} else {
495-
*prime_fd = ret;
496-
ret = 0;
488+
if (ret) {
489+
dma_buf_put(dmabuf);
490+
dmabuf = ERR_PTR(ret);
497491
}
498-
499-
goto out;
500-
501-
fail_put_dmabuf:
502-
dma_buf_put(dmabuf);
503492
out:
504493
drm_gem_object_put(obj);
505494
out_unlock:
506495
mutex_unlock(&file_priv->prime.lock);
496+
return dmabuf;
497+
}
498+
EXPORT_SYMBOL(drm_gem_prime_handle_to_dmabuf);
507499

508-
return ret;
500+
/**
501+
* drm_gem_prime_handle_to_fd - PRIME export function for GEM drivers
502+
* @dev: dev to export the buffer from
503+
* @file_priv: drm file-private structure
504+
* @handle: buffer handle to export
505+
* @flags: flags like DRM_CLOEXEC
506+
* @prime_fd: pointer to storage for the fd id of the create dma-buf
507+
*
508+
* This is the PRIME export function which must be used mandatorily by GEM
509+
* drivers to ensure correct lifetime management of the underlying GEM object.
510+
* The actual exporting from GEM object to a dma-buf is done through the
511+
* &drm_gem_object_funcs.export callback.
512+
*/
513+
int drm_gem_prime_handle_to_fd(struct drm_device *dev,
514+
struct drm_file *file_priv, uint32_t handle,
515+
uint32_t flags,
516+
int *prime_fd)
517+
{
518+
struct dma_buf *dmabuf;
519+
int fd = get_unused_fd_flags(flags);
520+
521+
if (fd < 0)
522+
return fd;
523+
524+
dmabuf = drm_gem_prime_handle_to_dmabuf(dev, file_priv, handle, flags);
525+
if (IS_ERR(dmabuf)) {
526+
put_unused_fd(fd);
527+
return PTR_ERR(dmabuf);
528+
}
529+
530+
fd_install(fd, dmabuf->file);
531+
*prime_fd = fd;
532+
return 0;
509533
}
510534
EXPORT_SYMBOL(drm_gem_prime_handle_to_fd);
511535

include/drm/drm_prime.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,9 @@ void drm_gem_dmabuf_release(struct dma_buf *dma_buf);
6969

7070
int drm_gem_prime_fd_to_handle(struct drm_device *dev,
7171
struct drm_file *file_priv, int prime_fd, uint32_t *handle);
72+
struct dma_buf *drm_gem_prime_handle_to_dmabuf(struct drm_device *dev,
73+
struct drm_file *file_priv, uint32_t handle,
74+
uint32_t flags);
7275
int drm_gem_prime_handle_to_fd(struct drm_device *dev,
7376
struct drm_file *file_priv, uint32_t handle, uint32_t flags,
7477
int *prime_fd);

0 commit comments

Comments
 (0)