drm/prime: Clarify DMA-BUF/GEM Object lifetime
Subsystems (2)
PATCH 1/1 drm/prime: Clarify DMA-BUF/GEM Object lifetime
From: Oleksandr Andrushchenko <[email protected]>
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.
--- drivers/gpu/drm/drm_prime.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/gpu/drm/drm_prime.c +8 -1
@@ -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 *
Signature · 6 lines
2.7.4 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Quoted 23 lines · Oleksandr Andrushchenko
From: Oleksandr Andrushchenko <[email protected]>
From the description of the "DMA-BUF/GEM Object references and lifetime overview" it is not clear when exactly
dma_bufgets 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 <[email protected]>Cc Dave Airlie <[email protected]>Cc Daniel Vetter <[email protected]>Signed-off-by Oleksandr Andrushchenko <[email protected]>--- drivers/gpu/drm/drm_prime.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)drivers/gpu/drm/drm_prime.c +0
-0
drivers/gpu/drm/drm_prime.c line 44
Quoted 5 lines
drivers/gpu/drm/drm_prime.c +1
-1@@ -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()."
drivers/gpu/drm/drm_prime.c line 59
Quoted 13 lines
drivers/gpu/drm/drm_prime.c +6
-0* * 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
Quoted 7 lines
drivers/gpu/drm/drm_prime.c +1
-0+* * Thus the chain of references always flows in one direction * (avoiding loops): importing_gem -> dmabuf -> exporting_gem * Signature · 1 line
2.7.4
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Quoted 35 lines · Daniel Vetter
Quoted 23 lines · Oleksandr Andrushchenko
From: Oleksandr Andrushchenko <[email protected]>
From the description of the "DMA-BUF/GEM Object references and lifetime overview" it is not clear when exactly
dma_bufgets 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 <[email protected]>Cc Dave Airlie <[email protected]>Cc Daniel Vetter <[email protected]>Signed-off-by Oleksandr Andrushchenko <[email protected]>--- drivers/gpu/drm/drm_prime.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)drivers/gpu/drm/drm_prime.c +0
-0drivers/gpu/drm/drm_prime.c line 44
Quoted 5 lines
drivers/gpu/drm/drm_prime.c +1
-1@@ -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_releasefunction, e.g. vmwgfx is ttm-based, not gem and hence can't usedrm_gem_dmabuf_release().
Quoted 6 lines
Maybe let's rewrite the entire sentence:
"This refernce needs to be released when the final reference to the
&dma_bufitself is dropped and its &dma_buf_ops.release function is called. For GEM-based drivers this is done by usingdrm_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
Signature · 5 lines
Chris Wilson, Intel Open Source Technology Centre _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Quoted 35 lines · Chris Wilson
Quoted 34 lines · Daniel Vetter
Quoted 23 lines · Oleksandr Andrushchenko
From: Oleksandr Andrushchenko <[email protected]>
From the description of the "DMA-BUF/GEM Object references
and lifetime overview" it is not clear when exactlydma_bufgets 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 <[email protected]>Cc Dave Airlie <[email protected]>Cc Daniel Vetter <[email protected]>Signed-off-by Oleksandr Andrushchenko <[email protected]>--- drivers/gpu/drm/drm_prime.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)drivers/gpu/drm/drm_prime.c +0
-0drivers/gpu/drm/drm_prime.c line 44
Quoted 5 lines
drivers/gpu/drm/drm_prime.c +1
-1@@ -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_releasefunction, e.g. vmwgfx is ttm-based, not gem and hence can't usedrm_gem_dmabuf_release().
Quoted 8 lines
Quoted 6 lines
Maybe let's rewrite the entire sentence:
"This refernce needs to be released when the final reference to the
&dma_bufitself is dropped and its &dma_buf_ops.release function is called. For GEM-based drivers this is done by usingdrm_gem_dmabuf_release()."For GEM-based drivers, the dmabuf should be exported using
drm_gem_dmabuf_export()and then released bydrm_gem_dmabuf_release().
Will use this in v1
Quoted 3 lines
Just the emphasize the symmetry (because of the internal magics)? -Chris
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Thank you for comments,
I will update the patch and send v1.
Quoted 19 lines · Daniel Vetter
Quoted 18 lines · Oleksandr Andrushchenko
From: Oleksandr Andrushchenko <[email protected]>
From the description of the "DMA-BUF/GEM Object references
and lifetime overview" it is not clear when exactlydma_bufgets 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 <[email protected]>Cc Dave Airlie <[email protected]>Cc Daniel Vetter <[email protected]>Signed-off-by Oleksandr Andrushchenko <[email protected]>--- drivers/gpu/drm/drm_prime.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
Quoted 14 lines
drivers/gpu/drm/drm_prime.c line 44
Quoted 10 lines
drivers/gpu/drm/drm_prime.c +1
-1@@ -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_releasefunction, e.g. vmwgfx is ttm-based, not gem and hence can't usedrm_gem_dmabuf_release().
ah, I am still new to DRM, so it is good to know
Quoted 6 lines
Maybe let's rewrite the entire sentence:
"This refernce needs to be released when the final reference to the
&dma_bufitself is dropped and its &dma_buf_ops.release function is called. For GEM-based drivers this is done by usingdrm_gem_dmabuf_release()."
Will put that into the v1
Quoted 17 lines
drivers/gpu/drm/drm_prime.c line 59
Quoted 13 lines
drivers/gpu/drm/drm_prime.c +6
-0* * 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.
Quoted 3 lines
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
Quoted 1 line
Thanks, Daniel
Thank you,
Oleksandr
Quoted 7 lines
Quoted 7 lines
drivers/gpu/drm/drm_prime.c +1
-0+* * Thus the chain of references always flows in one direction * (avoiding loops): importing_gem -> dmabuf -> exporting_gem * Signature · 1 line
2.7.4
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel