Re: [PATCH] drm/prime: Clarify DMA-BUF/GEM Object lifetime

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thank you for comments,
I will update the patch and send v1.

On 01/26/2017 12:36 PM, Daniel Vetter wrote:
On Thu, Jan 26, 2017 at 09:22:46AM +0200, Oleksandr Andrushchenko wrote:
From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>

 From the description of the "DMA-BUF/GEM Object references
and lifetime overview" it is not clear when exactly
dma_buf gets destroyed and memory freed: only driver
.release function mentioned which makes confusion on the
real buffer's lifetime.

Add more description so all the paths are covered.

Cc: Rob Clark <robdclark@xxxxxxxxx>
Cc: Dave Airlie <airlied@xxxxxxxx>
Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
---
  drivers/gpu/drm/drm_prime.c | 9 ++++++++-
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 8d77b2462594..c061a0b29819 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -41,7 +41,7 @@
   * object. It takes this reference in handle_to_fd_ioctl, when it
   * first calls .prime_export and stores the exporting GEM object in
   * the dma_buf priv. This reference is released when the dma_buf
- * object goes away in the driver .release function.
+ * object goes away.
This was meant to talke about the release function of the dma-buf file,
not the drm_driver. Note that not all drivers use the same _release
function, e.g. vmwgfx is ttm-based, not gem and hence can't use
drm_gem_dmabuf_release().
ah, I am still new to DRM, so it is good to know
Maybe let's rewrite the entire sentence:

"This refernce needs to be released when the final reference to the
&dma_buf itself is dropped and its &dma_buf_ops.release function is
called. For GEM-based drivers this is done by using
drm_gem_dmabuf_release()."
Will put that into the v1
   *
   * On the import the importing GEM object holds a reference to the
   * dma_buf (which in turn holds a ref to the exporting GEM object).
@@ -51,6 +51,13 @@
   * when the imported object is destroyed, we remove the attachment
   * and drop the reference to the dma_buf.
   *
+ * When all the references to the dma_buf are dropped, e.g. when
+ * userspace closes both handles to the imported (fd_to_handle_ioctl)
+ * and exported (handle_to_fd_ioctl) dma_buf and closes the corresponding
+ * file descriptor (handle_to_fd), then dma_buf gets destroyed.
+ * This can also happen as a part of the clean up procedure in the
+ * driver .release function if userspace fails to properly clean up.
This isn't accurate, since the kernel can also internally hold references
to the dma_buf, not just userspace by keeping the fd open. I'd drop this,
it doesn't really explain things.

Maybe add a note to the previous hunk like "Note that both the kernel and
userspace (by keeeping the PRIME file descriptors open) can hold
references onto a &dma_buf."
Will put that into the v1
Thanks, Daniel
Thank you,
Oleksandr
+ *
   * Thus the chain of references always flows in one direction
   * (avoiding loops): importing_gem -> dmabuf -> exporting_gem
   *
--
2.7.4


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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux