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