vm: read-locked fault handling for backing objects ; simplest take

[mjg]

Summary

commit 4988ddd9f529239ebb510ca8c5d05654f1ef9553
Author: Mateusz Guzik <[e-mail]>
Date:   Tue Mar 7 20:56:54 2023 +0000

    vm: read-locked fault handling for backing objects
    
    This is almost the simplest patch which manages to avoid write locking
    for backing objects, as a result mostly fixing vm object contention
    problems.
    
    What is not fixed:
    1. cacheline ping pong due to read-locks
    2. cacheline ping pong due to pip
    3. cacheling ping pong due to object busying
    4. write locking on first object
    
    On top of it the use VM_OBJECT_UNLOCK instead of explicitly tracking the
    state is slower multithreaded that it needs to be, done for simplicity
    for the time being.
    
    Sample lock profiling results doing -j 104 buildkernel on tmpfs:
    before:
    71446200 (rw:vmobject)
    14689706 (sx:vm map (user))
    4166251 (rw:pmap pv list)
    2799924 (spin mutex:turnstile chain)
    
    after:
    19940411 (rw:vmobject)
    8166012 (rw:pmap pv list)
    6017608 (sx:vm map (user))
    1151416 (sleep mutex:pipe mutex)
    
    Reviewed by:
    Differential Revision:  https://reviews.freebsd.org/D38964

commit 388c60608388bbe484322fbd01f03551602f0db5
Author: Mateusz Guzik <[e-mail]>
Date:   Tue Mar 7 20:47:44 2023 +0000

    vm: add VM_OBJECT_UNLOCK
    
    Reviewed by:
    Differential Revision:  https://reviews.freebsd.org/D38964

commit be2b02c0b73e1e46e0148ec0f825e902b62d3630
Author: Mateusz Guzik <[e-mail]>
Date:   Sun Aug 7 13:05:47 2022 +0000

    vm: move up object lock asserts in fault functions
    
    No functional changes.
    
    Reviewed by:
    Differential Revision:  https://reviews.freebsd.org/D38964

Replacement for D36038.

[mjg]

sys/vm/vm_fault.c

1459

note that for example here the code already unlocked the object and locked it again later. if this is correct behavior as is, the patch does not change it.

[mjg]

sys/vm/vm_fault.c

1434

this bit may be a little iffy but is not strictly needed for the scalability boost

what it can do instead is set a flag which disables readlocking and FAULT_RESTART

[mjg]

• add the readlock disablement thing

sys/vm/vm_fault.c

1434

so I added the restart here. I checked this case does not happen in cases I'm worried about anyway.

[kib]

sys/vm/vm_fault.c

1066

Free requires write lock for fs->object.

1435–1436

Mark, this comment needs to be updated for the removal of the default pager.

[mjg]

sys/vm/vm_fault.c

1066

why? only the page is modified and to my reading it has to be xbusy on entry

[mjg]

sys/vm/vm_fault.c

1066

now that i wrote it, i think i see vm_page_free_toq -> vm_page_free_prep -> vm_page_object_remove

I'll think about what to do here

[kib]

sys/vm/vm_fault.c

1066

Because the page is freed from the object.

This is actually asserted in fault_page_free(), and then in vm_page_free()->vm_page_free_prep()->vm_page_object_remove().

[kib]

sys/vm/vm_fault.c

1066

In fact I think that the situation is rare, in the sense that to get the object read-locked and need to free the page, you must find invalid page on the queue. If the fault handler allocated the page, the thread already locked the object exclusive.

It would be fine to leave the page alone, but it would prevent the shadowing from working.

[markj]

sys/vm/vm_fault.c

215

The write lock must be held in order to free a page.

1434

It would be worth adding a debug counter for this case. Akin to vm.stats.page and vm.stats.object we could have vm.stats.fault for tracking how often "rare" code paths in the fault handler actually get executed.

1435–1436

D38985

sys/vm/vm_object.h

271 ↗

(On Diff #118509)

BTW, this is a cheaper implementation of VM_OBJECT_DROP.

[mjg]

• write lock for fault_page_free and restart if not possible

[mjg]

So the idea is to tryupgrade and if that fails drop the page on the floor and goto RetryFault, which I would hope sorts out the problem -- the bad page will be found second time around with the write lock or be sorted out by someone else.

I verified with the printf that the condition *does* occur (about once for buildkernel on tmpfs) and several times on zfs, but it does not impede on lock contention reduction.

I have to say this *really* needs a stable lockless way to do the fault. seqc around obj collapse and freeing would definitely help here.

[mjg]

sys/vm/vm_fault.c

1593

bunch of ifs instead of switch because of the last case doing 'break' of the outer loop -- i find this to be much more readable

[mjg]

• whack WIP leftovers
• update commitm essage with numbers

the patch is under test by pho

[markj]

sys/vm/vm_fault.c

130

This isn't a "fault parameter", it belongs under "control state" .