[2/2] drm: share address space for dma bufs
diff mbox series

Message ID 20191121103807.18424-3-kraxel@redhat.com
State New
Headers show
Series
  • drm: mmap fixups
Related show

Commit Message

Gerd Hoffmann Nov. 21, 2019, 10:38 a.m. UTC
Use the shared address space of the drm device (see drm_open() in
drm_file.c) for dma-bufs too.  That removes a difference betweem drm
device mmap vmas and dma-buf mmap vmas and fixes corner cases like
unmaps not working properly.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/drm_prime.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Daniel Vetter Nov. 21, 2019, 1:55 p.m. UTC | #1
On Thu, Nov 21, 2019 at 11:38:07AM +0100, Gerd Hoffmann wrote:
> Use the shared address space of the drm device (see drm_open() in
> drm_file.c) for dma-bufs too.  That removes a difference betweem drm
> device mmap vmas and dma-buf mmap vmas and fixes corner cases like
> unmaps not working properly.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  drivers/gpu/drm/drm_prime.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index a9633bd241bb..c3fc341453c0 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -240,6 +240,7 @@ void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv)
>  struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev,
>  				      struct dma_buf_export_info *exp_info)
>  {
> +	struct drm_gem_object *obj = exp_info->priv;
>  	struct dma_buf *dma_buf;
>  
>  	dma_buf = dma_buf_export(exp_info);
> @@ -247,7 +248,8 @@ struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev,
>  		return dma_buf;
>  
>  	drm_dev_get(dev);
> -	drm_gem_object_get(exp_info->priv);
> +	drm_gem_object_get(obj);
> +	dma_buf->file->f_mapping = obj->dev->anon_inode->i_mapping;

Can you pls also throw the change to amdgpu_gem_prime_export on top here?
Imo makes a lot more sense that way. With that added I'm happy to r-b.
-Daniel

>  
>  	return dma_buf;
>  }
> -- 
> 2.18.1
>
Ruhl, Michael J Nov. 21, 2019, 4:42 p.m. UTC | #2
>-----Original Message-----
>From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Gerd
>Hoffmann
>Sent: Thursday, November 21, 2019 5:38 AM
>To: dri-devel@lists.freedesktop.org
>Cc: David Airlie <airlied@linux.ie>; intel-gfx@lists.freedesktop.org; open list
><linux-kernel@vger.kernel.org>; Maxime Ripard <mripard@kernel.org>; Gerd
>Hoffmann <kraxel@redhat.com>
>Subject: [Intel-gfx] [PATCH 2/2] drm: share address space for dma bufs
>
>Use the shared address space of the drm device (see drm_open() in
>drm_file.c) for dma-bufs too.  That removes a difference betweem drm
>device mmap vmas and dma-buf mmap vmas and fixes corner cases like
>unmaps not working properly.

Hi Gerd,

Just want to make sure I understand this...

So unmaps will not work correctly for mappings when a driver does a
drm_vma_node_unamp()?

I.e. the dmabuf mappings will not get cleaned up correctly?

This is a day one bug?

Thanks,

Mike


>Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>---
> drivers/gpu/drm/drm_prime.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
>index a9633bd241bb..c3fc341453c0 100644
>--- a/drivers/gpu/drm/drm_prime.c
>+++ b/drivers/gpu/drm/drm_prime.c
>@@ -240,6 +240,7 @@ void drm_prime_destroy_file_private(struct
>drm_prime_file_private *prime_fpriv)
> struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev,
> 				      struct dma_buf_export_info *exp_info)
> {
>+	struct drm_gem_object *obj = exp_info->priv;
> 	struct dma_buf *dma_buf;
>
> 	dma_buf = dma_buf_export(exp_info);
>@@ -247,7 +248,8 @@ struct dma_buf *drm_gem_dmabuf_export(struct
>drm_device *dev,
> 		return dma_buf;
>
> 	drm_dev_get(dev);
>-	drm_gem_object_get(exp_info->priv);
>+	drm_gem_object_get(obj);
>+	dma_buf->file->f_mapping = obj->dev->anon_inode->i_mapping;
>
> 	return dma_buf;
> }
>--
>2.18.1
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Gerd Hoffmann Nov. 22, 2019, 6:32 a.m. UTC | #3
On Thu, Nov 21, 2019 at 04:42:10PM +0000, Ruhl, Michael J wrote:
> >-----Original Message-----
> >From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Gerd
> >Hoffmann
> >Sent: Thursday, November 21, 2019 5:38 AM
> >To: dri-devel@lists.freedesktop.org
> >Cc: David Airlie <airlied@linux.ie>; intel-gfx@lists.freedesktop.org; open list
> ><linux-kernel@vger.kernel.org>; Maxime Ripard <mripard@kernel.org>; Gerd
> >Hoffmann <kraxel@redhat.com>
> >Subject: [Intel-gfx] [PATCH 2/2] drm: share address space for dma bufs
> >
> >Use the shared address space of the drm device (see drm_open() in
> >drm_file.c) for dma-bufs too.  That removes a difference betweem drm
> >device mmap vmas and dma-buf mmap vmas and fixes corner cases like
> >unmaps not working properly.
> 
> Hi Gerd,
> 
> Just want to make sure I understand this...
> 
> So unmaps will not work correctly for mappings when a driver does a
> drm_vma_node_unamp()?

Completely removing the mapping (aka munmap syscall) works fine.
Zapping the pte's (using madvise(dontneed) for example) doesn't.

> This is a day one bug?

I guess so, but I'll leave that to others being active longer than me in
drm hacking to answer ...

cheers,
  Gerd

Patch
diff mbox series

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index a9633bd241bb..c3fc341453c0 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -240,6 +240,7 @@  void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv)
 struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev,
 				      struct dma_buf_export_info *exp_info)
 {
+	struct drm_gem_object *obj = exp_info->priv;
 	struct dma_buf *dma_buf;
 
 	dma_buf = dma_buf_export(exp_info);
@@ -247,7 +248,8 @@  struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev,
 		return dma_buf;
 
 	drm_dev_get(dev);
-	drm_gem_object_get(exp_info->priv);
+	drm_gem_object_get(obj);
+	dma_buf->file->f_mapping = obj->dev->anon_inode->i_mapping;
 
 	return dma_buf;
 }