On 9/3/20 6:58 PM, Song Bao Hua (Barry Song) wrote: > >> -----Original Message----- >> From: Mike Kravetz [mailto:mike.kravetz@xxxxxxxxxx] >> Sent: Thursday, September 3, 2020 3:02 PM >> To: linux-mm@xxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; >> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-mips@xxxxxxxxxxxxxxx >> Cc: Roman Gushchin <guro@xxxxxx>; Song Bao Hua (Barry Song) >> <song.bao.hua@xxxxxxxxxxxxx>; Joonsoo Kim <js1304@xxxxxxxxx>; Rik van >> Riel <riel@xxxxxxxxxxx>; Aslan Bakirov <aslan@xxxxxx>; Michal Hocko >> <mhocko@xxxxxxxxxx>; Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>; Mike >> Kravetz <mike.kravetz@xxxxxxxxxx> >> Subject: [RFC PATCH] cma: make number of CMA areas dynamic, remove >> CONFIG_CMA_AREAS >> >> The number of distinct CMA areas is limited by the constant >> CONFIG_CMA_AREAS. In most environments, this was set to a default >> value of 7. Not too long ago, support was added to allocate hugetlb >> gigantic pages from CMA. More recent changes to make dma_alloc_coherent >> NUMA-aware on arm64 added more potential users of CMA areas. Along >> with the dma_alloc_coherent changes, the default value of CMA_AREAS >> was bumped up to 19 if NUMA is enabled. >> >> It seems that the number of CMA users is likely to grow. Instead of >> using a static array for cma areas, use a simple linked list. These >> areas are used before normal memory allocators, so use the memblock >> allocator. > > Hello Mike, It seems it is a good idea. Thanks for addressing this. > > I was focusing on per-numa cma feature in my patchset and I didn't take care of this > while I thought we should do something for the number of cma areas. > Thanks for taking a look. One area where I could use some help is testing/verifying on arm. See the changes to arch/arm/mm/dma-mapping.c. I have tested the generic changes on my x86 platform, but do not have an arm platform for easy testing. >> void __init dma_contiguous_early_fixup(phys_addr_t base, unsigned long >> size) >> { >> - dma_mmu_remap[dma_mmu_remap_num].base = base; >> - dma_mmu_remap[dma_mmu_remap_num].size = size; >> - dma_mmu_remap_num++; >> + struct dma_contig_early_reserve *d; >> + >> + d = memblock_alloc(sizeof(struct dma_contig_early_reserve), > > sizeof(*d)? Yes. thanks. >> @@ -172,15 +173,14 @@ int __init cma_init_reserved_mem(phys_addr_t >> base, phys_addr_t size, >> struct cma *cma; >> phys_addr_t alignment; >> >> - /* Sanity checks */ >> - if (cma_area_count == ARRAY_SIZE(cma_areas)) { >> - pr_err("Not enough slots for CMA reserved regions!\n"); >> - return -ENOSPC; >> - } >> + /* Do not attempt allocations after memblock allocator is torn down */ >> + if (slab_is_available()) >> + return -EINVAL; >> >> if (!size || !memblock_is_region_reserved(base, size)) >> return -EINVAL; >> >> + > > Is this empty line relevant? No, added by mistake. >> @@ -192,12 +192,17 @@ int __init cma_init_reserved_mem(phys_addr_t >> base, phys_addr_t size, >> if (ALIGN(base, alignment) != base || ALIGN(size, alignment) != size) >> return -EINVAL; >> >> + cma = memblock_alloc(sizeof(struct cma), sizeof(long)); > > sizeof(*cma)? Yes, thanks. > It seems we are going to write cma-> count, order_per_bit, debugfs fields. > To avoid false sharing of the cacheline of struct cma, it is better to align with > SMP_CACHE_BYTES. > > On the other hand, it seems we are unlikely to write the cma I thought about using SMP_CACHE_BYTES, but the structures are simply defined as an array today. This should not be any worse. I do not believe access to the structures is performance sensitive. Thanks, -- Mike Kravetz
