Re: [PATCH v6 3/4] arm64: use both ZONE_DMA and ZONE_DMA32

From: John Stultz
Date: Tue Dec 03 2019 - 01:03:33 EST


On Mon, Dec 2, 2019 at 9:38 PM John Stultz <john.stultz@xxxxxxxxxx> wrote:
> On Mon, Dec 2, 2019 at 9:08 PM John Stultz <john.stultz@xxxxxxxxxx> wrote:
> > Hey Nicolas,
> > Testing the db845c with linus/master, I found a regression causing
> > system hangs in early boot:
...
> > In the above log:
> > [ 0.000000] Built 1 zonelists, mobility grouping on. Total pages: -188245
> > looks the most suspect, and going back to the working a573cdd7973d +
> > build fix I see:
> > [ 0.000000] Built 1 zonelists, mobility grouping on. Total pages: 957419
> >
> > Do you have any suggestions for what might be going wrong?
>
> Digging further, it seems the error is found in calculate_node_totalpages()
> real_size = size - zone_absent_pages_in_node(pgdat->node_id, i,
> node_start_pfn, node_end_pfn,
> zholes_size);
>
> Where for zone DMA32 size is 262144, but real_size is calculated as -883520.
>
> I've not traced through to figure out why zone_absent_pages_in_node is
> coming up with such a large number yet, but I'm about to crash so I
> wanted to share.

Ok, narrowing it down further, it seems its the following bit from the patch:

> @@ -201,13 +212,18 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
> struct memblock_region *reg;
> unsigned long zone_size[MAX_NR_ZONES], zhole_size[MAX_NR_ZONES];
> unsigned long max_dma32 = min;
> + unsigned long max_dma = min;
>
> memset(zone_size, 0, sizeof(zone_size));
>
> - /* 4GB maximum for 32-bit only capable devices */
> +#ifdef CONFIG_ZONE_DMA
> + max_dma = PFN_DOWN(arm64_dma_phys_limit);
> + zone_size[ZONE_DMA] = max_dma - min;
> + max_dma32 = max_dma;
> +#endif
> #ifdef CONFIG_ZONE_DMA32
> max_dma32 = PFN_DOWN(arm64_dma32_phys_limit);
> - zone_size[ZONE_DMA32] = max_dma32 - min;
> + zone_size[ZONE_DMA32] = max_dma32 - max_dma;
> #endif
> zone_size[ZONE_NORMAL] = max - max_dma32;
>
> @@ -219,11 +235,17 @@ static void __init zone_sizes_init(unsigned long min, unsigned long max)
>
> if (start >= max)
> continue;
> -
> +#ifdef CONFIG_ZONE_DMA
> + if (start < max_dma) {
> + unsigned long dma_end = min_not_zero(end, max_dma);
> + zhole_size[ZONE_DMA] -= dma_end - start;
> + }
> +#endif
> #ifdef CONFIG_ZONE_DMA32
> if (start < max_dma32) {
> - unsigned long dma_end = min(end, max_dma32);
> - zhole_size[ZONE_DMA32] -= dma_end - start;
> + unsigned long dma32_end = min(end, max_dma32);
> + unsigned long dma32_start = max(start, max_dma);
> + zhole_size[ZONE_DMA32] -= dma32_end - dma32_start;
> }
> #endif
> if (end > max_dma32) {

The zhole_sizes end up being:
zhole_size: DMA: 67671, DMA32: 1145664 NORMAL: 0

This seems to be due to dma32_start being calculated as 786432 each
time - I'm guessing that's the max_dma value.
Where dma32_end is around 548800, but changes each iteration (so we
end up subtracting a negative value each pass, growing the size).

thanks
-john