Re: Changes to reduce pressure on uvm_pageqlock

[Chuck Silvers <chuq%chuq.com@localhost>]

I like this new version much more than the previous version.
The new diff looks incomplete though, it uses the "pdpol" field in
vm_page that was new in the previous diff.  It looks like there
should be changes in uvm_pdaemon.c to go with the new diff too?
Could you send the complete new diff again please?


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.

I like this.


> With that done, we'd then only need one version of uvm_pageactivate() etc,
> because those functions will no longer care about uvm_pageqlock.

The pageq lock is named that because it currently protects the pageq's.  :-)
If it's not going to protect the pageq's anymore then it ought to be renamed.
Probably better to just work on eliminating it though.

In your new diff, the pageq lock is still used by the pagedaemon
(for uvmpd_trylockowner() as you noted in the comment).
It would be good not to have a global lock involved in changing page identity,
and with the pageq's now protected by a separate lock, we should be able to
eliminate the need for the pageq lock when freeing a page with an RCUy thing
to prevent freeing uvm_object and uvm_anon structures while the pagedaemon might
still be accessing them via the vm_page fields.  We should be able to improve
the locking for pg->loan_count with a similar RCUy thing... my recollection is
that the only reason loan_count uses the pageq lock currently is to stabilize
the page identity while locking the owning object.  It might make sense to have
pg->wire_count protected by the pdpol lock too (though I have some thoughts
about changing the nature of wire_count that would probaby make that not
make sense, I haven't had time to think that through yet).

But anyway, I'll stop rambling at this point so we can focus on the diff at hand.
:-)

-Chuck


> 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