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

[Matthew Wilcox <willy@infradead.org>]

On Wed, Apr 10, 2024 at 08:20:04PM -0400, Peter Xu wrote:
> On Thu, Apr 11, 2024 at 12:59:09AM +0100, Matthew Wilcox wrote:
> > On Wed, Apr 10, 2024 at 05:23:18PM -0400, Peter Xu wrote:
> > > On Wed, Apr 10, 2024 at 10:10:45PM +0100, Matthew Wilcox wrote:
> > > > > I can do some tests later today or tomorrow. Any suggestion you have on
> > > > > amplifying such effect that you have concern with?
> > > > 
> > > > 8 socket NUMA system, 800MB text segment, 10,000 threads.  No, I'm not
> > > > joking, that's a real customer workload.
> > > 
> > > Well, I believe you, but even with this, that's a total of 800MB memory on
> > > a giant moster system... probably just to fault in once.
> > > 
> > > And even before we talk about that into details.. we're talking about such
> > > giant program running acorss hundreds of cores with hundreds of MB text,
> > > then... hasn't the program developer already considered mlockall() at the
> > > entry of the program?  Wouldn't that greatly beneficial already with
> > > whatever granule of locks that a future fault would take?
> > 
> > I don't care what your theory is, or even what your benchmarking shows.
> > I had basically the inverse of this patch, and my customer's workload
> > showed significant improvement as a result.  Data talks, bullshit walks.
> > Your patch is NAKed and will remain NAKed.
> 
> Either would you tell me your workload, I may try it.
> 
> Or, please explain why it helps?  If such huge library is in a single VMA,
> I don't see why per-vma lock is better than mmap lock.  If the text is
> combined with multiple vmas, it should only help when each core faults at
> least on different vmas, not the same.

Oh, you really don't understand.  The mmap_lock is catastrophically
overloaded.  Before the per-VMA lock, every page fault took it for read,
and every call to mmap() took it for write.  Because our rwsems are
fair, once one thread has called mmap() it waits for all existing page
faults to complete _and_ blocks all page faults from starting until
it has completed.  That's a huge source of unexpected latency for any
multithreaded application.

Anything we can do to avoid taking the mmap_sem, even for read, helps any
multithreaded workload.  Your suggestion that "this is rare, it doesn't
matter" shows that you don't get it.  That you haven't found a workload
where you can measure it shows that your testing is inadequate.

Yes, there's added complexity with the per-VMA locks.  But we need it for
good performance.  Throwing away performance on a very small reduction
in complexity is a terrible trade-off.