Re: [PATCH 3/4] dma-mapping: introduce a dma_common_find_pages helper
Hi Christoph, On Fri, Aug 30, 2019 at 8:30 AM Christoph Hellwig wrote: > A helper to find the backing page array based on a virtual address. > This also ensures we do the same vm_flags check everywhere instead > of slightly different or missing ones in a few places. > > Signed-off-by: Christoph Hellwig This is now commit 5cf4537975bbd569 ("dma-mapping: introduce a dma_common_find_pages helper") in v5.4-rc1, and causes the following warning during s2ram on r8a7740/armadillo, r7s72100/rskrza1, and r7s9210/rza2mevb: sh-eth e9a0.ethernet eth0: Link is Down +[ cut here ] +WARNING: CPU: 0 PID: 556 at kernel/dma/remap.c:93 remap_allocator_free+0x20/0x30 +trying to free invalid coherent area: 6909579a +Modules linked in: +CPU: 0 PID: 556 Comm: s2ram Not tainted 5.3.0-rc6-armadillo-00027-g5cf4537975bbd569-dirty #113 +Hardware name: Generic R8A7740 (Flattened Device Tree) +[] (unwind_backtrace) from [] (show_stack+0x10/0x14) +[] (show_stack) from [] (__warn+0xec/0x104) +[] (__warn) from [] (warn_slowpath_fmt+0x44/0x6c) +[] (warn_slowpath_fmt) from [] (remap_allocator_free+0x20/0x30) +[] (remap_allocator_free) from [] (__arm_dma_free.constprop.2+0x114/0x144) +[] (__arm_dma_free.constprop.2) from [] (sh_eth_ring_free+0xb8/0x114) +[] (sh_eth_ring_free) from [] (sh_eth_close+0x68/0x8c) +[] (sh_eth_close) from [] (sh_eth_resume+0x44/0x90) +[] (sh_eth_resume) from [] (dpm_run_callback+0x64/0xdc) +[] (dpm_run_callback) from [] (device_resume+0xbc/0x180) +[] (device_resume) from [] (dpm_resume+0x124/0x1b0) +[] (dpm_resume) from [] (dpm_resume_end+0xc/0x18) +[] (dpm_resume_end) from [] (suspend_devices_and_enter+0x15c/0x5ac) +[] (suspend_devices_and_enter) from [] (pm_suspend+0x240/0x2f4) +[] (pm_suspend) from [] (state_store+0x54/0x8c) +[] (state_store) from [] (kernfs_fop_write+0x154/0x1c8) +[] (kernfs_fop_write) from [] (__vfs_write+0x28/0xe0) +[] (__vfs_write) from [] (vfs_write+0x98/0xbc) +[] (vfs_write) from [] (ksys_write+0x68/0xb4) +[] (ksys_write) from [] (ret_fast_syscall+0x0/0x28) +Exception stack(0xdd74dfa8 to 0xdd74dff0) +dfa0: 0004 000e2408 0001 000e2408 0004 +dfc0: 0004 000e2408 b6f36d60 0004 000e2408 0004 +dfe0: be9fc74c b6e991bb b6ed5af6 +irq event stamp: 0 +hardirqs last enabled at (0): [<>] 0x0 +hardirqs last disabled at (0): [] copy_process+0x520/0x14b8 +softirqs last enabled at (0): [] copy_process+0x520/0x14b8 +softirqs last disabled at (0): [<>] 0x0 +---[ end trace 22461a068edbf2c1 ]--- +[ cut here ] +WARNING: CPU: 0 PID: 556 at kernel/dma/remap.c:93 remap_allocator_free+0x20/0x30 +trying to free invalid coherent area: f39c52ba [...] +---[ end trace 22461a068edbf2c2 ]--- SMSC LAN8710/LAN8720 e9a0.ethernet-:00: attached PHY driver [SMSC LAN8710/LAN8720] (mii_bus:phy_addr=e9a0.ethernet-:00, irq=POLL) (the dirty is due to the need for "ARM: fix __get_user_check() in case uaccess_* calls are not inlined"). BTW, I cannot trigger the issue on r8a7791/koelsch, which also uses sh-eth, not even when disabling CONFIG_IOMMU_SUPPORT and CONFIG_ARM_LPAE (both are not set for the affected platforms). > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -541,15 +541,6 @@ static struct page **__iommu_dma_alloc_pages(struct > device *dev, > return pages; > } > > -static struct page **__iommu_dma_get_pages(void *cpu_addr) > -{ > - struct vm_struct *area = find_vm_area(cpu_addr); > - > - if (!area || !area->pages) This checks area->pages... > - return NULL; > - return area->pages; > -} > - > /** > * iommu_dma_alloc_remap - Allocate and map a buffer contiguous in IOVA space > * @dev: Device to allocate memory for. Must be a real device > @@ -938,7 +929,7 @@ static void __iommu_dma_free(struct device *dev, size_t > size, void *cpu_addr) > * If it the address is remapped, then it's either > non-coherent > * or highmem CMA, or an iommu_dma_alloc_remap() construction. > */ > - pages = __iommu_dma_get_pages(cpu_addr); > + pages = dma_common_find_pages(cpu_addr); > if (!pages) > page = vmalloc_to_page(cpu_addr); > dma_common_free_remap(cpu_addr, alloc_size); > @@ -1045,7 +1036,7 @@ static int iommu_dma_mmap(struct device *dev, struct > vm_area_struct *vma, > return -ENXIO; > > if (IS_ENABLED(CONFIG_DMA_REMAP) && is_vmalloc_addr(cpu_addr)) { > - struct page **pages = __iommu_dma_get_pages(cpu_addr); > + struct page **pages = dma_common_find_pages(cpu_addr); > > if (pages) > return __i
Re: [PATCH 3/4] dma-mapping: introduce a dma_common_find_pages helper
On 02/10/2019 13:05, Geert Uytterhoeven wrote: Hi Christoph, On Fri, Aug 30, 2019 at 8:30 AM Christoph Hellwig wrote: A helper to find the backing page array based on a virtual address. This also ensures we do the same vm_flags check everywhere instead of slightly different or missing ones in a few places. Signed-off-by: Christoph Hellwig This is now commit 5cf4537975bbd569 ("dma-mapping: introduce a dma_common_find_pages helper") in v5.4-rc1, and causes the following warning during s2ram on r8a7740/armadillo, r7s72100/rskrza1, and r7s9210/rza2mevb: sh-eth e9a0.ethernet eth0: Link is Down +[ cut here ] +WARNING: CPU: 0 PID: 556 at kernel/dma/remap.c:93 remap_allocator_free+0x20/0x30 +trying to free invalid coherent area: 6909579a +Modules linked in: +CPU: 0 PID: 556 Comm: s2ram Not tainted 5.3.0-rc6-armadillo-00027-g5cf4537975bbd569-dirty #113 +Hardware name: Generic R8A7740 (Flattened Device Tree) +[] (unwind_backtrace) from [] (show_stack+0x10/0x14) +[] (show_stack) from [] (__warn+0xec/0x104) +[] (__warn) from [] (warn_slowpath_fmt+0x44/0x6c) +[] (warn_slowpath_fmt) from [] (remap_allocator_free+0x20/0x30) +[] (remap_allocator_free) from [] (__arm_dma_free.constprop.2+0x114/0x144) +[] (__arm_dma_free.constprop.2) from [] (sh_eth_ring_free+0xb8/0x114) +[] (sh_eth_ring_free) from [] (sh_eth_close+0x68/0x8c) +[] (sh_eth_close) from [] (sh_eth_resume+0x44/0x90) +[] (sh_eth_resume) from [] (dpm_run_callback+0x64/0xdc) +[] (dpm_run_callback) from [] (device_resume+0xbc/0x180) +[] (device_resume) from [] (dpm_resume+0x124/0x1b0) +[] (dpm_resume) from [] (dpm_resume_end+0xc/0x18) +[] (dpm_resume_end) from [] (suspend_devices_and_enter+0x15c/0x5ac) +[] (suspend_devices_and_enter) from [] (pm_suspend+0x240/0x2f4) +[] (pm_suspend) from [] (state_store+0x54/0x8c) +[] (state_store) from [] (kernfs_fop_write+0x154/0x1c8) +[] (kernfs_fop_write) from [] (__vfs_write+0x28/0xe0) +[] (__vfs_write) from [] (vfs_write+0x98/0xbc) +[] (vfs_write) from [] (ksys_write+0x68/0xb4) +[] (ksys_write) from [] (ret_fast_syscall+0x0/0x28) +Exception stack(0xdd74dfa8 to 0xdd74dff0) +dfa0: 0004 000e2408 0001 000e2408 0004 +dfc0: 0004 000e2408 b6f36d60 0004 000e2408 0004 +dfe0: be9fc74c b6e991bb b6ed5af6 +irq event stamp: 0 +hardirqs last enabled at (0): [<>] 0x0 +hardirqs last disabled at (0): [] copy_process+0x520/0x14b8 +softirqs last enabled at (0): [] copy_process+0x520/0x14b8 +softirqs last disabled at (0): [<>] 0x0 +---[ end trace 22461a068edbf2c1 ]--- +[ cut here ] +WARNING: CPU: 0 PID: 556 at kernel/dma/remap.c:93 remap_allocator_free+0x20/0x30 +trying to free invalid coherent area: f39c52ba [...] +---[ end trace 22461a068edbf2c2 ]--- SMSC LAN8710/LAN8720 e9a0.ethernet-:00: attached PHY driver [SMSC LAN8710/LAN8720] (mii_bus:phy_addr=e9a0.ethernet-:00, irq=POLL) (the dirty is due to the need for "ARM: fix __get_user_check() in case uaccess_* calls are not inlined"). BTW, I cannot trigger the issue on r8a7791/koelsch, which also uses sh-eth, not even when disabling CONFIG_IOMMU_SUPPORT and CONFIG_ARM_LPAE (both are not set for the affected platforms). --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -541,15 +541,6 @@ static struct page **__iommu_dma_alloc_pages(struct device *dev, return pages; } -static struct page **__iommu_dma_get_pages(void *cpu_addr) -{ - struct vm_struct *area = find_vm_area(cpu_addr); - - if (!area || !area->pages) This checks area->pages... 32-bit Arm doesn't use iommu-dma, so I don't see how this could be relevant to the reported problem, but either way this "check" of area->pages... - return NULL; - return area->pages; -} - /** * iommu_dma_alloc_remap - Allocate and map a buffer contiguous in IOVA space * @dev: Device to allocate memory for. Must be a real device @@ -938,7 +929,7 @@ static void __iommu_dma_free(struct device *dev, size_t size, void *cpu_addr) * If it the address is remapped, then it's either non-coherent * or highmem CMA, or an iommu_dma_alloc_remap() construction. */ - pages = __iommu_dma_get_pages(cpu_addr); + pages = dma_common_find_pages(cpu_addr); if (!pages) page = vmalloc_to_page(cpu_addr); dma_common_free_remap(cpu_addr, alloc_size); @@ -1045,7 +1036,7 @@ static int iommu_dma_mmap(struct device *dev, struct vm_area_struct *vma, return -ENXIO; if (IS_ENABLED(CONFIG_DMA_REMAP) && is_vmalloc_addr(cpu_addr)) { - struct pag
Re: [PATCH 3/4] dma-mapping: introduce a dma_common_find_pages helper
Hi Geert, please try Linus' current tree. It has a fix from Andrey Smirnov for what looks the same problem that you reported.
Re: [PATCH 3/4] dma-mapping: introduce a dma_common_find_pages helper
Hi Christoph, On Sun, Oct 6, 2019 at 8:45 PM Christoph Hellwig wrote: > please try Linus' current tree. It has a fix from Andrey Smirnov > for what looks the same problem that you reported. Thanks, confirmed fixed by commit 2cf2aa6a69db0b17 ("dma-mapping: fix false positivse warnings in dma_common_free_remap()"). Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds