Re: [PATCH 6/8] dma-direct: turn ARCH_ZONE_DMA_BITS into a variable
From: Nicolas Saenz Julienne
Date: Thu Aug 01 2019 - 12:00:33 EST
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
> * 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.
Description: This is a digitally signed message part