Re: Changes to reduce pressure on uvm_pageqlock

[Andrew Doran <ad%netbsd.org@localhost>]

Ok here's is a more complete patch for the change to uvm_pageqlock.  I have
edited by hand to remove lots of unrelated changes so it's to get a picture
of the changes - it won't apply or compile.

	http://www.netbsd.org/~ad/2019/partial.diff

(It's time consuming to detangle this from the rest of my changes which are
per-CPU stat collection, a topology-aware page allocator that dispenses with
the "per-cpu" lists we have now, and yamt's radixtree).

There are a few more things I want to look at tomorrow including the
PQ_READAHEAD flag (it's not safe now) whether the pagedaemon needs to know
about the pdpolicy lock at all, and a last go over it all checking
everything.

The earlier changes to vm_page I mentioned for alignment and the page
replacement policy I am going to postpone because radixtree changes the
picture bigtime and more testing is required. (That's a separate review.)

As to the reason why: at the start of November on my machine system time for
a kernel build was 3200-3300s.  With this plus the remaining changes it's
down to 850s so far and with !DIAGNOSTIC about 580s.

Andrew

On Fri, Dec 06, 2019 at 04:34:50PM +0000, Andrew Doran wrote:

> I ended up taking a disliking to these changes so I went back and took
> another look at the problem yesterday evening.
> 
> There are two distinct sets of data involved that need protection: page
> identity (connected anon, uobj, wiring, loaning) and page replacement state.
> 
> There are limited points of connection between those two sets, so it's easy
> to do two locks.  We can give the pdpolicy code a lock for its state and let
> it manage that lock, which only it and the pagedaemon need to know about. 
> The lock can be private to the file (uvm_pdpolicy_clock / clockpro.c), with
> a couple of hooks for the pagedaemon to acquire and release it.
> 
> With that done, we'd then only need one version of uvm_pageactivate() etc,
> because those functions will no longer care about uvm_pageqlock.
> 
> I tried it out: the code looks more elegant and the scheme works in practice
> with contention on uvm_pageqlock now quite minor, and the sum of contention
> on the two locks lower than it was on uvm_pageqlock prior.
> 
> With the locking for the pdpolicy code being private to the file I think it
> would then be easier to experiment with different strategies to reduce
> contention further.
> 
> http://www.netbsd.org/~ad/2019/pdpolicy.diff
> 
> Andrew
> 
> On Tue, Dec 03, 2019 at 12:41:25AM +0000, Andrew Doran wrote:
> > Hi,
> > 
> > I have some straightforward changes for uvm to improve the situation on
> > many-CPU machines.  I'm going to break them into pieces to make them easier
> > to review (only this first piece and what's already in CVS is ready).
> > 
> > I have carefuly measured the impact of these over hundreds of kernel builds,
> > using lockstat, tprof and some custom instrumentation so I'm confident that
> > for each, the effects at least are of value.
> > 
> > Anyway I'd be grateful if someone could take a look.  This one is about
> > reducing pressure on uvm_pageqlock, and cache misses on struct vm_page.
> > 
> > Cheers,
> > Andrew
> > 
> > 
> > http://www.netbsd.org/~ad/2019/uvm1.diff
> > 
> > 
> > vm_page: cluster largely static fields used during page lookup in the first
> > 64-bytes.  Increase wire_count to 32-bits, and add a field for use of the
> > page replacement policy.  This leaves 2 bytes spare in a word locked by
> > uvm_fpageqlock/uvm_pageqlock which I want to use later for changes to the
> > page allocator.  It also brings vm_page up to 128 bytes on amd64.
> > 
> > New functions:
> > 
> > => uvmpdpol_pageactivate_p()
> > 
> > For page replacement policy.  Returns true if pdpol thinks activation info
> > would be useful enough to cause disruption to page queues, vm_page and
> > uvm_fpageqlock.  For CLOCK this returns true if page is not active, or was
> > not activated within the last second.
> > 
> > => uvm_pageenqueue1()
> > 
> > Call without uvm_pageqlock.  Acquires the lock and enqueues the page only
> > if not already enqueued.
> > 
> > => uvm_pageactivate1()
> > 
> > Call without uvm_pageqlock.  Acquires the lock and activates the page only
> > if uvmpdpol_pageactivate_p() says yes.  No similar change for deactivate nor
> > dequeue as they are much more definite.
> > 
> > => uvm_pagefree1()
> > 
> > First part of uvm_pagefree() - strip page of identity.  Requires
> > uvm_pageqlock if associated with an object.
> > 
> > => uvm_pagefree2()
> > 
> > Second part of uvm_pagefree().  Send page to free list.  No locks required.