From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CFF553803D6 for ; Fri, 31 Jul 2026 22:10:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785535831; cv=none; b=UPnw4nDjE5kOD3xxCIHEggftrtj86ppkKEAl9GBjgN87oX5EshJlc+JFcqUWB2a/pEBeZTWofG+FVT4RT9E43L3iJPSUJmWDONJIK06iTwTgwem9yTt3VW41zI3vqUA6X+zzKNs217QIGcVAFxbtkOI9ZfdfkOtj2wfIF70ZoU8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785535831; c=relaxed/simple; bh=vetIsnfmk6L4W9ACw4SvWEZbHcBBdVpCKKCLnd1Qgj0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=vBRAEszBGdl62+gBX+QJwHAi29Zfy1UM6z2t6KGAKYKr4BmOCOLNZzWmA76RZZVl6Kg+JgLsM1tGjw56PBUidxH7iZvu6E38IUK6gqNiHJ1Vev1ZSmEh24tDMmWAW9RUQx/9vTnLA2/IVsR9fGD4HJrMkKmJ09JyQkhqxvAnxG0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=SgtmVEXM; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="SgtmVEXM" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 701381F00ACF; Fri, 31 Jul 2026 22:10:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1785535829; bh=KcMHynopxq0LB8O3aiZw02VpzCo3iMp78znRKkfrp3w=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=SgtmVEXMK+BaM0dDhNotsFBU1VNhPZ/5ykE3Gou3coxHECMuyL0PwNTOMkWSJ+eos ttqAFupqL1YevTA8JWZ1LdUYx09hwpFTjKg6odUNrzXnUuGGnfZe/jcT0ftoOrQWWk NC/HHmeoV1H+SU0ZzDGwPDDcico8eKzwfpTFqVSIJSAe7ayrSuUaIwcwHLEc58zifl pLOMRPpUiWTD0FdUwOD7qLD9qkhj+fbbQXaOODFBHzW0Z1sIdeaBy0+s0a5bH6YMve VYvBCt8UN4Bai7CfvxLY3gh0ZFquzAmbwVoWcH8c/weg0aVgYsM3EJIU7ZRImkUpuN H4oJtNNK8ztQA== Date: Fri, 31 Jul 2026 22:10:27 +0000 From: Yosry Ahmed To: Brendan Jackman Cc: Borislav Petkov , Dave Hansen , Peter Zijlstra , Andrew Morton , David Hildenbrand , Vlastimil Babka , Mike Rapoport , Wei Xu , Johannes Weiner , Zi Yan , Lorenzo Stoakes , linux-mm@kvack.org, linux-kernel@vger.kernel.org, x86@kernel.org, Sumit Garg , Will Deacon , rientjes@google.com, "Kalyazin, Nikita" , patrick.roy@linux.dev, "Itazuri, Takahiro" , Andy Lutomirski , David Kaplan , Thomas Gleixner , Patrick Bellasi , Reiji Watanabe , Sean Christopherson Subject: Re: [PATCH v3 04/26] x86/mm: split out preallocate_sub_pgd() Message-ID: References: <20260726-page_alloc-unmapped-v3-0-6f5729aa9832@google.com> <20260726-page_alloc-unmapped-v3-4-6f5729aa9832@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260726-page_alloc-unmapped-v3-4-6f5729aa9832@google.com> On Sun, Jul 26, 2026 at 10:22:37PM +0000, Brendan Jackman wrote: > This code will be needed elsewhere in a following patch. Split out the > trivial code move for easy review. > > As a side effect, change the logging slightly: instead of directly > reporting the level of the failure in panic(), show a generic panic > message, will be preceded by a separate warn that reports the level of > the failure. This is a simple way to have this helper suit the needs of > its new user as well as the existing one. > > Other than logging, no functional change intended. > > Signed-off-by: Brendan Jackman > --- > arch/x86/include/asm/pgalloc.h | 3 +++ > arch/x86/mm/init_64.c | 44 +++++++----------------------------------- > arch/x86/mm/pgtable.c | 38 ++++++++++++++++++++++++++++++++++++ > 3 files changed, 48 insertions(+), 37 deletions(-) > > diff --git a/arch/x86/include/asm/pgalloc.h b/arch/x86/include/asm/pgalloc.h > index c88691b15f3c6..2aba6cfabf495 100644 > --- a/arch/x86/include/asm/pgalloc.h > +++ b/arch/x86/include/asm/pgalloc.h > @@ -2,6 +2,7 @@ > #ifndef _ASM_X86_PGALLOC_H > #define _ASM_X86_PGALLOC_H > > +#include > #include > #include /* for struct page */ > #include > @@ -128,6 +129,8 @@ static inline void __pud_free_tlb(struct mmu_gather *tlb, pud_t *pud, > ___pud_free_tlb(tlb, pud); > } > > +extern int preallocate_sub_pgd(struct mm_struct *mm, unsigned long addr); > + Do we need extern here? > #if CONFIG_PGTABLE_LEVELS > 4 > static inline void pgd_populate(struct mm_struct *mm, pgd_t *pgd, p4d_t *p4d) > { > diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c > index ab4c5a02326f7..ac6688c70872e 100644 > --- a/arch/x86/mm/init_64.c > +++ b/arch/x86/mm/init_64.c > @@ -1293,46 +1293,16 @@ static struct kcore_list kcore_vsyscall; > static void __init preallocate_vmalloc_pages(void) > { > unsigned long addr; > - const char *lvl; > > for (addr = VMALLOC_START; addr <= VMEMORY_END; addr = ALIGN(addr + 1, PGDIR_SIZE)) { > - pgd_t *pgd = pgd_offset_k(addr); > - p4d_t *p4d; > - pud_t *pud; > - > - lvl = "p4d"; > - p4d = p4d_alloc(&init_mm, pgd, addr); > - if (!p4d) > - goto failed; > - > - if (pgtable_l5_enabled()) > - continue; > - > - /* > - * The goal here is to allocate all possibly required > - * hardware page tables pointed to by the top hardware > - * level. > - * > - * On 4-level systems, the P4D layer is folded away and > - * the above code does no preallocation. Below, go down > - * to the pud _software_ level to ensure the second > - * hardware level is allocated on 4-level systems too. > - */ > - lvl = "pud"; > - pud = pud_alloc(&init_mm, p4d, addr); > - if (!pud) > - goto failed; > + if (preallocate_sub_pgd(&init_mm, addr)) { > + /* > + * The pages have to be there now or they will be > + * missing in process page-tables later. > + */ > + panic("Failed to pre-allocate pagetables for vmalloc area\n"); > + } Nit: We can probably move this comment above the if block, and drop the curly braces: /* * The pages have to be there now or they will be missing in * process page-tables later. */ if (preallocate_sub_pgd(&init_mm, addr)) panic("Failed to pre-allocate pagetables for vmalloc area\n"); > } > - > - return; > - > -failed: > - > - /* > - * The pages have to be there now or they will be missing in > - * process page-tables later. > - */ > - panic("Failed to pre-allocate %s pages for vmalloc area\n", lvl); > } > > void __init arch_mm_preinit(void) > diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c > index f32facdb30354..fdd3709509946 100644 > --- a/arch/x86/mm/pgtable.c > +++ b/arch/x86/mm/pgtable.c > @@ -833,3 +833,41 @@ void arch_check_zapped_pud(struct vm_area_struct *vma, pud_t pud) > /* See note in arch_check_zapped_pte() */ > VM_WARN_ON_ONCE(!(vma->vm_flags & VM_SHADOW_STACK) && pud_shstk(pud)); > } > + > +#if CONFIG_PGTABLE_LEVELS > 3 > +/* > + * Allocate all possibly required hardware page tables pointed to ths ^the > + * top hardware level. In other words, allocate a p4d on 5-level or a allocate p4ds? > + * pud on 4-level. puds? > + */ > +int preallocate_sub_pgd(struct mm_struct *mm, unsigned long addr) > +{ > + const char *lvl; Nit: const char *lvl = "p4d"; or: const char *lvl = pgtable_l5_enabled() ? "p4d" : "pud"; But I am wondering how important this information is here? We should be able to tell whether 5-level paging is enabled based on kernel config and command line. If the information is generally not easy to get, maybe logging it during boot would generally be useful? Anyway, if we drop lvl here we can drop the gotos, which would be nice. > + p4d_t *p4d; > + pud_t *pud; > + > + lvl = "p4d"; > + p4d = p4d_alloc(mm, pgd_offset_pgd(mm->pgd, addr), addr); > + if (!p4d) > + goto failed; > + > + if (pgtable_l5_enabled()) > + return 0; > + > + /* > + * On 4-level systems, the P4D layer is folded away and > + * the above code does no preallocation. Below, go down > + * to the pud _software_ level to ensure the second > + * hardware level is allocated on 4-level systems too. > + */ > + lvl = "pud"; > + pud = pud_alloc(mm, p4d, addr); > + if (!pud) > + goto failed; > + return 0; > + > +failed: > + pr_warn_ratelimited("Failed to preallocate %s\n", lvl); Can this possibly fire more than once? IIUC we will panic right after returning. > + return -ENOMEM; > +} > +#endif > > -- > 2.54.0 >