* [PATCH] drm/prime: Clarify DMA-BUF/GEM Object lifetime @ 2017-01-26 7:22 Oleksandr Andrushchenko 2017-01-26 10:36 ` Daniel Vetter 0 siblings, 1 reply; 5+ messages in thread From: Oleksandr Andrushchenko @ 2017-01-26 7:22 UTC (permalink / raw) To: dri-devel Cc: oleksandr_andrushchenko, vlad.babchuk, Daniel Vetter, andrii.anisov, olekstysh, al1img, joculator From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> 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@gmail.com> Cc: Dave Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> --- 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. * * 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. + * * 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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/prime: Clarify DMA-BUF/GEM Object lifetime 2017-01-26 7:22 [PATCH] drm/prime: Clarify DMA-BUF/GEM Object lifetime Oleksandr Andrushchenko @ 2017-01-26 10:36 ` Daniel Vetter 2017-01-26 10:45 ` Chris Wilson 2017-01-26 10:46 ` Oleksandr Andrushchenko 0 siblings, 2 replies; 5+ messages in thread From: Daniel Vetter @ 2017-01-26 10:36 UTC (permalink / raw) To: Oleksandr Andrushchenko Cc: oleksandr_andrushchenko, vlad.babchuk, Daniel Vetter, dri-devel, andrii.anisov, olekstysh, al1img, joculator On Thu, Jan 26, 2017 at 09:22:46AM +0200, Oleksandr Andrushchenko wrote: > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > 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@gmail.com> > Cc: Dave Airlie <airlied@linux.ie> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > --- > 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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/prime: Clarify DMA-BUF/GEM Object lifetime 2017-01-26 10:36 ` Daniel Vetter @ 2017-01-26 10:45 ` Chris Wilson 2017-01-26 10:46 ` Oleksandr Andrushchenko 2017-01-26 10:46 ` Oleksandr Andrushchenko 1 sibling, 1 reply; 5+ messages in thread From: Chris Wilson @ 2017-01-26 10:45 UTC (permalink / raw) To: Daniel Vetter Cc: oleksandr_andrushchenko, Oleksandr Andrushchenko, Daniel Vetter, dri-devel, andrii.anisov, olekstysh, vlad.babchuk, al1img, joculator On Thu, Jan 26, 2017 at 11:36:05AM +0100, Daniel Vetter wrote: > On Thu, Jan 26, 2017 at 09:22:46AM +0200, Oleksandr Andrushchenko wrote: > > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > > > 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@gmail.com> > > Cc: Dave Airlie <airlied@linux.ie> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > --- > > 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()." For GEM-based drivers, the dmabuf should be exported using drm_gem_dmabuf_export() and then released by drm_gem_dmabuf_release(). Just the emphasize the symmetry (because of the internal magics)? -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/prime: Clarify DMA-BUF/GEM Object lifetime 2017-01-26 10:45 ` Chris Wilson @ 2017-01-26 10:46 ` Oleksandr Andrushchenko 0 siblings, 0 replies; 5+ messages in thread From: Oleksandr Andrushchenko @ 2017-01-26 10:46 UTC (permalink / raw) To: Chris Wilson, Daniel Vetter, oleksandr_andrushchenko, vlad.babchuk, Daniel Vetter, dri-devel, andrii.anisov, olekstysh, al1img, joculator On 01/26/2017 12:45 PM, Chris Wilson wrote: > On Thu, Jan 26, 2017 at 11:36:05AM +0100, Daniel Vetter wrote: >> On Thu, Jan 26, 2017 at 09:22:46AM +0200, Oleksandr Andrushchenko wrote: >>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >>> >>> 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@gmail.com> >>> Cc: Dave Airlie <airlied@linux.ie> >>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >>> --- >>> 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()." > For GEM-based drivers, the dmabuf should be exported using > drm_gem_dmabuf_export() and then released by drm_gem_dmabuf_release(). Will use this in v1 > Just the emphasize the symmetry (because of the internal magics)? > -Chris > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/prime: Clarify DMA-BUF/GEM Object lifetime 2017-01-26 10:36 ` Daniel Vetter 2017-01-26 10:45 ` Chris Wilson @ 2017-01-26 10:46 ` Oleksandr Andrushchenko 1 sibling, 0 replies; 5+ messages in thread From: Oleksandr Andrushchenko @ 2017-01-26 10:46 UTC (permalink / raw) To: Daniel Vetter Cc: oleksandr_andrushchenko, vlad.babchuk, Daniel Vetter, dri-devel, andrii.anisov, olekstysh, al1img, joculator 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@epam.com> >> >> 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@gmail.com> >> Cc: Dave Airlie <airlied@linux.ie> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >> --- >> 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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-01-26 10:46 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-01-26 7:22 [PATCH] drm/prime: Clarify DMA-BUF/GEM Object lifetime Oleksandr Andrushchenko 2017-01-26 10:36 ` Daniel Vetter 2017-01-26 10:45 ` Chris Wilson 2017-01-26 10:46 ` Oleksandr Andrushchenko 2017-01-26 10:46 ` Oleksandr Andrushchenko
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.