Messages in this thread | | | Subject | Re: [RFC PATCH] cma: make number of CMA areas dynamic, remove CONFIG_CMA_AREAS | From | Mike Kravetz <> | Date | Tue, 8 Sep 2020 11:29:15 -0700 |
| |
On 9/3/20 6:58 PM, Song Bao Hua (Barry Song) wrote: > >> -----Original Message----- >> From: Mike Kravetz [mailto:mike.kravetz@oracle.com] >> Sent: Thursday, September 3, 2020 3:02 PM >> To: linux-mm@kvack.org; linux-kernel@vger.kernel.org; >> linux-arm-kernel@lists.infradead.org; linux-mips@vger.kernel.org >> Cc: Roman Gushchin <guro@fb.com>; Song Bao Hua (Barry Song) >> <song.bao.hua@hisilicon.com>; Joonsoo Kim <js1304@gmail.com>; Rik van >> Riel <riel@surriel.com>; Aslan Bakirov <aslan@fb.com>; Michal Hocko >> <mhocko@kernel.org>; Andrew Morton <akpm@linux-foundation.org>; Mike >> Kravetz <mike.kravetz@oracle.com> >> 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
| |