Message-ID: <6B512C74DCAB0BF0+0ad9a892-6933-baa3-0848-1e1efc685c9f@tinylab.org>
Date:   Wed, 14 Jun 2023 17:51:23 +0800
From:   Song Shuai <songshuaishuai@...ylab.org>
To:     Alexandre Ghiti <alexghiti@...osinc.com>,
        Woody Zhang <woodylab@...mail.com>
Cc:     Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        Conor Dooley <conor.dooley@...rochip.com>,
        linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] riscv: move memblock_allow_resize() after lm is ready



在 2023/6/12 15:15, Alexandre Ghiti 写道:
> Hi Woody,
> 
> On Sat, Jun 10, 2023 at 1:49 AM Woody Zhang <woodylab@...mail.com> wrote:
>>
>> The initial memblock metadata is accessed from kernel image mapping. The
>> regions arrays need to "reallocated" from memblock and accessed through
>> linear mapping to cover more memblock regions. So the resizing should
>> not be allowed until linear mapping is ready. Note that there are
>> memblock allocations when building linear mapping.
>>
>> Signed-off-by: Woody Zhang <woodylab@...mail.com>
>> ---
>>   arch/riscv/mm/init.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
>> index 9e9da69720ce..8a33ecbb4d0f 100644
>> --- a/arch/riscv/mm/init.c
>> +++ b/arch/riscv/mm/init.c
>> @@ -258,7 +258,6 @@ static void __init setup_bootmem(void)
>>          dma_contiguous_reserve(dma32_phys_limit);
>>          if (IS_ENABLED(CONFIG_64BIT))
>>                  hugetlb_cma_reserve(PUD_SHIFT - PAGE_SHIFT);
>> -       memblock_allow_resize();
>>   }
>>
>>   #ifdef CONFIG_MMU
>> @@ -1250,6 +1249,9 @@ static void __init setup_vm_final(void)
>>          csr_write(CSR_SATP, PFN_DOWN(__pa_symbol(swapper_pg_dir)) | satp_mode);
>>          local_flush_tlb_all();
>>
>> +       /* Depend on that Linear Mapping is ready */
>> +       memblock_allow_resize();
>> +
>>          pt_ops_set_late();
>>   }
>>   #else
>> --
>> 2.39.2
>>
> 
> The commit log does not describe the issue thoroughly enough to me,
> maybe you could point to the arm64 commit that did the same? I mean
> commit 24cc61d8cb5a ("arm64: memblock: don't permit memblock resizing
> until linear mapping is up").
@Alex

I reproduced the problem as the arm64 commit describes.
You can find the complete log via this link: https://termbin.com/bx0o

I constructed the dtb with numerous discrete /memreserve/ regions
(the numbers of these regions approximate INIT_MEMBLOCK_REGIONS) which 
full the reserved regions up.

When memblock_allow_resize was set, the calling of memblock_reserve() would
double/resize the reserved regions and do the __memcopy() from the old 
regions ( mapped by kernel)
to the new ones (provided by __va()).
But before the linear mapping was ready (like: during the creating of 
linear mapping),
memblock_reserve() was called and the memcopy would trigger a Store/AMO 
page fault.

> 
> Another point is that I would not put this call into setup_vm_final(),
> I'd rather add it in paging_init() as it does not seem like a good fit
> for setup_vm_final(). But that's a nit so up to you of course.
> 
I agree.

@Woody

I noticed your V2 [1] didn't take this suggestion, maybe you can take it 
at V3.
And it will be more sound if you supplement the commit-msg with the 
panic info from the log.

[1]: 
https://lore.kernel.org/linux-riscv/tencent_97F6C19BAF7C99B22BB1199FE16B266C2B07@qq.com/
> Anyway, that's a good catch, thanks!
> 
> Alex
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

-- 
Thanks
Song Shuai

Powered by blists - more mailing lists