Re: [PATCH] mm: Always sanity check anon_vma first for per-vma locks

[Suren Baghdasaryan]

On Wed, Apr 10, 2024 at 1:26 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
>
> On Wed, Apr 10, 2024 at 01:06:21PM -0400, Peter Xu wrote:
> > anon_vma is a tricky object in the context of per-vma lock, because it's
> > racy to modify it in that context and mmap lock is needed if it's not
> > stable yet.
>
> I object to this commit message.  First, it's not a "sanity check".  It's
> a check to see if we already have an anon VMA.  Second, it's not "racy
> to modify it" at all.  The problem is that we need to look at other
> VMAs, for which we do not hold the lock.
>
> > So the trivial side effect of such patch is:
> >
> >   - We may do slightly better on the first WRITE of a private file mapping,
> >   because we can retry earlier (in lock_vma_under_rcu(), rather than
> >   vmf_anon_prepare() later).
> >
> >   - We may always use mmap lock for the initial READs on a private file
> >   mappings, while before this patch it _can_ (only when no WRITE ever
> >   happened... but it doesn't make much sense for a MAP_PRIVATE..) do the
> >   read fault with per-vma lock.
>
> But that's a super common path!  Look at 'cat /proc/self/maps'.  All
> your program text (including libraries) is mapped PRIVATE, and never
> written to (except by ptrace, I guess).

Uh, indeed I didn't realize this would be the side-effect from this
early check. And that's exactly why I wanted Matthew's input on this
in [1].

>
> NAK this patch.
>