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

[James Houghton]

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

Oh yeah, page_idle only uses LRU pages (which HugeTLB pages are not),
that's right.

For kpageflags (and kpagecount and kpagecgroup), I still think it is
"bad" to provide a mechanism that allows userspace, even privileged
userspace, to permanently freeze a CPU. Fuzzers could find it, for
example.

I wonder what Catalin thinks about my reasoning. :)

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

The fact that AF can be set without an architectural access means we
can't really reason about what may or may not set the AF. It
definitely seems implausible for the microarchitecture to continuously
attempt to set the AF for a generally-unused but valid PTE many times
in a row, but it isn't forbidden (as far as I can tell).

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

Thanks, yeah I'm curious what others think too. I can go either way really.

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

Yeah that's fair. Thanks for your feedback, Muchun.