Re: [PATCH 05/11] mm/hugetlb: Refactor code around vmemmap_walk

[Kiryl Shutsemau]

On Sat, Dec 06, 2025 at 04:42:30PM +0000, Usama Arif wrote:
> > @@ -308,7 +298,8 @@ static int vmemmap_remap_split(unsigned long start, unsigned long end,
> >   *		to remap.
> >   * @end:	end address of the vmemmap virtual address range that we want to
> >   *		remap.
> > - * @reuse:	reuse address.
> > + * @vmemmap_head: the page to be installed as first in the vmemmap range
> > + * @vmemmap_tail: the page to be installed as non-first in the vmemmap range
> >   * @vmemmap_pages: list to deposit vmemmap pages to be freed.  It is callers
> >   *		responsibility to free pages.
> >   * @flags:	modifications to vmemmap_remap_walk flags
> > @@ -316,69 +307,40 @@ static int vmemmap_remap_split(unsigned long start, unsigned long end,
> >   * Return: %0 on success, negative error code otherwise.
> >   */
> >  static int vmemmap_remap_free(unsigned long start, unsigned long end,
> > -			      unsigned long reuse,
> > +			      struct page *vmemmap_head,
> > +			      struct page *vmemmap_tail,
> >  			      struct list_head *vmemmap_pages,
> >  			      unsigned long flags)
> 
> Need to fix the doc above vmemmap_remap_free as it mentions reuse.

Ack.

> > +	vmemmap_remap_range(start + PAGE_SIZE, end, &walk);
> 
> 
> I think this should be vmemmap_remap_range(start, end, &walk)? Otherwise if start failed to remap,
> you wont restore it?

I think it should be safe to keep newly allocated vmemmap_head there and
free the old one.

I will add a comment.

> > @@ -592,9 +550,21 @@ static int __hugetlb_vmemmap_optimize_folio(const struct hstate *h,
> >  	 */
> >  	folio_set_hugetlb_vmemmap_optimized(folio);
> >  
> > +	nid = folio_nid(folio);
> > +	vmemmap_head = alloc_pages_node(nid, GFP_KERNEL, 0);
> 
> Should we add __GFP_NORETRY | __GFP_NOWARN here? It was there in the previous code. I am guessing
> that it was there in the previous code as its an optimization and if it fails its not a big issue.

I removed the fallback. Being noisy if we actually hit this in practice
makes sense to me. We can re-consider fallback path if we see it being a
problem.

> > @@ -602,8 +572,10 @@ static int __hugetlb_vmemmap_optimize_folio(const struct hstate *h,
> >  	 * mapping the range to vmemmap_pages list so that they can be freed by
> >  	 * the caller.
> >  	 */
> > -	ret = vmemmap_remap_free(vmemmap_start, vmemmap_end, vmemmap_reuse,
> > +	ret = vmemmap_remap_free(vmemmap_start, vmemmap_end,
> > +				 vmemmap_head, vmemmap_tail,
> >  				 vmemmap_pages, flags);
> 
> The doc above this also mentions vmemmap_reuse.

Ack.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov