linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Dev Jain <dev.jain@arm.com>
To: Justin He <Justin.He@arm.com>,
	Catalin Marinas <Catalin.Marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Uladzislau Rezki <urezki@gmail.com>
Cc: Anshuman Khandual <Anshuman.Khandual@arm.com>,
	Ryan Roberts <Ryan.Roberts@arm.com>, Peter Xu <peterx@redhat.com>,
	Joey Gouly <Joey.Gouly@arm.com>,
	Yicong Yang <yangyicong@hisilicon.com>,
	"Matthew Wilcox (Oracle)" <willy@infradead.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>
Subject: Re: [PATCH] mm: vmalloc: use VMALLOC_EARLY_START boundary for early vmap area
Date: Tue, 29 Jul 2025 21:56:16 +0530	[thread overview]
Message-ID: <f5efa19f-7eb9-4ffd-aa12-6aae19379cf8@arm.com> (raw)
In-Reply-To: <AS2PR08MB97864CC7022A8DB7BDB87952F75AA@AS2PR08MB9786.eurprd08.prod.outlook.com>


On 28/07/25 11:49 am, Justin He wrote:
> Hi Dev,
>
>> -----Original Message-----
>> From: Dev Jain <Dev.Jain@arm.com>
>> Sent: Tuesday, July 22, 2025 2:48 PM
>> To: Justin He <Justin.He@arm.com>; Catalin Marinas
>> <Catalin.Marinas@arm.com>; Will Deacon <will@kernel.org>; Andrew
>> Morton <akpm@linux-foundation.org>; Uladzislau Rezki <urezki@gmail.com>
>> Cc: Anshuman Khandual <Anshuman.Khandual@arm.com>; Ryan Roberts
>> <Ryan.Roberts@arm.com>; Peter Xu <peterx@redhat.com>; Joey Gouly
>> <Joey.Gouly@arm.com>; Yicong Yang <yangyicong@hisilicon.com>; Matthew
>> Wilcox (Oracle) <willy@infradead.org>; linux-arm-kernel@lists.infradead.org;
>> linux-kernel@vger.kernel.org; linux-mm@kvack.org
>> Subject: Re: [PATCH] mm: vmalloc: use VMALLOC_EARLY_START boundary for
>> early vmap area
>>
>>
>> On 22/07/25 9:38 am, Jia He wrote:
>>> When VMALLOC_START is redefined to a new boundary, most subsystems
>>> continue to function correctly. However, vm_area_register_early()
>>> assumes the use of the global _vmlist_ structure before vmalloc_init()
>>> is invoked. This assumption can lead to issues during early boot.
>>>
>>> See the calltrace as follows:
>>> 	start_kernel()
>>> 		setup_per_cpu_areas()
>>> 			pcpu_page_first_chunk()
>>> 				vm_area_register_early()
>>> 		mm_core_init()
>>> 			vmalloc_init()
>>>
>>> The early vm areas will be added to vmlist at declare_kernel_vmas()
>>> ->declare_vma():
>>> ffff800080010000 T _stext
>>> ffff800080da0000 D __start_rodata
>>> ffff800081890000 T __inittext_begin
>>> ffff800081980000 D __initdata_begin
>>> ffff800081ee0000 D _data
>>> The starting address of the early areas is tied to the *old*
>>> VMALLOC_START (i.e. 0xffff800080000000 on an arm64 N2 server).
>>>
>>> If VMALLOC_START is redefined, it can disrupt early VM area
>>> allocation, particularly in like pcpu_page_first_chunk()-
>>> vm_area_register_early().
>>>
>>> To address this potential risk on arm64, introduce a new boundary,
>>> VMALLOC_EARLY_START, to avoid boot issues when VMALLOC_START is
>>> occasionaly redefined.
>> Sorry but I am unable to understand the point of the patch. If a particular
>> value of VMALLOC_START causes a problem because the vma declarations of
>> the kernel are tied to that value, surely we should be reasoning about what
>> was wrong about the new value, and not circumventing the actual problem by
>> introducing VMALLOC_EARLY_START?
>>
>> Also by your patch description I don't think you have run into a reproducible
>> boot issue, so this patch is basically adding dead code because both macros
>> are defined to MODULES_END?
>>
> Please try this *debugging* purpose patch which can trigger the boot panic
> more easily(I can always reproduce the boot panic on an ARM64 server):
>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>   index 192d86e1cc76..2be8db8d0205 100644
>   --- a/arch/arm64/include/asm/pgtable.h
>   +++ b/arch/arm64/include/asm/pgtable.h
>   @@ -20,7 +20,8 @@
>     * VMALLOC_START: beginning of the kernel vmalloc space
>     * VMALLOC_END: extends to the available space below vmemmap
>     */
>   -#define VMALLOC_START          (MODULES_END)
>   +//#define VMALLOC_START                (MODULES_END)
>   +#define VMALLOC_START          ((MODULES_END & PGDIR_MASK) + PGDIR_SIZE)
>    #if VA_BITS == VA_BITS_MIN
>    #define VMALLOC_END            (VMEMMAP_START - SZ_8M)
>    #else
>   diff --git a/mm/percpu.c b/mm/percpu.c
>   index b35494c8ede2..53d187172b5e 100644
>   --- a/mm/percpu.c
>   +++ b/mm/percpu.c
>   @@ -3051,7 +3051,7 @@ int __init pcpu_embed_first_chunk(size_t reserved_size, size_t dyn_size,
>           max_distance += ai->unit_size * ai->groups[highest_group].nr_units;
>
>           /* warn if maximum distance is further than 75% of vmalloc space */
>   -       if (max_distance > VMALLOC_TOTAL * 3 / 4) {
>   +       if (1 || max_distance > VMALLOC_TOTAL * 3 / 4) {

This will always return true - which leads to returning -EINVAL and then
panicking in setup_per_cpu_areas(). Probably you made this change by mistake
and are trying to say that the redefinition above panics?

>                   pr_warn("max_distance=0x%lx too large for vmalloc space 0x%lx\n",
>                                   max_distance, VMALLOC_TOTAL);
>    #ifdef CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK
>
>
> ---
> Cheers,
> Justin He(Jia He)
>
>


  reply	other threads:[~2025-07-29 16:27 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-22  4:08 [PATCH] mm: vmalloc: use VMALLOC_EARLY_START boundary for early vmap area Jia He
2025-07-22  6:48 ` Dev Jain
2025-07-28  6:19   ` Justin He
2025-07-29 16:26     ` Dev Jain [this message]
2025-07-22 19:39 ` Uladzislau Rezki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f5efa19f-7eb9-4ffd-aa12-6aae19379cf8@arm.com \
    --to=dev.jain@arm.com \
    --cc=Anshuman.Khandual@arm.com \
    --cc=Catalin.Marinas@arm.com \
    --cc=Joey.Gouly@arm.com \
    --cc=Justin.He@arm.com \
    --cc=Ryan.Roberts@arm.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=peterx@redhat.com \
    --cc=urezki@gmail.com \
    --cc=will@kernel.org \
    --cc=willy@infradead.org \
    --cc=yangyicong@hisilicon.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).