linux-kernel - Re: [RFC PATCH] cma: make number of CMA areas dynamic, remove CONFIG_CMA

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <7e730a74-3a9d-a18c-3f6a-f7bb206298d5@oracle.com>
Date:   Wed, 16 Sep 2020 10:30:57 -0700
From:   Mike Kravetz <mike.kravetz@...cle.com>
To:     Christoph Hellwig <hch@...radead.org>
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-mips@...r.kernel.org,
        Roman Gushchin <guro@...com>,
        Barry Song <song.bao.hua@...ilicon.com>,
        Joonsoo Kim <js1304@...il.com>,
        Rik van Riel <riel@...riel.com>,
        Aslan Bakirov <aslan@...com>, Michal Hocko <mhocko@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [RFC PATCH] cma: make number of CMA areas dynamic, remove
 CONFIG_CMA_AREAS

On 9/15/20 9:32 PM, Christoph Hellwig wrote:
> On Wed, Sep 02, 2020 at 08:02:04PM -0700, Mike Kravetz wrote:
>> --- a/arch/arm/mm/dma-mapping.c
>> +++ b/arch/arm/mm/dma-mapping.c
>> @@ -383,25 +383,34 @@ postcore_initcall(atomic_pool_init);
>>  struct dma_contig_early_reserve {
>>  	phys_addr_t base;
>>  	unsigned long size;
>> +	struct list_head areas;
>>  };
>>  
>> +static __initdata LIST_HEAD(dma_mmu_remap_areas);
>>  
>>  void __init dma_contiguous_early_fixup(phys_addr_t base, unsigned long size)
>>  {
>> +	struct dma_contig_early_reserve *d;
>> +
>> +	d = memblock_alloc(sizeof(struct dma_contig_early_reserve),
>> +			sizeof(void *));
>> +	if (!d) {
>> +		pr_err("Unable to allocate dma_contig_early_reserve struct!\n");
>> +		return;
>> +	}
>> +
>> +	d->base = base;
>> +	d->size = size;
>> +	list_add_tail(&d->areas, &dma_mmu_remap_areas);
>>  }
> 
> I wonder if struct cma should grow a flags or type field, so that the
> arm code can simply use cma_for_each_area to iterate the CMA areas for
> the DMA fixup, and we can remove the extra list and the magic hook.

I will look into a way of doing that.

> 
>> +/* modify here */
>> +LIST_HEAD(cma_areas);
> 
> What does this comment mean?

Sorry, that might have been a note to myself that was accidentally left.

> 
>> +static unsigned int cma_area_count;
> 
> It seems this is only used to provide a default name for the CMA
> areas, but all areas actually provide a name, so I think we can drop
> the default naming and the cma_area_count variable entirely.
> 

Seems reasonable.
We can change behavior to require a name.

>>  	if (!size || !memblock_is_region_reserved(base, size))
>>  		return -EINVAL;
>>  
>> +
>>  	/* ensure minimal alignment required by mm core */
> 
> This adds a spurious empty line.

yes, my bad.

>>  static int __init cma_debugfs_init(void)
>>  {
>>  	struct dentry *cma_debugfs_root;
>> -	int i;
>> +	struct cma *c;
>>  
>>  	cma_debugfs_root = debugfs_create_dir("cma", NULL);
>>  
>> -	for (i = 0; i < cma_area_count; i++)
>> -		cma_debugfs_add_one(&cma_areas[i], cma_debugfs_root);
>> +	list_for_each_entry(c, &cma_areas, areas)
>> +		cma_debugfs_add_one(c, cma_debugfs_root);
> 
> I think this should use cma_for_each_area, that way cma_areas can be
> keep static in cma.c.

Yes, will provide a cma_for_each_area routine.

-- 
Mike Kravetz