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(). 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()." > * > * 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." Thanks, Daniel > + * > * Thus the chain of references always flows in one direction > * (avoiding loops): importing_gem -> dmabuf -> exporting_gem > * > -- > 2.7.4 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
