Re: [PATCH 3/3] dma-pool: limit DMA and DMA32 zone size pools

From: Michal Hocko
Date: Wed Aug 17 2022 - 08:54:02 EST


On Wed 17-08-22 08:06:47, Christoph Hellwig wrote:
> Limit the sizing of the atomic pools for allocations from
> ZONE_DMA and ZONE_DMA32 based on the number of pages actually
> present in those zones instead of the total memory.
>
> Fixes: c84dc6e68a1d ("dma-pool: add additional coherent pools to map to gfp mask")
> Reported-by: Michal Hocko <mhocko@xxxxxxxx>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>

Yes this is much more reasonable estimation. Especially for the DMA
zone. I still believe some downscaling would make sense but as said
earlier nothing to really insist on. Also you likely want to have
pr_warn for non-poppulated DMA zone as pointed in other thread[1].

Other than that looks good to me. FWIW
Acked-by: Michal Hocko <mhocko@xxxxxxxx>

[1] https://lore.kernel.org/all/YvyN/vI7+cTv0geS@xxxxxxxxxxxxxx/?q=http%3A%2F%2Flkml.kernel.org%2Fr%2FYvyN%2FvI7%2BcTv0geS%40dhcp22.suse.cz

> ---
> kernel/dma/pool.c | 40 +++++++++++++++++++++++++++++-----------
> 1 file changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
> index 5b07b0379a501..f629c6dfd8555 100644
> --- a/kernel/dma/pool.c
> +++ b/kernel/dma/pool.c
> @@ -193,28 +193,46 @@ static unsigned long calculate_pool_size(unsigned long zone_pages)
> return max_t(unsigned long, nr_pages << PAGE_SHIFT, SZ_128K);
> }
>
> +#if defined(CONFIG_ZONE_DMA) || defined(CONFIG_ZONE_DMA32)
> +static unsigned long __init nr_managed_pages(int zone_idx)
> +{
> + struct pglist_data *pgdat;
> + unsigned long nr_pages = 0;
> +
> + for_each_online_pgdat(pgdat)
> + nr_pages += zone_managed_pages(&pgdat->node_zones[zone_idx]);
> + return nr_pages;
> +}
> +#endif /* CONFIG_ZONE_DMA || CONFIG_ZONE_DMA32 */
> +
> static int __init dma_atomic_pool_init(void)
> {
> + unsigned long nr_zone_dma_pages, nr_zone_dma32_pages;
> +
> +#ifdef CONFIG_ZONE_DMA
> + nr_zone_dma_pages = nr_managed_pages(ZONE_DMA);
> + if (nr_zone_dma_pages)
> + atomic_pool_dma = __dma_atomic_pool_init(
> + calculate_pool_size(nr_zone_dma_pages),
> + GFP_DMA);
> +#endif
> +#ifdef CONFIG_ZONE_DMA32
> + nr_zone_dma32_pages = nr_managed_pages(ZONE_DMA32);
> + if (nr_zone_dma32_pages)
> + atomic_pool_dma32 = __dma_atomic_pool_init(
> + calculate_pool_size(nr_zone_dma32_pages),
> + GFP_DMA32);
> +#endif
> /*
> * If coherent_pool was not used on the command line, default the pool
> * sizes to 128KB per 1GB of memory, min 128KB, max MAX_ORDER-1.
> */
> if (!atomic_pool_size)
> atomic_pool_size = calculate_pool_size(totalram_pages());
> -
> - INIT_WORK(&atomic_pool_work, atomic_pool_work_fn);
> -
> atomic_pool_kernel = __dma_atomic_pool_init(atomic_pool_size,
> GFP_KERNEL);
> - if (has_managed_dma()) {
> - atomic_pool_dma = __dma_atomic_pool_init(atomic_pool_size,
> - GFP_KERNEL | GFP_DMA);
> - }
> - if (IS_ENABLED(CONFIG_ZONE_DMA32)) {
> - atomic_pool_dma32 = __dma_atomic_pool_init(atomic_pool_size,
> - GFP_KERNEL | GFP_DMA32);
> - }
>
> + INIT_WORK(&atomic_pool_work, atomic_pool_work_fn);
> dma_atomic_pool_debugfs_init();
> return 0;
> }
> --
> 2.30.2

--
Michal Hocko
SUSE Labs