Re: [PATCH] dma-buf: heaps: Set VM_PFNMAP in mmap for system and cma heaps
On Sat, Feb 27, 2021 at 1:44 AM Christoph Hellwig 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
Re: [PATCH] dma-buf: heaps: Set VM_PFNMAP in mmap for system and cma heaps
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.
Re: [PATCH] dma-buf: heaps: Set VM_PFNMAP in mmap for system and cma heaps
On Fri, Feb 26, 2021 at 5:09 AM John Stultz wrote: > > Per discussion and patches here: > > https://lore.kernel.org/dri-devel/20210223105951.912577-1-daniel.vet...@ffwll.ch/ > > 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 > Cc: Jason Gunthorpe > Cc: Christian Koenig > Cc: Sumit Semwal > Cc: Liam Mark > Cc: Chris Goldsworthy > Cc: Laura Abbott > Cc: Brian Starkey > Cc: Hridya Valsaraju > Cc: Suren Baghdasaryan > Cc: Sandeep Patil > Cc: Daniel Mentz > Cc: Ørjan Eide > Cc: Robin Murphy > Cc: Ezequiel Garcia > Cc: Simon Ser > Cc: James Jones > Cc: linux-me...@vger.kernel.org > Cc: dri-de...@lists.freedesktop.org > Signed-off-by: John Stultz 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
[PATCH] dma-buf: heaps: Set VM_PFNMAP in mmap for system and cma heaps
Per discussion and patches here: https://lore.kernel.org/dri-devel/20210223105951.912577-1-daniel.vet...@ffwll.ch/ 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 Cc: Jason Gunthorpe Cc: Christian Koenig Cc: Sumit Semwal Cc: Liam Mark Cc: Chris Goldsworthy Cc: Laura Abbott Cc: Brian Starkey Cc: Hridya Valsaraju Cc: Suren Baghdasaryan Cc: Sandeep Patil Cc: Daniel Mentz Cc: Ørjan Eide Cc: Robin Murphy Cc: Ezequiel Garcia Cc: Simon Ser Cc: James Jones Cc: linux-me...@vger.kernel.org Cc: dri-de...@lists.freedesktop.org Signed-off-by: John Stultz --- 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
