Re: [PATCH 03/12] mm: hugetlb_vmemmap: simplify vmemmap head page handling

[Qi Zheng <qi.zheng@linux.dev>]

On 10/20/26 7:58 PM, Muchun Song wrote:
> The new head page is already tracked on the vmemmap page list before
> remapping starts.  Passing it separately gives the remap walker two
> ways to refer to the same page and unnecessarily complicates its
> interface.
> 
> Use the page list as the single handoff mechanism for the head page,
> removing the redundant state from the walker.
> 
> No functional change intended.
> 
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> ---
>   mm/hugetlb_vmemmap.c | 17 +++++++----------
>   1 file changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c
> index 8b3abb562599..164e93e980bf 100644
> --- a/mm/hugetlb_vmemmap.c
> +++ b/mm/hugetlb_vmemmap.c
> @@ -25,7 +25,6 @@
>    *
>    * @remap_pte:		called for each lowest-level entry (PTE).
>    * @nr_walked:		the number of walked pte.
> - * @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:	the list head of the vmemmap pages that can be freed
>    *			or is mapped from.

The @vmemmap_pages description may need updating as well. After this
change, the list is no longer only an output list for pages to be freed;
its first entry is also consumed as the new head backing page. It would
be helpful to document this input contract explicitly.

Other than that, once Lance's comment is addressed:

Reviewed-by: Qi Zheng <qi.zheng@linux.dev>

Thanks,
Qi

> @@ -37,7 +36,6 @@ struct vmemmap_remap_walk {
>   					     struct vmemmap_remap_walk *walk);
>   
>   	unsigned long		nr_walked;
> -	struct page		*vmemmap_head;
>   	struct page		*vmemmap_tail;
>   	struct list_head	*vmemmap_pages;
>   
> @@ -208,10 +206,13 @@ static void vmemmap_remap_pte(pte_t *pte, unsigned long addr,
>   	pte_t entry;
>   
>   	/* Remapping the head page requires r/w */
> -	if (unlikely(walk->nr_walked == 0 && walk->vmemmap_head)) {
> +	if (unlikely(walk->nr_walked == 0)) {
> +		struct page *head;
> +
>   		VM_WARN_ON_ONCE(!PageHead((const struct page *)addr));
>   
> -		list_del(&walk->vmemmap_head->lru);
> +		head = list_first_entry(walk->vmemmap_pages, struct page, lru);
> +		list_del(&head->lru);
>   
>   		/*
>   		 * Makes sure that preceding stores to the page contents from
> @@ -220,7 +221,7 @@ static void vmemmap_remap_pte(pte_t *pte, unsigned long addr,
>   		 */
>   		smp_wmb();
>   
> -		entry = mk_pte(walk->vmemmap_head, PAGE_KERNEL);
> +		entry = mk_pte(head, PAGE_KERNEL);
>   	} else {
>   		VM_WARN_ON_ONCE(!PageTail((const struct page *)addr));
>   
> @@ -289,7 +290,6 @@ 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.
> - * @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.
> @@ -298,7 +298,6 @@ 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,
> -			      struct page *vmemmap_head,
>   			      struct page *vmemmap_tail,
>   			      struct list_head *vmemmap_pages,
>   			      unsigned long flags)
> @@ -306,7 +305,6 @@ static int vmemmap_remap_free(unsigned long start, unsigned long end,
>   	int ret;
>   	struct vmemmap_remap_walk walk = {
>   		.remap_pte	= vmemmap_remap_pte,
> -		.vmemmap_head	= vmemmap_head,
>   		.vmemmap_tail	= vmemmap_tail,
>   		.vmemmap_pages	= vmemmap_pages,
>   		.flags		= flags,
> @@ -547,8 +545,7 @@ static int __hugetlb_vmemmap_optimize_folio(const struct hstate *h,
>   	 * they can be freed by the caller.
>   	 */
>   	ret = vmemmap_remap_free(vmemmap_start, vmemmap_end,
> -				 vmemmap_head, vmemmap_tail,
> -				 vmemmap_pages, flags);
> +				 vmemmap_tail, vmemmap_pages, flags);
>   out:
>   	if (ret)
>   		folio_clear_hugetlb_vmemmap_optimized(folio);