Re: [PATCH v9 09/10] mm/vmalloc: map contiguous pages in batches for vmap() if possible

[Uladzislau Rezki <urezki@gmail.com>]

On Thu, Oct 01, 2026 at 04:09:24PM +0800, Wen Jiang wrote:
> On Wed, 30 Sept 2026 at 23:05, Uladzislau Rezki <urezki@gmail.com> wrote:
> >
> > On Wed, Sep 23, 2026 at 02:28:31PM +0800, Wen Jiang wrote:
> > > From: "Barry Song (Xiaomi)" <baohua@kernel.org>
> > >
> > > In many cases, the pages passed to vmap() may include high-order
> > > pages. For example, the systemheap often allocates pages in descending
> > > order: order 8, then 4, then 0. Currently, vmap() iterates over every
> > > page individually—even pages inside a high-order block are handled
> > > one by one.
> > >
> > > This patch detects physically contiguous pages (regardless of whether
> > > they are compound or non-compound) by scanning with
> > > num_pages_contiguous(), and maps them as a single contiguous block
> > > whenever possible. The mapping order is determined by taking the
> > > minimum of the contiguous page count and the pfn alignment, allowing
> > > graceful degradation when pfn alignment is less than the contiguous
> > > range.
> > >
> > > Pages with the same page_shift are coalesced and mapped via
> > > vmap_pages_range_noflush_walk() to avoid page table rewalk.
> > >
> > > As users typically allocate memory in descending orders (e.g.
> > > 8 → 4 → 0), once an order-0 page is encountered, we stop scanning
> > > for contiguous pages since subsequent pages are likely order-0 as well.
> > >
> > > Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
> > > Co-developed-by: Dev Jain <dev.jain@arm.com>
> > > Signed-off-by: Dev Jain <dev.jain@arm.com>
> > > Signed-off-by: Wen Jiang <jiangwen6@xiaomi.com>
> > > Tested-by: Xueyuan Chen <xueyuan.chen21@gmail.com>
> > > Tested-by: Leo Yan <leo.yan@arm.com>
> > > ---
> > >  mm/vmalloc.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++--
> > >  1 file changed, 80 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > > index a43965016eb4d..37cddb7f45a4f 100644
> > > --- a/mm/vmalloc.c
> > > +++ b/mm/vmalloc.c
> > > @@ -3576,6 +3576,85 @@ static inline unsigned int vm_shift(pgprot_t prot, unsigned long size)
> > >       return arch_vmap_pte_supported_shift(size);
> > >  }
> > >
> > > +static inline int get_vmap_batch_order(struct page **pages,
> > > +             pgprot_t prot, unsigned int nr_pages)
> > >
> > get_vmap_mapping_order()?
> >
> 
> Agreed, that's a clearer name.
> 
> > > +{
> > > +     unsigned long pfn;
> > > +     unsigned int nr_contig;
> > > +     int order;
> > > +
> > > +     if (!IS_ENABLED(CONFIG_HAVE_ARCH_HUGE_VMAP))
> > > +             return 0;
> > > +
> > > +     /* Limit nr_pages by pfn alignment */
> > > +     pfn = page_to_pfn(*pages);
> > > +     if (pfn > 0)
> > > +             nr_pages = min_t(size_t, nr_pages, 1UL << __ffs(pfn));
> > > +
> > > +     nr_contig = num_pages_contiguous(pages, nr_pages);
> > > +     if (nr_contig < 2)
> > > +             return 0;
> > > +
> > > +     order = ilog2(nr_contig);
> > > +
> > > +     if (vm_shift(prot, PAGE_SIZE << order) == PAGE_SHIFT)
> > > +             return 0;
> > > +
> > > +     return order;
> > > +}
> > > +
> > > +static int vmap_pages_range_batched(unsigned long addr, unsigned long end,
> > > +             pgprot_t prot, struct page **pages)
> > > +{
> > > +     const unsigned int nr_pages = (end - addr) >> PAGE_SHIFT;
> > > +     unsigned int prev_shift = 0, batch_idx = 0;
> > > +     unsigned long batch_start = addr, batch_end = addr;
> > > +     int err;
> > > +
> > > +     err = kmsan_vmap_pages_range_noflush(addr, end, prot, pages,
> > > +                                          PAGE_SHIFT, GFP_KERNEL);
> > > +
> > > +     if (err)
> > > +             goto out;
> > > +
> > > +     for (unsigned int i = 0; i < nr_pages; ) {
> > > +             unsigned int shift = PAGE_SHIFT +
> > > +                     get_vmap_batch_order(pages + i, prot, nr_pages - i);
> > > +
> > > +             if (!i)
> > > +                     prev_shift = shift;
> > > +
> > > +             if (shift != prev_shift) {
> > > +                     err = vmap_pages_range_noflush_walk(batch_start, batch_end,
> > > +                                     prot, pages + batch_idx, prev_shift);
> > > +                     if (err)
> > > +                             goto out;
> > > +                     prev_shift = shift;
> > > +                     batch_start = batch_end;
> > > +                     batch_idx = i;
> > > +             }
> > > +
> > > +             /*
> > > +              * Once we fail to batch pages, we expect to fail batching
> > > +              * for all remaining pages, so just give up.
> > > +              */
> > > +             if (shift == PAGE_SHIFT)
> > > +                     break;
> > > +
> > > +             batch_end += 1UL << shift;
> > > +             i += 1U << (shift - PAGE_SHIFT);
> > > +     }
> > > +
> > > +     /* Remaining */
> > > +     if (batch_start < end)
> > > +             err = vmap_pages_range_noflush_walk(batch_start, end, prot,
> > > +                             pages + batch_idx, prev_shift);
> > > +
> > > +out:
> > > +     flush_cache_vmap(addr, end);
> > > +     return err;
> > >
> > On error, if kmsan_vmap_pages_range_noflush() succeed you do not do a
> > proper cleanup?
> >
> > i.e. to cleanup KMSAN metadata: kmsan_vunmap_range_noflush(addr, end);
> >
> 
> The KMSAN metadata is cleaned up by the caller: on error vmap calls
> vunmap(area->addr), which reaches kmsan_vunmap_range_noflush() through
> remove_vm_area() -> free_unmap_vmap_area() -> vunmap_range_noflush().
> That is the same contract as the pre-existing vmap_pages_range() path,
> which likewise relied on the caller's vunmap() to clean up. So there
> is no behavioral
> change here.
> 
Right. With better naming:

Reviewed-by: Uladzislau Rezki (Sony) <urezki@gmail.com>

--
Uladzislau Rezki