Re: Changes to reduce pressure on uvm_pageqlock

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

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.