Re: [PATCH] arm64: mm: fix zone_dma_limit calculation

[Yang Shi]

On 11/25/24 10:27 PM, Baruch Siach wrote:
> Hi Yang,
>
> On Mon, Nov 25 2024, Yang Shi wrote:
> > The commit ba0fb44aed47 ("dma-mapping: replace zone_dma_bits by
> > zone_dma_limit") changed how zone_dma_limit was calculated.  Now it
> > returns the memsize limit in IORT or device tree instead of U32_MAX if
> > the memsize limit is greater than U32_MAX.
> Can you give a concrete example of memory layout and dma-ranges that
> demonstrates this issue?

Our 2 sockets system has physical memory starts at 0x0 on node 0 and 
0x200000000000 on node 1. The memory size limit defined in IORT is 0x30 
(48 bits).

The DMA zone is:

pages free     887722
        boost    0
        min      229
        low      1108
        high     1987
        promo    2866
        spanned  983040
        present  982034
        managed  903238
        cma      16384
        protection: (0, 0, 124824, 0, 0)
 start_pfn:           65536

When allocating DMA buffer, dma_direct_optimal_gfp_mask() is called to 
determine the proper zone constraints. If the phys_limit is less than 
zone_dma_limit, it will use GFP_DMA. But zone_dma_limit is 
0xffffffffffff on v6.12 instead of 4G prior v6.12, it means all DMA 
buffer allocation will go to DMA zone even though the devices don't 
require it.

DMA zone is on node 0, so we saw excessive remote access on 2 sockets 
system.

> > This resulted in DMA allocations may use GFP_DMA even though the devices
> > don't require it.  It caused regression on our two sockets systems due
> > to excessive remote memory access.
> That is, DMA zone used to cover all memory before commit ba0fb44aed47,
> but now DMA zone is limited to the smallest dma-ranges. Is that correct?

The physical addr range for DMA zone is correct, the problem is wrong 
zone_dma_limit. Before commit ba0fb44aed47 zone_dma_limit was 4G, after 
it it is the whole memory even though DMA zone just covers low 4G. 
Thanks, Yang
> Thanks,
> baruch
>
> > Fixes: ba0fb44aed47 ("dma-mapping: replace zone_dma_bits by zone_dma_limit")
> > Cc: <stable@xxxxxxxxxxxxxxx>    [6.12+]
> > Reported-by: Yutang Jiang <jiangyutang@xxxxxxxxxxxxxxxxxxxxxx>
> > Tested-by: Yutang Jiang <jiangyutang@xxxxxxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Yang Shi <yang@xxxxxxxxxxxxxxxxxxxxxx>
> > ---
> >   arch/arm64/mm/init.c | 17 ++++++++---------
> >   1 file changed, 8 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> > index d21f67d67cf5..ccdef53872a0 100644
> > --- a/arch/arm64/mm/init.c
> > +++ b/arch/arm64/mm/init.c
> > @@ -117,15 +117,6 @@ static void __init arch_reserve_crashkernel(void)
> >   
> >   static phys_addr_t __init max_zone_phys(phys_addr_t zone_limit)
> >   {
> > -	/**
> > -	 * Information we get from firmware (e.g. DT dma-ranges) describe DMA
> > -	 * bus constraints. Devices using DMA might have their own limitations.
> > -	 * Some of them rely on DMA zone in low 32-bit memory. Keep low RAM
> > -	 * DMA zone on platforms that have RAM there.
> > -	 */
> > -	if (memblock_start_of_DRAM() < U32_MAX)
> > -		zone_limit = min(zone_limit, U32_MAX);
> > -
> >   	return min(zone_limit, memblock_end_of_DRAM() - 1) + 1;
> >   }
> >   
> > @@ -141,6 +132,14 @@ static void __init zone_sizes_init(void)
> >   	acpi_zone_dma_limit = acpi_iort_dma_get_max_cpu_address();
> >   	dt_zone_dma_limit = of_dma_get_max_cpu_address(NULL);
> >   	zone_dma_limit = min(dt_zone_dma_limit, acpi_zone_dma_limit);
> > +	/*
> > +	 * Information we get from firmware (e.g. DT dma-ranges) describe DMA
> > +	 * bus constraints. Devices using DMA might have their own limitations.
> > +	 * Some of them rely on DMA zone in low 32-bit memory. Keep low RAM
> > +	 * DMA zone on platforms that have RAM there.
> > +	 */
> > +	if (memblock_start_of_DRAM() < U32_MAX)
> > +		zone_dma_limit = min(zone_dma_limit, U32_MAX);
> >   	arm64_dma_phys_limit = max_zone_phys(zone_dma_limit);
> >   	max_zone_pfns[ZONE_DMA] = PFN_DOWN(arm64_dma_phys_limit);
> >   #endif