Re: [RESEND RFC PATCH 1/2] mm/vmalloc: Do not align size to huge size

[Uladzislau Rezki <urezki@gmail.com>]

On Fri, Dec 12, 2025 at 09:57:00AM +0530, Dev Jain wrote:
> vmalloc() consists of the following:
> 
> (1) find empty space in the vmalloc space -> (2) get physical pages from
> the buddy system -> (3) map the pages into the pagetable.
> 
> It turns out that the cost of (1) and (3) is pretty insignificant. Hence,
> the cost of vmalloc becomes highly sensitive to physical memory allocation
> time.
> 
> Currently, if we decide to use huge mappings, apart from aligning the start
> of the target vm_struct region to the huge-alignment, we also align the
> size. This does not seem to produce any benefit (apart from simplification
> of the code), and there is a clear disadvantage - as mentioned above, the
> main cost of vmalloc comes from its interaction with the buddy system, and
> thus requesting more memory than was requested by the caller is suboptimal
> and unnecessary.
> 
> This change is also motivated due to the next patch ("arm64/mm: Enable
> vmalloc-huge by default"). Suppose that some user of vmalloc maps 17 pages,
> uses that mapping for an extremely short time, and vfree's it. That patch,
> without this patch, on arm64 will ultimately map 16 * 2 = 32 pages in a
> contiguous way. Since the mapping is used for a very short time, it is
> likely that the extra cost of mapping 15 pages defeats any benefit from
> reduced TLB pressure, and regresses that code path. 
> 
> Signed-off-by: Dev Jain <dev.jain@arm.com>
> ---
>  mm/vmalloc.c | 38 ++++++++++++++++++++++++++++++--------
>  1 file changed, 30 insertions(+), 8 deletions(-)
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index ecbac900c35f..389225a6f7ef 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -654,7 +654,7 @@ static int vmap_small_pages_range_noflush(unsigned long addr, unsigned long end,
>  int __vmap_pages_range_noflush(unsigned long addr, unsigned long end,
>  		pgprot_t prot, struct page **pages, unsigned int page_shift)
>  {
> -	unsigned int i, nr = (end - addr) >> PAGE_SHIFT;
> +	unsigned int i, step, nr = (end - addr) >> PAGE_SHIFT;
>  
>  	WARN_ON(page_shift < PAGE_SHIFT);
>  
> @@ -662,7 +662,8 @@ int __vmap_pages_range_noflush(unsigned long addr, unsigned long end,
>  			page_shift == PAGE_SHIFT)
>  		return vmap_small_pages_range_noflush(addr, end, prot, pages);
>  
> -	for (i = 0; i < nr; i += 1U << (page_shift - PAGE_SHIFT)) {
> +	step = 1U << (page_shift - PAGE_SHIFT);
> +	for (i = 0; i < ALIGN_DOWN(nr, step); i += step) {
>  		int err;
>  
>  		err = vmap_range_noflush(addr, addr + (1UL << page_shift),
> @@ -673,8 +674,9 @@ int __vmap_pages_range_noflush(unsigned long addr, unsigned long end,
>  
>  		addr += 1UL << page_shift;
>  	}
> -
> -	return 0;
> +	if (IS_ALIGNED(nr, step))
> +		return 0;
> +	return vmap_small_pages_range_noflush(addr, end, prot, pages + i);
>  }
>  
Can we improve the readability?

<snip>
index 25a4178188ee..14ca019b57af 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -655,6 +655,8 @@ int __vmap_pages_range_noflush(unsigned long addr, unsigned long end,
                pgprot_t prot, struct page **pages, unsigned int page_shift)
 {
        unsigned int i, step, nr = (end - addr) >> PAGE_SHIFT;
+       unsigned int nr_aligned;
+       unsigned long chunk_size;

        WARN_ON(page_shift < PAGE_SHIFT);

@@ -662,20 +664,24 @@ int __vmap_pages_range_noflush(unsigned long addr, unsigned long end,
                        page_shift == PAGE_SHIFT)
                return vmap_small_pages_range_noflush(addr, end, prot, pages);

-       step = 1U << (page_shift - PAGE_SHIFT);
-       for (i = 0; i < ALIGN_DOWN(nr, step); i += step) {
-               int err;
+       step = 1U << (page_shift - PAGE_SHIFT); /* small pages per huge chunk. */
+       nr_aligned = ALIGN_DOWN(nr, step);
+       chunk_size = 1UL << page_shift;

-               err = vmap_range_noflush(addr, addr + (1UL << page_shift),
+       for (i = 0; i < nr_aligned; i += step) {
+               int err = vmap_range_noflush(addr, addr + chunk_size,
                                        page_to_phys(pages[i]), prot,
                                        page_shift);
                if (err)
                        return err;

-               addr += 1UL << page_shift;
+               addr += chunk_size;
        }
-       if (IS_ALIGNED(nr, step))
+
+       if (i == nr)
                return 0;
+
+       /* Map the tail using small pages. */
        return vmap_small_pages_range_noflush(addr, end, prot, pages + i);
 }
<snip>


>  int vmap_pages_range_noflush(unsigned long addr, unsigned long end,
> @@ -3197,7 +3199,7 @@ struct vm_struct *__get_vm_area_node(unsigned long size,
>  	unsigned long requested_size = size;
>  
>  	BUG_ON(in_interrupt());
> -	size = ALIGN(size, 1ul << shift);
> +	size = PAGE_ALIGN(size);
>  	if (unlikely(!size))
>  		return NULL;
>  
> @@ -3353,7 +3355,7 @@ static void vm_reset_perms(struct vm_struct *area)
>  	 * Find the start and end range of the direct mappings to make sure that
>  	 * the vm_unmap_aliases() flush includes the direct map.
>  	 */
> -	for (i = 0; i < area->nr_pages; i += 1U << page_order) {
> +	for (i = 0; i < ALIGN_DOWN(area->nr_pages, 1U << page_order); i += (1U << page_order)) {
>
nr_blocks?

>  		unsigned long addr = (unsigned long)page_address(area->pages[i]);
>  
>  		if (addr) {
> @@ -3365,6 +3367,18 @@ static void vm_reset_perms(struct vm_struct *area)
>  			flush_dmap = 1;
>  		}
>  	}
> +	for (; i < area->nr_pages; ++i) {
> +		unsigned long addr = (unsigned long)page_address(area->pages[i]);
> +
> +		if (addr) {
> +			unsigned long page_size;
> +
> +			page_size = PAGE_SIZE;
> +			start = min(addr, start);
> +			end = max(addr + page_size, end);
> +			flush_dmap = 1;
> +		}
> +	}
>  
>  	/*
>  	 * Set direct map to something invalid so that it won't be cached if
> @@ -3673,6 +3687,7 @@ vm_area_alloc_pages(gfp_t gfp, int nid,
>  	 * more permissive.
>  	 */
>  	if (!order) {
> +single_page:
>  		while (nr_allocated < nr_pages) {
>  			unsigned int nr, nr_pages_request;
>  
> @@ -3704,13 +3719,18 @@ vm_area_alloc_pages(gfp_t gfp, int nid,
>  			 * If zero or pages were obtained partly,
>  			 * fallback to a single page allocator.
>  			 */
> -			if (nr != nr_pages_request)
> +			if (nr != nr_pages_request) {
> +				order = 0;
>  				break;
> +			}
>  		}
>  	}
>  
>  	/* High-order pages or fallback path if "bulk" fails. */
>  	while (nr_allocated < nr_pages) {
> +		if (nr_pages - nr_allocated < (1UL << order)) {
> +			goto single_page;
> +		}
>  		if (!(gfp & __GFP_NOFAIL) && fatal_signal_pending(current))
>  			break;
>
Yes, it requires more attention. That "goto single_page" should be
eliminated, IMO. We should not jump between blocks, logically the
single_page belongs to "order-0 alloc path".

Probably it requires more refactoring to simplify it.

>  
> @@ -5179,7 +5199,9 @@ static void show_numa_info(struct seq_file *m, struct vm_struct *v,
>  
>  	memset(counters, 0, nr_node_ids * sizeof(unsigned int));
>  
> -	for (nr = 0; nr < v->nr_pages; nr += step)
> +	for (nr = 0; nr < ALIGN_DOWN(v->nr_pages, step); nr += step)
> +		counters[page_to_nid(v->pages[nr])] += step;
> +	for (; nr < v->nr_pages; ++nr)
>  		counters[page_to_nid(v->pages[nr])] += step;
>
Can we fit it into one loop? Last tail loop continuous adding step?

--
Uladzislau Rezki