Re: [PATCH] sparc: Use vmemmap_populate_hugepages for vmemmap_populate

[David Hildenbrand (Red Hat)]

On 12/18/25 12:02, Tao pilgrim wrote:
> On Thu, Dec 18, 2025 at 4:44 PM David Hildenbrand (Red Hat)
> <david@xxxxxxxxxx> wrote:
> > On 12/17/25 13:08, chengkaitao wrote:
> > > From: Chengkaitao <chengkaitao@xxxxxxxxxx>

^ 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?


[...]

> > >
> >
> > 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() ?

--
Cheers

David