Re: [PATCH 2/4] usb: usbfs: Use consistent mmap functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10.05.23 17:41, Ruihan Li wrote:
Hi Alan,

On Wed, May 10, 2023 at 10:38:48AM -0400, Alan Stern wrote:
On Wed, May 10, 2023 at 04:55:25PM +0800, Ruihan Li wrote:
When hcd->localmem_pool is non-null, it is used to allocate DMA memory.
In this case, the dma address will be properly returned (in dma_handle),
and dma_mmap_coherent should be used to map this memory into the user
space. However, the current implementation uses pfn_remap_range, which
is supposed to map normal pages (instead of DMA pages).

Instead of repeating the logic in the memory allocation function, this
patch introduces a more robust solution. To address the previous issue,
this patch checks the type of allocated memory by testing whether
dma_handle is properly set. If dma_handle is properly returned, it means
some DMA pages are allocated and dma_mmap_coherent should be used to map
them. Otherwise, normal pages are allocated and pfn_remap_range should
be called. This ensures that the correct mmap functions are used
consistently, independently with logic details that determine which type
of memory gets allocated.

Fixes: a0e710a7def4 ("USB: usbfs: fix mmap dma mismatch")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Ruihan Li <lrh2000@xxxxxxxxxx>
---
  drivers/usb/core/devio.c | 10 ++++++++--
  1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index b4cf9e860..5067030b7 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -235,7 +235,7 @@ static int usbdev_mmap(struct file *file, struct vm_area_struct *vma)
  	size_t size = vma->vm_end - vma->vm_start;
  	void *mem;
  	unsigned long flags;
-	dma_addr_t dma_handle;
+	dma_addr_t dma_handle = DMA_MAPPING_ERROR;
  	int ret;
ret = usbfs_increase_memory_usage(size + sizeof(struct usb_memory));
@@ -265,7 +265,13 @@ static int usbdev_mmap(struct file *file, struct vm_area_struct *vma)
  	usbm->vma_use_count = 1;
  	INIT_LIST_HEAD(&usbm->memlist);
- if (hcd->localmem_pool || !hcd_uses_dma(hcd)) {
+	/* In DMA-unavailable cases, hcd_buffer_alloc_pages allocates
+	 * normal pages and assigns DMA_MAPPING_ERROR to dma_handle. Check
+	 * whether we are in such cases, and then use remap_pfn_range (or
+	 * dma_mmap_coherent) to map normal (or DMA) pages into the user
+	 * space, respectively.
+	 */

Another stylistic issue.  For multi-line comments, the format we use is:

	/*
	 * Blah, blah, blah
	 * Blah, blah, blah
	 */

Alan Stern

Yeah, I am pretty sure it is another style difference with the net
subsystem. Again, in the next version, I'll follow the coding style that
you have pointed out.

Documentation/process/coding-style.rst

spells out that net/ and drivers/net/ are "special".

Regarding breaking long lines, it's just an inconsistent, undocumented mess IIRC ...

--
Thanks,

David / dhildenb





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux