Re: [PATCH] sparc: Use vmemmap_populate_hugepages for vmemmap_populate

[Tao pilgrim <pilgrimtao@gmail.com>]

On Thu, Dec 18, 2025 at 7:49 PM David Hildenbrand (Red Hat)
<david@kernel.org> wrote:
>
> On 12/18/25 12:02, Tao pilgrim wrote:
> > On Thu, Dec 18, 2025 at 4:44 PM David Hildenbrand (Red Hat)
> > <david@kernel.org> wrote:
> >>
> >> On 12/17/25 13:08, chengkaitao wrote:
> >>> From: Chengkaitao <chengkaitao@kylinos.cn>
>
> ^ this is the author
>
> And you reply from a completely different address with a completely
> different name.
>
> To whom am I talking here? The patch author?

Yes, I'm the author. Due to company security policies, I have to
use two different email addresses, which I have no choice...

> [...]
>
> >> Bunch of unrelated changes that should not go in here.
> >
> > This indeed contains some unrelated code changes and removal of
> > extra whitespace. These could be split into a separate patch,
> > but the new patch might be somewhat redundant, lol. If you'd
> > like me to proceed this way, please reply confirming.
>
> We usually don't do random other stuff as part of one patch. It's a
> different story if you touch the surrounding code, but that doesn't look
> like that here?
>
> >
> >>> @@ -2581,8 +2581,8 @@ unsigned long _PAGE_CACHE __read_mostly;
> >>>    EXPORT_SYMBOL(_PAGE_CACHE);
> >>>
> >>>    #ifdef CONFIG_SPARSEMEM_VMEMMAP
> >>> -int __meminit vmemmap_populate(unsigned long vstart, unsigned long vend,
> >>> -                            int node, struct vmem_altmap *altmap)
> >>> +void __meminit vmemmap_set_pmd(pmd_t *pmd, void *p, int node,
> >>> +                            unsigned long addr, unsigned long next)
> >>>    {
> >>>        unsigned long pte_base;
> >>>
> >>> @@ -2595,39 +2595,25 @@ int __meminit vmemmap_populate(unsigned long vstart, unsigned long vend,
> >>>
> >>>        pte_base |= _PAGE_PMD_HUGE;
> >>>
> >>> -     vstart = vstart & PMD_MASK;
> >>> -     vend = ALIGN(vend, PMD_SIZE);
> >>> -     for (; vstart < vend; vstart += PMD_SIZE) {
> >>> -             pgd_t *pgd = vmemmap_pgd_populate(vstart, node);
> >>> -             unsigned long pte;
> >>> -             p4d_t *p4d;
> >>> -             pud_t *pud;
> >>> -             pmd_t *pmd;
> >>> -
> >>> -             if (!pgd)
> >>> -                     return -ENOMEM;
> >>> -
> >>> -             p4d = vmemmap_p4d_populate(pgd, vstart, node);
> >>> -             if (!p4d)
> >>> -                     return -ENOMEM;
> >>> -
> >>> -             pud = vmemmap_pud_populate(p4d, vstart, node);
> >>> -             if (!pud)
> >>> -                     return -ENOMEM;
> >>> -
> >>> -             pmd = pmd_offset(pud, vstart);
> >>> -             pte = pmd_val(*pmd);
> >>> -             if (!(pte & _PAGE_VALID)) {
> >>> -                     void *block = vmemmap_alloc_block(PMD_SIZE, node);
> >>> +     pmd_val(*pmd) = pte_base | __pa(p);
> >>> +}
> >>>
> >>> -                     if (!block)
> >>> -                             return -ENOMEM;
> >>> +bool __meminit vmemmap_false_pmd(pmd_t *pmd, int node)
> >>> +{
> >>> +     return true;
> >>> +}
> >>>
> >>> -                     pmd_val(*pmd) = pte_base | __pa(block);
> >>> -             }
> >>> -     }
> >>> +int __meminit vmemmap_check_pmd(pmd_t *pmdp, int node,
> >>> +                             unsigned long addr, unsigned long next)
> >>> +{
> >>> +     vmemmap_verify((pte_t *)pmdp, node, addr, next);
> >>> +     return 1;
> >>> +}
> >>>
> >>> -     return 0;
> >>> +int __meminit vmemmap_populate(unsigned long vstart, unsigned long vend,
> >>> +                            int node, struct vmem_altmap *altmap)
> >>> +{
> >>> +     return vmemmap_populate_hugepages(vstart, vend, node, altmap);
> >>>    }
> >>>    #endif /* CONFIG_SPARSEMEM_VMEMMAP */
> >>>
> >>> diff --git a/include/linux/mm.h b/include/linux/mm.h
> >>> index 15076261d0c2..5e005b0f947d 100644
> >>> --- a/include/linux/mm.h
> >>> +++ b/include/linux/mm.h
> >>> @@ -4248,6 +4248,7 @@ void *vmemmap_alloc_block_buf(unsigned long size, int node,
> >>>    void vmemmap_verify(pte_t *, int, unsigned long, unsigned long);
> >>>    void vmemmap_set_pmd(pmd_t *pmd, void *p, int node,
> >>>                     unsigned long addr, unsigned long next);
> >>> +bool vmemmap_false_pmd(pmd_t *pmd, int node);
> >>>    int vmemmap_check_pmd(pmd_t *pmd, int node,
> >>>                      unsigned long addr, unsigned long next);
> >>>    int vmemmap_populate_basepages(unsigned long start, unsigned long end,
> >>> diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c
> >>> index 37522d6cb398..bd54b8c6f56e 100644
> >>> --- a/mm/sparse-vmemmap.c
> >>> +++ b/mm/sparse-vmemmap.c
> >>> @@ -407,6 +407,11 @@ void __weak __meminit vmemmap_set_pmd(pmd_t *pmd, void *p, int node,
> >>>    {
> >>>    }
> >>>
> >>> +bool __weak __meminit vmemmap_false_pmd(pmd_t *pmd, int node)
> >>> +{
> >>> +     return 0;
> >>> +}
> >>> +
> >>
> >> Reading that function I have absolutely no clue what this is supposed to
> >> do. :)
> >>
> >> Also, why are you passing pmd+node when sparc ignores them completely
> >> and statically returns "true" ?
> >
> > The pmd+node is indeed unnecessary. My original intention was
> > to provide convenience for future architecture extensions, but
> > upon reflection, this appears to be a case of over-engineering.
>
> Jup.
>
> >
> >> If you can tell me what the semantics of that function should be, maybe
> >> we can come up with a more descriptive name.
> >
> > In the SPARC architecture, the original vmemmap_populate
> > function does not retry with vmemmap_populate_basepages
> > after vmemmap_alloc_block fails. I suspect SPARC doesn't
> > support basepages, which is why we need to modify
> > vmemmap_populate_hugepages to provide an interface that
> > skips basepages handling.
>
> So, something like vmemmap_pte_fallback_allowed() ?

LGTM！

-- 
Yours,
Kaitao Cheng