[PATCH] ARM64: dma-mapping: preallocate DMA-debug hash tables in core_initcall

[PATCH] ARM64: dma-mapping: preallocate DMA-debug hash tables in core_initcall

[Marek Szyprowski]

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. This issue
has been observed on ARM 32bit, but the same driver can be used also on
ARM64.

This patch moves initialization of DMA-debug to core_initcall. This is
safe from the initialization perspective. dma_debug_do_init() internally
calls debugfs functions and debugfs also gets initialised at
core_initcall(), and that is earlier than arch code in the link order,
so it will get initialized just before the DMA-debug.

Reported-by: Seung-Woo Kim <sw0312.kim@samsung.com>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
For more details on this issue, see the patch for ARM 32bit arch:
https://www.spinics.net/lists/arm-kernel/msg542721.html
https://www.spinics.net/lists/arm-kernel/msg542782.html
---
 arch/arm64/mm/dma-mapping.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 3f74d0d..8653426 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -538,7 +538,7 @@ static int __init dma_debug_do_init(void)
 	dma_debug_init(PREALLOC_DMA_DEBUG_ENTRIES);
 	return 0;
 }
-fs_initcall(dma_debug_do_init);
+core_initcall(dma_debug_do_init);
 
 
 #ifdef CONFIG_IOMMU_DMA
-- 
1.9.1

[PATCH] ARM64: dma-mapping: preallocate DMA-debug hash tables in core_initcall

[Robin Murphy]

On 16/11/16 09:20, 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. This issue
> has been observed on ARM 32bit, but the same driver can be used also on
> ARM64.
> 
> This patch moves initialization of DMA-debug to core_initcall. This is
> safe from the initialization perspective. dma_debug_do_init() internally
> calls debugfs functions and debugfs also gets initialised at
> core_initcall(), and that is earlier than arch code in the link order,
> so it will get initialized just before the DMA-debug.
> 
> Reported-by: Seung-Woo Kim <sw0312.kim@samsung.com>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

Acked-by: Robin Murphy <robin.murphy@arm.com>

Cheers Marek - as it happens, the ARM SMMU drivers (and presumably the
Mediatek one) do hit this on arm64 since I added the DMA API support to
the io-pgtable code, I've just been quietly ignoring it in the hope we'd
get the probe-deferral stuff done before anyone else noticed.

Robin.

> ---
> For more details on this issue, see the patch for ARM 32bit arch:
> https://www.spinics.net/lists/arm-kernel/msg542721.html
> https://www.spinics.net/lists/arm-kernel/msg542782.html
> ---
>  arch/arm64/mm/dma-mapping.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index 3f74d0d..8653426 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -538,7 +538,7 @@ static int __init dma_debug_do_init(void)
>  	dma_debug_init(PREALLOC_DMA_DEBUG_ENTRIES);
>  	return 0;
>  }
> -fs_initcall(dma_debug_do_init);
> +core_initcall(dma_debug_do_init);
>  
>  
>  #ifdef CONFIG_IOMMU_DMA
> 

[PATCH] ARM64: dma-mapping: preallocate DMA-debug hash tables in core_initcall

[Catalin Marinas]

On Wed, Nov 16, 2016 at 10:20:13AM +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. This issue
> has been observed on ARM 32bit, but the same driver can be used also on
> ARM64.
> 
> This patch moves initialization of DMA-debug to core_initcall. This is
> safe from the initialization perspective. dma_debug_do_init() internally
> calls debugfs functions and debugfs also gets initialised at
> core_initcall(), and that is earlier than arch code in the link order,
> so it will get initialized just before the DMA-debug.

Do we really want to rely on the link order within an initcall level?
What guarantees this?

I hope someone sorts out the deferred probe or some other dependency
detection mechanism to address this issue. But in the meantime I
wouldn't merge a patch which relies on just the link order.

-- 
Catalin

[PATCH] ARM64: dma-mapping: preallocate DMA-debug hash tables in core_initcall

[Marek Szyprowski]

Hi Catalin,


On 2016-11-16 13:39, Catalin Marinas wrote:
> On Wed, Nov 16, 2016 at 10:20:13AM +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. This issue
>> has been observed on ARM 32bit, but the same driver can be used also on
>> ARM64.
>>
>> This patch moves initialization of DMA-debug to core_initcall. This is
>> safe from the initialization perspective. dma_debug_do_init() internally
>> calls debugfs functions and debugfs also gets initialised at
>> core_initcall(), and that is earlier than arch code in the link order,
>> so it will get initialized just before the DMA-debug.
> Do we really want to rely on the link order within an initcall level?
> What guarantees this?

There are many places in the kernel which rely on link order and I'm 
convinced
that calling initcalls in link order is guaranteed.

> I hope someone sorts out the deferred probe or some other dependency
> detection mechanism to address this issue. But in the meantime I
> wouldn't merge a patch which relies on just the link order.

This has nothing to deferred probe. This patch is related to initialization
of dma-debug framework. In my initial submission for ARM arch I proposed
pure_initcall to have this infrastructure available as early as possible,
but Russell pointed that dma-debug depends on debugfs initialization, so
it should be initialized after it. He also pointed that core_initcall will
be fine for this.

Please also note that dt devices are also populated from core_initcall and
drivers can then bind to them and try to use dma-mapping api, what results
in false warnings about using uninitialized memory as dma-debug framework
is unable to track allocations done before its initialization.

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