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

[Muchun Song]

> On Nov 1, 2026, at 14:31, Qi Zheng <qi.zheng@xxxxxxxxx> wrote:
> 
> 
> 
> 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@xxxxxxxxxxxxx>
>> ---
>>  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

Yes, the comment is out of date even without this patch.

> 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.

But I can fix this in this along the way.

> 
> Other than that, once Lance's comment is addressed:
> 
> Reviewed-by: Qi Zheng <qi.zheng@xxxxxxxxx>

Thanks.

> 
> 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);
>