Re: [PATCH 6/8] dma-direct: turn ARCH_ZONE_DMA_BITS into a variable
Hi Christoph, thanks for the review. On Thu, 2019-08-01 at 16:04 +0200, Christoph Hellwig wrote: > A few nitpicks, otherwise this looks great: > > > @@ -201,7 +202,7 @@ static int __init mark_nonram_nosave(void) > > * everything else. GFP_DMA32 page allocations automatically fall back to > > * ZONE_DMA. > > * > > - * By using 31-bit unconditionally, we can exploit ARCH_ZONE_DMA_BITS to > > + * By using 31-bit unconditionally, we can exploit arch_zone_dma_bits to > > * inform the generic DMA mapping code. 32-bit only devices (if not > > handled > > * by an IOMMU anyway) will take a first dip into ZONE_NORMAL and get > > * otherwise served by ZONE_DMA. > > @@ -237,9 +238,18 @@ void __init paging_init(void) > > printk(KERN_DEBUG "Memory hole size: %ldMB\n", > >(long int)((top_of_ram - total_ram) >> 20)); > > > > + /* > > +* Allow 30-bit DMA for very limited Broadcom wifi chips on many > > +* powerbooks. > > +*/ > > + if (IS_ENABLED(CONFIG_PPC32)) > > + arch_zone_dma_bits = 30; > > + else > > + arch_zone_dma_bits = 31; > > + > > So the above unconditionally comment obviously isn't true any more, and > Ben also said for the recent ppc32 hack he'd prefer dynamic detection. > > Maybe Ben and or other ppc folks can chime in an add a patch to the series > to sort this out now that we have a dynamic ZONE_DMA threshold? Noted, for now I'll remove the comment. > > diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c > > index 59bdceea3737..40dfc9b4ee4c 100644 > > --- a/kernel/dma/direct.c > > +++ b/kernel/dma/direct.c > > @@ -19,9 +19,7 @@ > > * Most architectures use ZONE_DMA for the first 16 Megabytes, but > > * some use it for entirely different regions: > > */ > > -#ifndef ARCH_ZONE_DMA_BITS > > -#define ARCH_ZONE_DMA_BITS 24 > > -#endif > > +unsigned int arch_zone_dma_bits __ro_after_init = 24; > > I'd prefer to drop the arch_ prefix and just calls this zone_dma_bits. > In the long run we really need to find a way to just automatically set > this from the meminit code, but that is out of scope for this series. > For now can you please just update the comment above to say something > like: > > /* > * Most architectures use ZONE_DMA for the first 16 Megabytes, but some use it > * it for entirely different regions. In that case the arch code needs to > * override the variable below for dma-direct to work properly. > */ Ok perfect. signature.asc Description: This is a digitally signed message part
Re: [PATCH 6/8] dma-direct: turn ARCH_ZONE_DMA_BITS into a variable
A few nitpicks, otherwise this looks great: > @@ -201,7 +202,7 @@ static int __init mark_nonram_nosave(void) > * everything else. GFP_DMA32 page allocations automatically fall back to > * ZONE_DMA. > * > - * By using 31-bit unconditionally, we can exploit ARCH_ZONE_DMA_BITS to > + * By using 31-bit unconditionally, we can exploit arch_zone_dma_bits to > * inform the generic DMA mapping code. 32-bit only devices (if not handled > * by an IOMMU anyway) will take a first dip into ZONE_NORMAL and get > * otherwise served by ZONE_DMA. > @@ -237,9 +238,18 @@ void __init paging_init(void) > printk(KERN_DEBUG "Memory hole size: %ldMB\n", > (long int)((top_of_ram - total_ram) >> 20)); > > + /* > + * Allow 30-bit DMA for very limited Broadcom wifi chips on many > + * powerbooks. > + */ > + if (IS_ENABLED(CONFIG_PPC32)) > + arch_zone_dma_bits = 30; > + else > + arch_zone_dma_bits = 31; > + So the above unconditionally comment obviously isn't true any more, and Ben also said for the recent ppc32 hack he'd prefer dynamic detection. Maybe Ben and or other ppc folks can chime in an add a patch to the series to sort this out now that we have a dynamic ZONE_DMA threshold? > diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c > index 59bdceea3737..40dfc9b4ee4c 100644 > --- a/kernel/dma/direct.c > +++ b/kernel/dma/direct.c > @@ -19,9 +19,7 @@ > * Most architectures use ZONE_DMA for the first 16 Megabytes, but > * some use it for entirely different regions: > */ > -#ifndef ARCH_ZONE_DMA_BITS > -#define ARCH_ZONE_DMA_BITS 24 > -#endif > +unsigned int arch_zone_dma_bits __ro_after_init = 24; I'd prefer to drop the arch_ prefix and just calls this zone_dma_bits. In the long run we really need to find a way to just automatically set this from the meminit code, but that is out of scope for this series. For now can you please just update the comment above to say something like: /* * Most architectures use ZONE_DMA for the first 16 Megabytes, but some use it * it for entirely different regions. In that case the arch code needs to * override the variable below for dma-direct to work properly. */
[PATCH 6/8] dma-direct: turn ARCH_ZONE_DMA_BITS into a variable
Some architectures, notably arm64, are interested in tweaking this
depending on their runtime dma addressing limitations.
Signed-off-by: Nicolas Saenz Julienne
---
arch/powerpc/include/asm/page.h | 9 -
arch/powerpc/mm/mem.c | 14 --
arch/s390/include/asm/page.h| 2 --
arch/s390/mm/init.c | 1 +
include/linux/dma-direct.h | 2 ++
kernel/dma/direct.c | 8 +++-
6 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index 0d52f57fca04..73668a21ae78 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -319,13 +319,4 @@ struct vm_area_struct;
#endif /* __ASSEMBLY__ */
#include
-/*
- * Allow 30-bit DMA for very limited Broadcom wifi chips on many powerbooks.
- */
-#ifdef CONFIG_PPC32
-#define ARCH_ZONE_DMA_BITS 30
-#else
-#define ARCH_ZONE_DMA_BITS 31
-#endif
-
#endif /* _ASM_POWERPC_PAGE_H */
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 9191a66b3bc5..3792a998ca02 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -31,6 +31,7 @@
#include
#include
#include
+#include
#include
#include
@@ -201,7 +202,7 @@ static int __init mark_nonram_nosave(void)
* everything else. GFP_DMA32 page allocations automatically fall back to
* ZONE_DMA.
*
- * By using 31-bit unconditionally, we can exploit ARCH_ZONE_DMA_BITS to
+ * By using 31-bit unconditionally, we can exploit arch_zone_dma_bits to
* inform the generic DMA mapping code. 32-bit only devices (if not handled
* by an IOMMU anyway) will take a first dip into ZONE_NORMAL and get
* otherwise served by ZONE_DMA.
@@ -237,9 +238,18 @@ void __init paging_init(void)
printk(KERN_DEBUG "Memory hole size: %ldMB\n",
(long int)((top_of_ram - total_ram) >> 20));
+ /*
+* Allow 30-bit DMA for very limited Broadcom wifi chips on many
+* powerbooks.
+*/
+ if (IS_ENABLED(CONFIG_PPC32))
+ arch_zone_dma_bits = 30;
+ else
+ arch_zone_dma_bits = 31;
+
#ifdef CONFIG_ZONE_DMA
max_zone_pfns[ZONE_DMA] = min(max_low_pfn,
- 1UL << (ARCH_ZONE_DMA_BITS - PAGE_SHIFT));
+ 1UL << (arch_zone_dma_bits - PAGE_SHIFT));
#endif
max_zone_pfns[ZONE_NORMAL] = max_low_pfn;
#ifdef CONFIG_HIGHMEM
diff --git a/arch/s390/include/asm/page.h b/arch/s390/include/asm/page.h
index 823578c6b9e2..a4d38092530a 100644
--- a/arch/s390/include/asm/page.h
+++ b/arch/s390/include/asm/page.h
@@ -177,8 +177,6 @@ static inline int devmem_is_allowed(unsigned long pfn)
#define VM_DATA_DEFAULT_FLAGS (VM_READ | VM_WRITE | \
VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
-#define ARCH_ZONE_DMA_BITS 31
-
#include
#include
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index 20340a03ad90..07d93955d3e4 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -118,6 +118,7 @@ void __init paging_init(void)
sparse_memory_present_with_active_regions(MAX_NUMNODES);
sparse_init();
+ arch_zone_dma_bits = 31;
memset(max_zone_pfns, 0, sizeof(max_zone_pfns));
max_zone_pfns[ZONE_DMA] = PFN_DOWN(MAX_DMA_ADDRESS);
max_zone_pfns[ZONE_NORMAL] = max_low_pfn;
diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index adf993a3bd58..a1b353b77858 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -5,6 +5,8 @@
#include
#include
+extern unsigned int arch_zone_dma_bits;
+
#ifdef CONFIG_ARCH_HAS_PHYS_TO_DMA
#include
#else
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 59bdceea3737..40dfc9b4ee4c 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -19,9 +19,7 @@
* Most architectures use ZONE_DMA for the first 16 Megabytes, but
* some use it for entirely different regions:
*/
-#ifndef ARCH_ZONE_DMA_BITS
-#define ARCH_ZONE_DMA_BITS 24
-#endif
+unsigned int arch_zone_dma_bits __ro_after_init = 24;
static void report_addr(struct device *dev, dma_addr_t dma_addr, size_t size)
{
@@ -72,7 +70,7 @@ static gfp_t __dma_direct_optimal_gfp_mask(struct device
*dev, u64 dma_mask,
* Note that GFP_DMA32 and GFP_DMA are no ops without the corresponding
* zones.
*/
- if (*phys_mask <= DMA_BIT_MASK(ARCH_ZONE_DMA_BITS))
+ if (*phys_mask <= DMA_BIT_MASK(arch_zone_dma_bits))
return GFP_DMA;
if (*phys_mask <= DMA_BIT_MASK(32))
return GFP_DMA32;
@@ -387,7 +385,7 @@ int dma_direct_supported(struct device *dev, u64 mask)
u64 min_mask;
if (IS_ENABLED(CONFIG_ZONE_DMA))
- min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS);
+ min_mask = DMA_BIT_MASK(arch_zone_dma_bits);
else
min_mask = DMA_BIT_MASK(32);
--
2.22.0
