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

[James Houghton]

On Wed, Oct 21, 2026 at 2:54 AM Muchun Song <muchun.song@xxxxxxxxx> wrote:
>
> 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.

This was done defensively, yes. With 16 retries, I never hit this
without fault injection. (Perhaps I should try with no retries...)

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

>From userspace, you could use kpageflags or page_idle to continuously
access a single struct page over and over again. I think that's a
pretty good reason to have a bound. Also, the AF can be set even
without an architectural memory access (so a mispredicted branch can
still set AF, for example); that also makes me want to have a bound.

Catalin also said wanted a bound here. See the bottom of this reply at [1].

So I think we should leave it in. The extra complexity is pretty much
entirely in patch #4[2], and I don't think it's too large of a change.
I hope you agree. :)

[1] https://lore.kernel.org/linux-mm/aoRn3Fn1dhTdsk6d@xxxxxxx/
[2] https://lore.kernel.org/linux-mm/20261003002123.505555-5-jthoughton@xxxxxxxxxx/