Re: [PATCH 2/6] mm/vmalloc: set area's page_order after allocation succeeds

On Mon, Aug 17, 2026 at 07:31:17PM +0200, Uladzislau Rezki wrote:
> On Sun, Aug 16, 2026 at 01:59:25PM +0300, Mike Rapoport (Microsoft) wrote:
> > __vmalloc_area_node() calls set_vm_area_page_order() to set area's
> > page_order before actually allocating pages to populate the area.
> > 
> > If allocation of large pages in HUGE_VMAP case fails midway, this leaves
> > the area with elevated page_order throughout the cleanup path.
> > 
> > There is no actual issue with this because the only place that currently
> > relies on area->page_order on the cleanup path is the loop calculating
> > the direct map alias range in vm_reset_perms() and it anyway skips
> > unpopulated pages.
> > 
> > But having set_vm_area_page_order() in the middle of __vmalloc_area_node()
> > makes things very obscure, hard to reason about and error prone against
> > future changes of the cleanup path.
> > 
> > Move the call to set_vm_area_page_order() after __vmalloc_area_node()
> > succeeded where page order is guaranteed.
> > 
> > Signed-off-by: Mike Rapoport (Microsoft) <[email protected]>
> > ---
> >  mm/vmalloc.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > index 22566e0b6e38..6822f0fe9583 100644
> > --- a/mm/vmalloc.c
> > +++ b/mm/vmalloc.c
> > @@ -3901,8 +3901,7 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
> >  		goto fail;
> >  	}
> >  
> > -	set_vm_area_page_order(area, page_shift - PAGE_SHIFT);
> > -	page_order = vm_area_page_order(area);
> > +	page_order = page_shift - PAGE_SHIFT;
> >  
> >  	/*
> >  	 * High-order nofail allocations are really expensive and
> > @@ -4106,6 +4105,14 @@ void *__vmalloc_node_range_noprof(unsigned long size, unsigned long align,
> >  	if (!ret)
> >  		goto fail;
> >  
> > +	/*
> > +	 * Set area->page_order once it's known exactly that the order of the
> > +	 * pages the area contains.
> > +	 * Even if we succeeded to partially populate the area with large pages,
> > +	 * still treat the area as populated with order-0 pages.
> > +	 */
> > +	set_vm_area_page_order(area, shift - PAGE_SHIFT);
> > +
> >  	/*
> >  	 * Mark the pages as accessible, now that they are mapped.
> >  	 * The condition for setting KASAN_VMALLOC_INIT should complement the
> > 
> > -- 
> > 2.53.0
> > 
> OK, can we just set it right after the:
> 
> 	area->nr_pages = vm_area_alloc_pages(
> 			vmalloc_gfp_adjust(gfp_mask, page_order), node,
> 			page_order, nr_small_pages, area->pages);
> 
> 
> succeeds?

Here it's still not clear if vm_area_page_order() succeeded :)

I can move it just before

 	return area->addr;

in __vmalloc_area_node().
 
> I am not sure there is a good reason to move it out of the
> __vmalloc_area_node().
> 
> --
> Uladzislau Rezki

-- 
Sincerely yours,
Mike.