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

[John Stultz]

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