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

Kiryl Shutsemau
2025-12-08T10:31:03Z

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