| Message ID | 1479288013-30945-1-git-send-email-m.szyprowski@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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.
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
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
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(-)