Message ID | 1403499924-11214-1-git-send-email-msalter@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Mon, Jun 23, 2014 at 01:05:24AM -0400, Mark Salter wrote: > +config ZONE_DMA > + bool "32-bit DMA memory allocation" > + def_bool y This should be either bool + default or def_bool. Not bool + def_bool.
On Mon, Jun 23, 2014 at 06:05:24AM +0100, Mark Salter wrote: > Commit 19e7640d1f (arm64: Replace ZONE_DMA32 with ZONE_DMA) > moves support for 32-bit DMA addressing into ZONE_DMA and renames > CONFIG_ZONE_DMA32 to CONFIG_ZONE_DMA. > > Commit 2d5a5612bc (arm64: Limit the CMA buffer to 32-bit if ZONE_DMA) > forces the CMA buffer to be 32-bit addressable if CONFIG_ZONE_DMA is > defined. > > These two patches pose a problem for platforms which have no 32-bit > addressable DRAM. It's actually the bus/dma address that needs to be 32-bit rather than the DRAM as seen by the CPU (which can be beyond 32-bit like the Keystone platform). > If CONFIG_ZONE_DMA is turned on for such platforms, > CMA is unable to reserve a buffer for allocations. CONFIG_ZONE_DMA is > not user settable however, so there is no way to turn it off without > editing arch/arm64/Kconfig. Even if one edits Kconfig to turn off > CONFIG_ZONE_DMA, the kernel fails to build with such a config: > > arch/arm64/mm/init.c: In function ‘zone_sizes_init’: > arch/arm64/mm/init.c:76:13: error: ‘ZONE_DMA’ undeclared (first use in this function) > zone_size[ZONE_DMA] = max_dma - min; > ^ > > This patch makes CONFIG_ZONE_DMA user settable and fixes the kernel > build when it is turned off. The reason I left CONFIG_ZONE_DMA not user settable is because on arm64 we aim for single Image by default. So ZONE_DMA would most likely be needed on at least one platform. Even with ZONE_DMA it's just a guess without proper system topology description. dma_to_phys() just takes a NULL argument for device when trying to guess the physical address for a 32-bit DMA mask (and 32-bit devices may have some physical offset wired already). With the CMA fix, does the kernel work properly with a zero sized DMA zone? Any GFP_DMA allocations will fail, including the swiotlb bounce buffer. It may be fine if no devices require 32-bit DMA memory but I wonder whether on such platforms it would be better to simply add all the memory to ZONE_DMA. Which gets us back to finding a way for describing such system topology in hardware. We may be able to find a way with DT but unlikely for ACPI. My proposal (in the absence of any kind of description) is to still create a ZONE_DMA if we have DMA memory below 32-bit, otherwise just add everything (>32-bit) to ZONE_DMA. Basically an extension from your CMA patch, make dma_phys_limit static in that file and set it to memblock_end_of_DRAM() if no 32-bit DMA. Re-use it in the zone_sizes_init() function for ZONE_DMA (maybe with a pr_info for no 32-bit only DMA zone).
On Mon, 2014-06-23 at 12:09 +0100, Catalin Marinas wrote: > On Mon, Jun 23, 2014 at 06:05:24AM +0100, Mark Salter wrote: > > Commit 19e7640d1f (arm64: Replace ZONE_DMA32 with ZONE_DMA) > > moves support for 32-bit DMA addressing into ZONE_DMA and renames > > CONFIG_ZONE_DMA32 to CONFIG_ZONE_DMA. > > > > Commit 2d5a5612bc (arm64: Limit the CMA buffer to 32-bit if ZONE_DMA) > > forces the CMA buffer to be 32-bit addressable if CONFIG_ZONE_DMA is > > defined. > > > > These two patches pose a problem for platforms which have no 32-bit > > addressable DRAM. > > It's actually the bus/dma address that needs to be 32-bit rather than > the DRAM as seen by the CPU (which can be beyond 32-bit like the > Keystone platform). Ah, right. > > > If CONFIG_ZONE_DMA is turned on for such platforms, > > CMA is unable to reserve a buffer for allocations. CONFIG_ZONE_DMA is > > not user settable however, so there is no way to turn it off without > > editing arch/arm64/Kconfig. Even if one edits Kconfig to turn off > > CONFIG_ZONE_DMA, the kernel fails to build with such a config: > > > > arch/arm64/mm/init.c: In function ‘zone_sizes_init’: > > arch/arm64/mm/init.c:76:13: error: ‘ZONE_DMA’ undeclared (first use in this function) > > zone_size[ZONE_DMA] = max_dma - min; > > ^ > > > > This patch makes CONFIG_ZONE_DMA user settable and fixes the kernel > > build when it is turned off. > > The reason I left CONFIG_ZONE_DMA not user settable is because on arm64 > we aim for single Image by default. So ZONE_DMA would most likely be > needed on at least one platform. Even with ZONE_DMA it's just a guess > without proper system topology description. dma_to_phys() just takes a > NULL argument for device when trying to guess the physical address for a > 32-bit DMA mask (and 32-bit devices may have some physical offset wired > already). > > With the CMA fix, does the kernel work properly with a zero sized DMA > zone? Any GFP_DMA allocations will fail, including the swiotlb bounce > buffer. It may be fine if no devices require 32-bit DMA memory but I > wonder whether on such platforms it would be better to simply add > all the memory to ZONE_DMA. The kernel works, but there are no devices needing GFP_DMA. And yes, the swiotlb init fails because of no GFP_DMA. But that has always been the case on platforms with no 32-bit memory. > > Which gets us back to finding a way for describing such system topology > in hardware. We may be able to find a way with DT but unlikely for ACPI. > > My proposal (in the absence of any kind of description) is to still > create a ZONE_DMA if we have DMA memory below 32-bit, otherwise just add > everything (>32-bit) to ZONE_DMA. Basically an extension from your CMA > patch, make dma_phys_limit static in that file and set it to > memblock_end_of_DRAM() if no 32-bit DMA. Re-use it in the > zone_sizes_init() function for ZONE_DMA (maybe with a pr_info for no > 32-bit only DMA zone). There's a performance issue with all memory being in ZONE_DMA. It means all normal allocations will fail on ZONE_NORMAL and then have to fall back to ZONE_DMA. It would be better to put some percentage of memory in ZONE_DMA.
On Mon, Jun 23, 2014 at 02:17:03PM +0100, Mark Salter wrote: > On Mon, 2014-06-23 at 12:09 +0100, Catalin Marinas wrote: > > My proposal (in the absence of any kind of description) is to still > > create a ZONE_DMA if we have DMA memory below 32-bit, otherwise just add > > everything (>32-bit) to ZONE_DMA. Basically an extension from your CMA > > patch, make dma_phys_limit static in that file and set it to > > memblock_end_of_DRAM() if no 32-bit DMA. Re-use it in the > > zone_sizes_init() function for ZONE_DMA (maybe with a pr_info for no > > 32-bit only DMA zone). > > There's a performance issue with all memory being in ZONE_DMA. It means > all normal allocations will fail on ZONE_NORMAL and then have to fall > back to ZONE_DMA. It would be better to put some percentage of memory > in ZONE_DMA. Is the performance penalty real or just theoretical? I haven't run any benchmarks myself.
On Tue, 2014-06-24 at 15:14 +0100, Catalin Marinas wrote: > On Mon, Jun 23, 2014 at 02:17:03PM +0100, Mark Salter wrote: > > On Mon, 2014-06-23 at 12:09 +0100, Catalin Marinas wrote: > > > My proposal (in the absence of any kind of description) is to still > > > create a ZONE_DMA if we have DMA memory below 32-bit, otherwise just add > > > everything (>32-bit) to ZONE_DMA. Basically an extension from your CMA > > > patch, make dma_phys_limit static in that file and set it to > > > memblock_end_of_DRAM() if no 32-bit DMA. Re-use it in the > > > zone_sizes_init() function for ZONE_DMA (maybe with a pr_info for no > > > 32-bit only DMA zone). > > > > There's a performance issue with all memory being in ZONE_DMA. It means > > all normal allocations will fail on ZONE_NORMAL and then have to fall > > back to ZONE_DMA. It would be better to put some percentage of memory > > in ZONE_DMA. > > Is the performance penalty real or just theoretical? I haven't run any > benchmarks myself. > It is real insofar as you must eat cycles eliminating ZONE_NORMAL from consideration in the page allocation hot path. How much that really costs, I don't know. But it seems like it could be easily avoided by limiting ZONE_DMA size. Is there any reason it needs to be larger than 4GiB?
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index c8b1d0b..f669597 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -99,9 +99,6 @@ config GENERIC_CSUM config GENERIC_CALIBRATE_DELAY def_bool y -config ZONE_DMA - def_bool y - config ARCH_DMA_ADDR_T_64BIT def_bool y @@ -211,6 +208,14 @@ config HOTPLUG_CPU Say Y here to experiment with turning CPUs off and on. CPUs can be controlled through /sys/devices/system/cpu. +config ZONE_DMA + bool "32-bit DMA memory allocation" + def_bool y + help + 32-bit DMA memory allocation supports devices limited to 32-bit + DMA addressing to allocate within the first 4GiB of address space. + Disable if no such devices will be used. + source kernel/Kconfig.preempt config HZ diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index f43db8a..c5415e2 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -69,12 +69,14 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max) memset(zone_size, 0, sizeof(zone_size)); /* 4GB maximum for 32-bit only capable devices */ - if (IS_ENABLED(CONFIG_ZONE_DMA)) { +#ifdef CONFIG_ZONE_DMA + { unsigned long max_dma_phys = (unsigned long)(dma_to_phys(NULL, DMA_BIT_MASK(32)) + 1); max_dma = max(min, min(max, max_dma_phys >> PAGE_SHIFT)); zone_size[ZONE_DMA] = max_dma - min; } +#endif zone_size[ZONE_NORMAL] = max - max_dma; memcpy(zhole_size, zone_size, sizeof(zhole_size)); @@ -86,10 +88,12 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max) if (start >= max) continue; - if (IS_ENABLED(CONFIG_ZONE_DMA) && start < max_dma) { +#ifdef CONFIG_ZONE_DMA + if (start < max_dma) { unsigned long dma_end = min(end, max_dma); zhole_size[ZONE_DMA] -= dma_end - start; } +#endif if (end > max_dma) { unsigned long normal_end = min(end, max);
Commit 19e7640d1f (arm64: Replace ZONE_DMA32 with ZONE_DMA) moves support for 32-bit DMA addressing into ZONE_DMA and renames CONFIG_ZONE_DMA32 to CONFIG_ZONE_DMA. Commit 2d5a5612bc (arm64: Limit the CMA buffer to 32-bit if ZONE_DMA) forces the CMA buffer to be 32-bit addressable if CONFIG_ZONE_DMA is defined. These two patches pose a problem for platforms which have no 32-bit addressable DRAM. If CONFIG_ZONE_DMA is turned on for such platforms, CMA is unable to reserve a buffer for allocations. CONFIG_ZONE_DMA is not user settable however, so there is no way to turn it off without editing arch/arm64/Kconfig. Even if one edits Kconfig to turn off CONFIG_ZONE_DMA, the kernel fails to build with such a config: arch/arm64/mm/init.c: In function ‘zone_sizes_init’: arch/arm64/mm/init.c:76:13: error: ‘ZONE_DMA’ undeclared (first use in this function) zone_size[ZONE_DMA] = max_dma - min; ^ This patch makes CONFIG_ZONE_DMA user settable and fixes the kernel build when it is turned off. Signed-off-by: Mark Salter <msalter@redhat.com> --- arch/arm64/Kconfig | 11 ++++++++--- arch/arm64/mm/init.c | 8 ++++++-- 2 files changed, 14 insertions(+), 5 deletions(-)