diff mbox

ARM: dma-mapping: preallocate DMA-debug hash tables in pure_initcall

Message ID 1479204295-29773-1-git-send-email-m.szyprowski@samsung.com
State Superseded
Headers show

Commit Message

Marek Szyprowski Nov. 15, 2016, 10:04 a.m. UTC
fs_initcall is definitely too late to initialize DMA-debug hash tables,
because some drivers might get probed and use DMA mapping framework
already in core_initcall. Late initialization of DMA-debug results in
false warning about accessing memory, that was not allocated, like this
one:
------------[ cut here ]------------
WARNING: CPU: 5 PID: 1 at lib/dma-debug.c:1104 check_unmap+0xa1c/0xe50
exynos-sysmmu 10a60000.sysmmu: DMA-API: device driver tries to free DMA memory it has not allocated [device
address=0x000000006ebd0000] [size=16384 bytes]
Modules linked in:
CPU: 5 PID: 1 Comm: swapper/0 Not tainted 4.9.0-rc5-00028-g39dde3d-dirty #44
Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[<c0119dd4>] (unwind_backtrace) from [<c01122bc>] (show_stack+0x20/0x24)
[<c01122bc>] (show_stack) from [<c062714c>] (dump_stack+0x84/0xa0)
[<c062714c>] (dump_stack) from [<c0132560>] (__warn+0x14c/0x180)
[<c0132560>] (__warn) from [<c01325dc>] (warn_slowpath_fmt+0x48/0x50)
[<c01325dc>] (warn_slowpath_fmt) from [<c06814f8>] (check_unmap+0xa1c/0xe50)
[<c06814f8>] (check_unmap) from [<c06819c4>] (debug_dma_unmap_page+0x98/0xc8)
[<c06819c4>] (debug_dma_unmap_page) from [<c076c3e8>] (exynos_iommu_domain_free+0x158/0x380)
[<c076c3e8>] (exynos_iommu_domain_free) from [<c0764a30>] (iommu_domain_free+0x34/0x60)
[<c0764a30>] (iommu_domain_free) from [<c011f168>] (release_iommu_mapping+0x30/0xb8)
[<c011f168>] (release_iommu_mapping) from [<c011f23c>] (arm_iommu_release_mapping+0x4c/0x50)
[<c011f23c>] (arm_iommu_release_mapping) from [<c0b061ac>] (s5p_mfc_probe+0x640/0x80c)
[<c0b061ac>] (s5p_mfc_probe) from [<c07e6750>] (platform_drv_probe+0x70/0x148)
[<c07e6750>] (platform_drv_probe) from [<c07e25c0>] (driver_probe_device+0x12c/0x6b0)
[<c07e25c0>] (driver_probe_device) from [<c07e2c6c>] (__driver_attach+0x128/0x17c)
[<c07e2c6c>] (__driver_attach) from [<c07df74c>] (bus_for_each_dev+0x88/0xc8)
[<c07df74c>] (bus_for_each_dev) from [<c07e1b6c>] (driver_attach+0x34/0x58)
[<c07e1b6c>] (driver_attach) from [<c07e1350>] (bus_add_driver+0x18c/0x32c)
[<c07e1350>] (bus_add_driver) from [<c07e4198>] (driver_register+0x98/0x148)
[<c07e4198>] (driver_register) from [<c07e5cb0>] (__platform_driver_register+0x58/0x74)
[<c07e5cb0>] (__platform_driver_register) from [<c174cb30>] (s5p_mfc_driver_init+0x1c/0x20)
[<c174cb30>] (s5p_mfc_driver_init) from [<c0102690>] (do_one_initcall+0x64/0x258)
[<c0102690>] (do_one_initcall) from [<c17014c0>] (kernel_init_freeable+0x3d0/0x4d0)
[<c17014c0>] (kernel_init_freeable) from [<c116eeb4>] (kernel_init+0x18/0x134)
[<c116eeb4>] (kernel_init) from [<c010bbd8>] (ret_from_fork+0x14/0x3c)
---[ end trace dc54c54bd3581296 ]---

This patch moves initialization of DMA-debug to pure_initcall.

Reported-by: Seung-Woo Kim <sw0312.kim@samsung.com>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

---
 arch/arm/mm/dma-mapping.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
1.9.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Comments

Arnd Bergmann Nov. 15, 2016, 10:31 a.m. UTC | #1
On Tuesday, November 15, 2016 11:04:55 AM CET Marek Szyprowski wrote:
> fs_initcall is definitely too late to initialize DMA-debug hash tables,

> because some drivers might get probed and use DMA mapping framework

> already in core_initcall. Late initialization of DMA-debug results in

> false warning about accessing memory, that was not allocated, like this

> one:


To ask the obvious question: what driver does DMA allocations in a
core_initcall, and have you tried to change that first?

	Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Marek Szyprowski Nov. 15, 2016, 10:39 a.m. UTC | #2
On 2016-11-15 11:31, Arnd Bergmann wrote:
> On Tuesday, November 15, 2016 11:04:55 AM CET Marek Szyprowski wrote:

>> fs_initcall is definitely too late to initialize DMA-debug hash tables,

>> because some drivers might get probed and use DMA mapping framework

>> already in core_initcall. Late initialization of DMA-debug results in

>> false warning about accessing memory, that was not allocated, like this

>> one:

> To ask the obvious question: what driver does DMA allocations in a

> core_initcall, and have you tried to change that first?


See the attached stack trace.

Exynos IOMMU driver does that in its domain_alloc implementation (to 
allocate
first level of PTE) and I see no easy way to avoid that, as iommu 
domains are
allocated very early (as well as the whole IOMMU initialization and 
attaching
to the devices). Exynos IOMMU driver has to use DMA-mapping API for PTE
management, because the IOMMU controllers are not coherent with system 
CPU and
there is no other way to ensure proper CPU cache management.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Arnd Bergmann Nov. 15, 2016, 11:18 a.m. UTC | #3
On Tuesday, November 15, 2016 11:39:17 AM CET Marek Szyprowski wrote:
> On 2016-11-15 11:31, Arnd Bergmann wrote:

> > On Tuesday, November 15, 2016 11:04:55 AM CET Marek Szyprowski wrote:

> >> fs_initcall is definitely too late to initialize DMA-debug hash tables,

> >> because some drivers might get probed and use DMA mapping framework

> >> already in core_initcall. Late initialization of DMA-debug results in

> >> false warning about accessing memory, that was not allocated, like this

> >> one:

> > To ask the obvious question: what driver does DMA allocations in a

> > core_initcall, and have you tried to change that first?

> 

> See the attached stack trace.

> 

> Exynos IOMMU driver does that in its domain_alloc implementation (to 

> allocate

> first level of PTE) and I see no easy way to avoid that, as iommu 

> domains are

> allocated very early (as well as the whole IOMMU initialization and 

> attaching

> to the devices). Exynos IOMMU driver has to use DMA-mapping API for PTE

> management, because the IOMMU controllers are not coherent with system 

> CPU and

> there is no other way to ensure proper CPU cache management.


Could the allocation be deferred until the first user of the IOMMU
comes up?

	Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Russell King (Oracle) Nov. 15, 2016, 12:40 p.m. UTC | #4
On Tue, Nov 15, 2016 at 11:04:55AM +0100, Marek Szyprowski wrote:
> fs_initcall is definitely too late to initialize DMA-debug hash tables,

> because some drivers might get probed and use DMA mapping framework

> already in core_initcall. Late initialization of DMA-debug results in

> false warning about accessing memory, that was not allocated, like this

> one:


I think using pure_initcall() is not justified here - and if you read
the comments in include/linux/init.h, you're going against its
documented purpose - it says that it exists only to allow variables
that could not be statically initialised to be so.

It's also wrong, because debugfs gets initialised at core_initcall()
time, which happens after pure_initcall(), and dma_debug_init() calls
debugfs functions.

So, I really don't like this patch - it's subsituting one initcall
ordering issue for a different ordering issue.

Core code (which includes arch/arm/mm/) gets linked before drivers, so
moving this to be a core_initcall() would result in it being initialised
before drivers - and it actually makes total sense that this is a core
initcall.

It looks like this is safe from the debugfs perspective as well -
debugfs also gets initialised at core_initcall(), and that is earlier
than arch code in the link order, so debugfs should get initialised
first and then the DMA debug.

So, I'll accept a patch to change this from fs_initcall() to
core_initcall().

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff mbox

Patch

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index ab4f745..d1abbcf 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1167,7 +1167,7 @@  static int __init dma_debug_do_init(void)
 	dma_debug_init(PREALLOC_DMA_DEBUG_ENTRIES);
 	return 0;
 }
-fs_initcall(dma_debug_do_init);
+pure_initcall(dma_debug_do_init);
 
 #ifdef CONFIG_ARM_DMA_USE_IOMMU