lkml.org 
[lkml]   [2020]   [Sep]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC PATCH] cma: make number of CMA areas dynamic, remove CONFIG_CMA_AREAS
From
Date
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

\
 
 \ /
  Last update: 2020-09-08 20:30    [W:0.114 / U:1.744 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site