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

[Muchun Song]

> On Oct 22, 2026, at 05:59, James Houghton <jthoughton@xxxxxxxxxx> wrote:
> 
> 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

page_idle does not support HugeTLB pages, right? We should only consider
/proc/kpageflags; reading this file requires privilege — unless a privileged
user intentionally creates malicious software.

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

However, I don't think this scenario would cause a significant amount of
concurrent reads.

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

I see, but I'd still like to explain my reasoning and see what everyone
thinks.

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

Because I personally feel that having only part of the vmemmap optimized
is a somewhat awkward intermediate state — the all-or-nothing semantics
seems more natural to me. At least until a clear problem actually arises,
I'd prefer not to consider it. At the same time, we can also keep the design
simple.

Thanks,
Muchun

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