Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] cma: make number of CMA areas dynamic, remove
 CONFIG_CMA_AREAS
To:     "Song Bao Hua (Barry Song)" <song.bao.hua@hisilicon.com>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-mips@vger.kernel.org" <linux-mips@vger.kernel.org>
Cc:     Roman Gushchin <guro@fb.com>, Mike Rapoport <rppt@kernel.org>,
        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>,
        Will Deacon <will@kernel.org>
References: <20200915205703.34572-1-mike.kravetz@oracle.com>
 <4b7b14c9eb6a42509f8324f7ed84b46f@hisilicon.com>
From:   Mike Kravetz <mike.kravetz@oracle.com>
Message-ID: <2bf99eee-29b1-4965-da7d-d4e341803440@oracle.com>
Date:   Wed, 16 Sep 2020 09:30:23 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.11.0
MIME-Version: 1.0
In-Reply-To: <4b7b14c9eb6a42509f8324f7ed84b46f@hisilicon.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 9/16/20 2:14 AM, Song Bao Hua (Barry Song) wrote:
>>> -----Original Message-----
>>> From: Mike Kravetz [mailto:mike.kravetz@oracle.com]
>>> Sent: Wednesday, September 16, 2020 8:57 AM
>>> 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>; Mike Rapoport <rppt@kernel.org>; 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: [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.
>>>
>>> Acked-by: Roman Gushchin <guro@fb.com>
>>> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
>>> ---
>>> rfc->v1
>>>   - Made minor changes suggested by Song Bao Hua (Barry Song)
>>>   - Removed check for late calls to cma_init_reserved_mem that was part
>>>     of RFC.
>>>   - Added ACK from Roman Gushchin
>>>   - Still in need of arm testing
>>
>> Unfortunately, the test result on my arm64 board is negative, Linux can't boot
>> after applying
>> this patch.
>>
>> I guess we have to hold on this patch for a while till this is fixed. BTW, Mike, do
>> you have
>> a qemu-based arm64 numa system to debug? It is very easy to reproduce, we
>> don't need to
>> use hugetlb_cma and pernuma_cma. Just the default cma will make the boot
>> hang.
> 
> Hi Mike,
> I spent some time on debugging the boot issue and sent a patch here:
> https://lore.kernel.org/linux-mm/20200916085933.25220-1-song.bao.hua@hisilicon.com/
> All details and knic oops can be found there.
> pls feel free to merge my patch into your v2 if you want. And we probably need ack from
> arm maintainers.
> 
> Also,  +Will,
> 
> Hi Will, the whole story is that Mike tried to remove the cma array with CONFIG_CMA_AREAS
> and moved to use memblock_alloc() to allocate cma area, so that the number of cma areas
> could be dynamic. It turns out it causes a kernel panic on arm64 during system boot as the
> returned address from memblock_alloc is invalid before paging_init() is done on arm64.
> 

Thank you!

Based on your analysis, I am concerned that other architectures may also
have issues.

Andrew,
I suggest we remove this patch from your tree.  I will audit all architectures
which enable CMA and look for similar issues there.  Will then merge Barry's
patch into a V2 with any other arch specific changes.
-- 
Mike Kravetz