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

From: Suren Baghdasaryan
Date: Thu Apr 11 2024 - 13:10:22 EST


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