Re: [PATCH v2 1/5] mm/sparse-vmemmap: provide generic vmemmap_set_pmd() and vmemmap_check_pmd()
From: Mike Rapoport
Date: Thu Apr 09 2026 - 06:44:25 EST
On Sun, Apr 05, 2026 at 10:07:36PM +0800, Muchun Song wrote:
> > On Apr 5, 2026, at 15:07, Mike Rapoport <rppt@xxxxxxxxxx> wrote:
> > On Sat, Apr 04, 2026 at 08:20:54PM +0800, Muchun Song wrote:
> >> The two weak functions are currently no-ops on every architecture,
> >> forcing each platform that needs them to duplicate the same handful
> >> of lines. Provide a generic implementation:
> >>
> >> - vmemmap_set_pmd() simply sets a huge PMD with PAGE_KERNEL protection.
> >>
> >> - vmemmap_check_pmd() verifies that the PMD is present and leaf,
> >> then calls the existing vmemmap_verify() helper.
> >>
> >> Architectures that need special handling can continue to override the
> >> weak symbols; everyone else gets the standard version for free.
> >>
> >> Signed-off-by: Muchun Song <songmuchun@xxxxxxxxxxxxx>
> >> ---
> >> mm/sparse-vmemmap.c | 7 ++++++-
> >> 1 file changed, 6 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c
> >> index 6eadb9d116e4..1eb990610d50 100644
> >> --- a/mm/sparse-vmemmap.c
> >> +++ b/mm/sparse-vmemmap.c
> >> @@ -391,12 +391,17 @@ int __meminit vmemmap_populate_hvo(unsigned long addr, unsigned long end,
> >> void __weak __meminit vmemmap_set_pmd(pmd_t *pmd, void *p, int node,
> >> unsigned long addr, unsigned long next)
> >> {
> >> + BUG_ON(!pmd_set_huge(pmd, virt_to_phys(p), PAGE_KERNEL));
> >
> > Do we have to crash the kernel here?
> > Wouldn't be better to make vmemmap_set_pmd() return error and make
> > vmemmap_populate_hugepages() fall back to base pages in case
> > vmemmap_set_pmd() errored?
>
> Hi Mike,
>
> Thanks for the review. Let me explain my original thought process here.
>
> My assumption was that pmd_set_huge() for the kernel virtual address space
> should rarely, if ever, fail in this context. Furthermore, if we look at the
> architectures this patch replaces (e.g., arm64 and riscv), they are either
> ignoring the return value of pmd_set_huge() entirely or lacking any graceful
> fallback mechanism anyway.
>
> So, to keep the initial generic implementation as simple as possible, I used
> BUG_ON() as a strict assertion.
Fair enough, let's just make it VM_BUG_ON().
--
Sincerely yours,
Mike.