comedi: allocate DMA coherent buffer as individual pages

Depending on the driver, the acquisition buffer is allocated either from
normal memory, or from DMA coherent memory.  For normal memory, the
buffer is allocated as individual pages, but for DMA coherent memory, it
is allocated as a single block.  Prior to commit e36472145aa7 ("staging:
comedi: use dma_mmap_coherent for DMA-able buffer mmap"), the buffer was
allocated as individual pages for DMA coherent memory too, but that was
changed to allocate it as a single block to allow `dma_mmap_coherent()`
to be used to mmap it, because that requires the pages being mmap'ed to
be contiguous.

This patch allocates the buffer from DMA coherent memory a page at a
time again, and works around the limitation of `dma_mmap_coherent()` by
calling it in a loop for each page, with temporarily modified `vm_start`
and `vm_end` values in the VMA.  (The `vm_pgoff` value is 0.)

Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
Link: https://lore.kernel.org/r/20250415114008.5977-5-abbotti@mev.co.uk
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/comedi/comedi_buf.c b/drivers/comedi/comedi_buf.c
index 5807007bb3dd95ce90a1ce6c2b9479ba68fea0dd..002c0e76baff26ed435e378f20e4b4a43e74e5ba 100644 (file)
--- a/drivers/comedi/comedi_buf.c
+++ b/drivers/comedi/comedi_buf.c
@@ -27,14 +27,12 @@ static void comedi_buf_map_kref_release(struct kref *kref)
 
        if (bm->page_list) {
                if (bm->dma_dir != DMA_NONE) {
-                       /*
-                        * DMA buffer was allocated as a single block.
-                        * Address is in page_list[0].
-                        */
-                       buf = &bm->page_list[0];
-                       dma_free_coherent(bm->dma_hw_dev,
-                                         PAGE_SIZE * bm->n_pages,
-                                         buf->virt_addr, buf->dma_addr);
+                       for (i = 0; i < bm->n_pages; i++) {
+                               buf = &bm->page_list[i];
+                               dma_free_coherent(bm->dma_hw_dev, PAGE_SIZE,
+                                                 buf->virt_addr,
+                                                 buf->dma_addr);
+                       }
                } else {
                        for (i = 0; i < bm->n_pages; i++) {
                                buf = &bm->page_list[i];
@@ -88,26 +86,14 @@ comedi_buf_map_alloc(struct comedi_device *dev, enum dma_data_direction dma_dir,
                goto err;
 
        if (bm->dma_dir != DMA_NONE) {
-               void *virt_addr;
-               dma_addr_t dma_addr;
-
-               /*
-                * Currently, the DMA buffer needs to be allocated as a
-                * single block so that it can be mmap()'ed.
-                */
-               virt_addr = dma_alloc_coherent(bm->dma_hw_dev,
-                                              PAGE_SIZE * n_pages, &dma_addr,
-                                              GFP_KERNEL);
-               if (!virt_addr)
-                       goto err;
-
                for (i = 0; i < n_pages; i++) {
                        buf = &bm->page_list[i];
-                       buf->virt_addr = virt_addr + (i << PAGE_SHIFT);
-                       buf->dma_addr = dma_addr + (i << PAGE_SHIFT);
+                       buf->virt_addr =
+                           dma_alloc_coherent(bm->dma_hw_dev, PAGE_SIZE,
+                                              &buf->dma_addr, GFP_KERNEL);
+                       if (!buf->virt_addr)
+                               break;
                }
-
-               bm->n_pages = i;
        } else {
                for (i = 0; i < n_pages; i++) {
                        buf = &bm->page_list[i];
@@ -117,11 +103,10 @@ comedi_buf_map_alloc(struct comedi_device *dev, enum dma_data_direction dma_dir,
 
                        SetPageReserved(virt_to_page(buf->virt_addr));
                }
-
-               bm->n_pages = i;
-               if (i < n_pages)
-                       goto err;
        }
+       bm->n_pages = i;
+       if (i < n_pages)
+               goto err;
 
        return bm;
 

diff --git a/drivers/comedi/comedi_fops.c b/drivers/comedi/comedi_fops.c
index 37cfef36c1ad6a9da8ce381e63251d30ad5146e3..3383a7ce27ff55b79c662467c38494045bcd2d9f 100644 (file)
--- a/drivers/comedi/comedi_fops.c
+++ b/drivers/comedi/comedi_fops.c
@@ -2387,13 +2387,27 @@ static int comedi_mmap(struct file *file, struct vm_area_struct *vma)
                goto done;
        }
        if (bm->dma_dir != DMA_NONE) {
+               unsigned long vm_start = vma->vm_start;
+               unsigned long vm_end = vma->vm_end;
+
                /*
-                * DMA buffer was allocated as a single block.
-                * Address is in page_list[0].
+                * Buffer pages are not contiguous, so temporarily modify VMA
+                * start and end addresses for each buffer page.
                 */
-               buf = &bm->page_list[0];
-               retval = dma_mmap_coherent(bm->dma_hw_dev, vma, buf->virt_addr,
-                                          buf->dma_addr, n_pages * PAGE_SIZE);
+               for (i = 0; i < n_pages; ++i) {
+                       buf = &bm->page_list[i];
+                       vma->vm_start = start;
+                       vma->vm_end = start + PAGE_SIZE;
+                       retval = dma_mmap_coherent(bm->dma_hw_dev, vma,
+                                                  buf->virt_addr,
+                                                  buf->dma_addr, PAGE_SIZE);
+                       if (retval)
+                               break;
+
+                       start += PAGE_SIZE;
+               }
+               vma->vm_start = vm_start;
+               vma->vm_end = vm_end;
        } else {
                for (i = 0; i < n_pages; ++i) {
                        unsigned long pfn;
@@ -2407,19 +2421,18 @@ static int comedi_mmap(struct file *file, struct vm_area_struct *vma)
 
                        start += PAGE_SIZE;
                }
+       }
 
 #ifdef CONFIG_MMU
-               /*
-                * Leaving behind a partial mapping of a buffer we're about to
-                * drop is unsafe, see remap_pfn_range_notrack().
-                * We need to zap the range here ourselves instead of relying
-                * on the automatic zapping in remap_pfn_range() because we call
-                * remap_pfn_range() in a loop.
-                */
-               if (retval)
-                       zap_vma_ptes(vma, vma->vm_start, size);
+       /*
+        * Leaving behind a partial mapping of a buffer we're about to drop is
+        * unsafe, see remap_pfn_range_notrack().  We need to zap the range
+        * here ourselves instead of relying on the automatic zapping in
+        * remap_pfn_range() because we call remap_pfn_range() in a loop.
+        */
+       if (retval)
+               zap_vma_ptes(vma, vma->vm_start, size);
 #endif
-       }
 
        if (retval == 0) {
                vma->vm_ops = &comedi_vm_ops;