Re: [PATCH v2 03/20] hugetlb_vmemmap: Always flush TLB if needed upon PTE remapping

[Muchun Song]

Hi James,

> On Oct 3, 2026, at 08:21, James Houghton <jthoughton@xxxxxxxxxx> wrote:
> 
> vmemmap_remap_range() can potentially leave the vmemmap in a partially
> optimized state. In this case, a TLB flush should be done if the walk
> caller asked for it, even if the optimization was not fully applied.
> 
> A follow-up patch will more completely deal with partially-optimized
> folios.

Why are we dealing with partial optimization cases right from
the start?

I know that in patch 12, you try cmpxchg 16 times, and if it
fails, it leads to the partial optimization state. What I want
to ask is, why do we limit the maximum number of attempts? Is
it because during testing you found that this kind of failure
is very common? If we did not limit the number of attempts,
was it because during testing you found that the scheduling
latency caused here is very large? Or is this just defensive
programming? In my view, scenarios with such intense
concurrent access to these struct page structures should be
very rare.

If your testing did not find any problems, my suggestion is
not to set an upper limit on the number of cmpxchg attempts.
This should be able to simplify the design of the initial
version of the code to a certain extent. If we want to
determine whether an upper limit should be added in the
future, I think we can add timing monitoring in the arm64
try_update_vmemmap_pte function and trigger a warning when
the measured time exceeds a certain threshold. If someone
reports this kind of warning to the community in the future,
then I think we can improve it at that time.

What do you think?

Thanks,
Muchun

> 
> Signed-off-by: James Houghton <jthoughton@xxxxxxxxxx>
> ---
> mm/hugetlb_vmemmap.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c
> index 0057fa2a16a8..1a2645c382ba 100644
> --- a/mm/hugetlb_vmemmap.c
> +++ b/mm/hugetlb_vmemmap.c
> @@ -163,13 +163,11 @@ static int vmemmap_remap_range(unsigned long start, unsigned long end,
> 	ret = walk_kernel_page_table_range(start, end, &vmemmap_remap_ops,
>    					NULL, walk);
> 	mmap_read_unlock(&init_mm);
> - 	if (ret)
> - 		return ret;
> 
> 	if (walk->remap_pte && !(walk->flags & VMEMMAP_REMAP_NO_TLB_FLUSH))
> 		flush_tlb_kernel_range(start, end);
> 
> - 	return 0;
> + 	return ret;
> }
> 
> /*
> -- 
> 2.56.0.rc1.315.gc6ed9934b7-goog
>