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.
Attachment:
signature.asc
Description: This is a digitally signed message part
