Re: [PATCH v2 6/7] mm/vmalloc: align vm_area so vmap() can batch mappings

[Uladzislau Rezki <urezki@gmail.com>]

On Wed, May 20, 2026 at 03:55:02PM +0800, Barry Song wrote:
> On Wed, May 20, 2026 at 3:37 PM Uladzislau Rezki <urezki@gmail.com> wrote:
> >
> > On Thu, May 14, 2026 at 05:41:07PM +0800, Wen Jiang wrote:
> > > From: "Barry Song (Xiaomi)" <baohua@kernel.org>
> > >
> > > Try to align the vmap virtual address to PMD_SHIFT or a
> > > larger PTE mapping size hinted by the architecture, so
> > > contiguous pages can be batch-mapped when setting PMD or
> > > PTE entries.
> > >
> > > Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> > > Signed-off-by: Wen Jiang <jiangwen6@xiaomi.com>
> > > Tested-by: Xueyuan Chen <xueyuan.chen21@gmail.com>
> > > ---
> > >  mm/vmalloc.c | 31 ++++++++++++++++++++++++++++++-
> > >  1 file changed, 30 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > > index c30a7673e..b3389c8f1 100644
> > > --- a/mm/vmalloc.c
> > > +++ b/mm/vmalloc.c
> > > @@ -3591,6 +3591,35 @@ static int __vmap_huge(unsigned long addr, unsigned long end,
> > >       return err;
> > >  }
> > >
> > > +static struct vm_struct *get_aligned_vm_area(unsigned long size, unsigned long flags)
> > > +{
> > > +     unsigned int shift = (size >= PMD_SIZE) ? PMD_SHIFT :
> > > +                             arch_vmap_pte_supported_shift(size);
> > > +     struct vm_struct *vm_area = NULL;
> > > +
> > > +     /*
> > > +      * Try to allocate an aligned vm_area so contiguous pages can be
> > > +      * mapped in batches.
> > > +      */
> > > +     while (1) {
> > > +             unsigned long align = 1UL << shift;
> > > +
> > > +             vm_area = __get_vm_area_node(size, align, PAGE_SHIFT, flags,
> > > +                             VMALLOC_START, VMALLOC_END,
> > > +                             NUMA_NO_NODE, GFP_KERNEL,
> > > +                             __builtin_return_address(0));
> > > +             if (vm_area || shift <= PAGE_SHIFT)
> > > +                     goto out;
> > > +             if (shift == PMD_SHIFT)
> > > +                     shift = arch_vmap_pte_supported_shift(size);
> > > +             else if (shift > PAGE_SHIFT)
> > > +                     shift = PAGE_SHIFT;
> > > +     }
> > > +
> > > +out:
> > > +     return vm_area;
> > > +}
> > > +
> > IMO, we should get rid of this while(1) loop. It looks like you need to
> > handle just few cases. 3?
> 
Hello, Barry!

> 
> I don’t quite understand what you mean — are you suggesting
> calling __get_vm_area_node() three times? We try 2MB first,
> then 64KB, and finally 4KB. If 2MB succeeds, there is no
> reason to try 64KB. Likewise, if 64KB succeeds, there is no
> need to fall back to 4KB.
> 
I mean either to make three cases kind of open-coded:

...
if (size >= PMD_SIZE)
  alloc_vm_area_with_shift(PMD_SHIFT);
  if (vm_area)
    return vm_area;

shift = get_supported_shift(size)
if (shift > PAGE_SHIFT)
  alloc_vm_area_with_shift(shift);
    if (vm_area)
      return vm_area;

return alloc_vm_area_with_shift(PAGE_SHIFT)
...

or put everything into the: for (i = 0; i < 3; i++) - that way it will
finish in any case and for user it is obvious that we handle max 3 scenario.

> >
> >
> > shift min value is PAGE_SHIFT, could you please clarify when it can be less?
> 
> I guess this should be changed to "==" ?
> 
I assume shift can not be less than PAGE_SHIFT :)

--
Uladzislau Rezki