Per discussion and patches here:
https://lore.kernel.org/dri-devel/[email protected]/
Daniel is planning on making VM_PFNMAP required on dmabufs.
Thus to avoid the warn_on noise, set the VM_PFNMAP in the
system and cma heap's mmap handler.
Cc: Daniel Vetter <[email protected]>
Cc: Jason Gunthorpe <[email protected]>
Cc: Christian Koenig <[email protected]>
Cc: Sumit Semwal <[email protected]>
Cc: Liam Mark <[email protected]>
Cc: Chris Goldsworthy <[email protected]>
Cc: Laura Abbott <[email protected]>
Cc: Brian Starkey <[email protected]>
Cc: Hridya Valsaraju <[email protected]>
Cc: Suren Baghdasaryan <[email protected]>
Cc: Sandeep Patil <[email protected]>
Cc: Daniel Mentz <[email protected]>
Cc: Ørjan Eide <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Ezequiel Garcia <[email protected]>
Cc: Simon Ser <[email protected]>
Cc: James Jones <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: John Stultz <[email protected]>
---
drivers/dma-buf/heaps/cma_heap.c | 1 +
drivers/dma-buf/heaps/system_heap.c | 4 +++-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
index 364fc2f3e499..34bc3987f942 100644
--- a/drivers/dma-buf/heaps/cma_heap.c
+++ b/drivers/dma-buf/heaps/cma_heap.c
@@ -185,6 +185,7 @@ static int cma_heap_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
vma->vm_ops = &dma_heap_vm_ops;
vma->vm_private_data = buffer;
+ vma->vm_flags |= VM_PFNMAP;
return 0;
}
diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index 3548b20cb98c..8995e3cbfcaf 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -228,8 +228,10 @@ static int system_heap_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
return ret;
addr += PAGE_SIZE;
if (addr >= vma->vm_end)
- return 0;
+ break;
}
+
+ vma->vm_flags |= VM_PFNMAP;
return 0;
}
--
2.25.1
On Fri, Feb 26, 2021 at 5:09 AM John Stultz <[email protected]> wrote:
>
> Per discussion and patches here:
> https://lore.kernel.org/dri-devel/[email protected]/
>
> Daniel is planning on making VM_PFNMAP required on dmabufs.
>
> Thus to avoid the warn_on noise, set the VM_PFNMAP in the
> system and cma heap's mmap handler.
>
> Cc: Daniel Vetter <[email protected]>
> Cc: Jason Gunthorpe <[email protected]>
> Cc: Christian Koenig <[email protected]>
> Cc: Sumit Semwal <[email protected]>
> Cc: Liam Mark <[email protected]>
> Cc: Chris Goldsworthy <[email protected]>
> Cc: Laura Abbott <[email protected]>
> Cc: Brian Starkey <[email protected]>
> Cc: Hridya Valsaraju <[email protected]>
> Cc: Suren Baghdasaryan <[email protected]>
> Cc: Sandeep Patil <[email protected]>
> Cc: Daniel Mentz <[email protected]>
> Cc: Ørjan Eide <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: Ezequiel Garcia <[email protected]>
> Cc: Simon Ser <[email protected]>
> Cc: James Jones <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: John Stultz <[email protected]>
System heap uses remap_pfn_range so looks fine, but cma heap inserts
pages, and that's not fine for VM_PFNMAP. You need to use
vm_insert_pfn or remap_pfn_range or one of the related pfn mapping
functions for that too. I think this should splat when you run it.
Also note that remap_pfn_range updates the vma flags already correctly
for you, so you probably don't want to do that.
Also given that both deal with struct page there's a ton of divergence
between these two that doesn't make much sense. Maybe could even share
the code fully, aside from how you allocate the struct pages.
-Daniel
> ---
> drivers/dma-buf/heaps/cma_heap.c | 1 +
> drivers/dma-buf/heaps/system_heap.c | 4 +++-
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c
> index 364fc2f3e499..34bc3987f942 100644
> --- a/drivers/dma-buf/heaps/cma_heap.c
> +++ b/drivers/dma-buf/heaps/cma_heap.c
> @@ -185,6 +185,7 @@ static int cma_heap_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
>
> vma->vm_ops = &dma_heap_vm_ops;
> vma->vm_private_data = buffer;
> + vma->vm_flags |= VM_PFNMAP;
>
> return 0;
> }
> diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
> index 3548b20cb98c..8995e3cbfcaf 100644
> --- a/drivers/dma-buf/heaps/system_heap.c
> +++ b/drivers/dma-buf/heaps/system_heap.c
> @@ -228,8 +228,10 @@ static int system_heap_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
> return ret;
> addr += PAGE_SIZE;
> if (addr >= vma->vm_end)
> - return 0;
> + break;
> }
> +
> + vma->vm_flags |= VM_PFNMAP;
> return 0;
> }
>
> --
> 2.25.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
On Fri, Feb 26, 2021 at 08:36:55AM +0100, Daniel Vetter wrote:
> Also given that both deal with struct page there's a ton of divergence
> between these two that doesn't make much sense. Maybe could even share
> the code fully, aside from how you allocate the struct pages.
I've been saying that since the code was first submitted. Once pages
are allocated from CMA they should be treated not different from normal
pages.
Please take a look at how the DMA contigous allocator manages to share
all code for handling CMA vs alloc_pages pages.
On Sat, Feb 27, 2021 at 1:44 AM Christoph Hellwig <[email protected]> wrote:
>
> On Fri, Feb 26, 2021 at 08:36:55AM +0100, Daniel Vetter wrote:
> > Also given that both deal with struct page there's a ton of divergence
> > between these two that doesn't make much sense. Maybe could even share
> > the code fully, aside from how you allocate the struct pages.
>
> I've been saying that since the code was first submitted. Once pages
> are allocated from CMA they should be treated not different from normal
> pages.
>
> Please take a look at how the DMA contigous allocator manages to share
> all code for handling CMA vs alloc_pages pages.
I'll take a look at that! Thanks for the pointer!
-john