Use pmap_enter(..., psind=1) in vm_fault_populate()

[alc]

Summary

Use pmap_enter(..., psind=1) in vm_fault_populate() rather than relying on automatic promotion in the pmap.

Test Plan

I've tested this change with kern.ipc.shm_use_phys="1" and shmat() on shared segments of varying sizes and alignments.

[kib]

vm/vm_fault.c

489 ↗

(On Diff #42990)

With this structure of the loop, it is not clear to me is it better to just call vm_fault_fill_hold() outside the loop.

497 ↗

(On Diff #42990)

There is no single use of continue in the loop. Why not remove the step from for() and remove if (npages > 1) check there ?

[alc]

vm/vm_fault.c

489 ↗

(On Diff #42990)

I thought about that too. My conclusions were (1) that the branch predictor would quickly learn that m_hold == NULL and (2) that moving the hold outside the loop requires a more complex check:

if (pidx <= fs->first_pindex &&
    fs->first_pindex < pidx + OFF_TO_IDX(pagesizes[psind]))
        vm_fault_fill_hold(m_hold, &m[fs->first_pindex - pidx];

or

if (m_hold != NULL && pidx <= fs->first_pindex &&

to avoid pointless calculations.

497 ↗

(On Diff #42990)

We still need to use vm_page_next() to step from the last 4KB page within a superpage to the next 4KB page. They might not be physically contiguous.

Strictly speaking, the check is redundant even if npages == 1. The arithmetic will Do The Right Thing(tm) for npages == 1.

I could rewrite the step as

...; pidx += npages, m = vm_page_next(&m[npages - 1])

and eliminate this snippet entirely.

[kib]

vm/vm_fault.c

489 ↗

(On Diff #42990)

Ok, then vm_fault_fill_hold() does not make sense anymore, since there were two uses. The last remaining one should be inlined then.

497 ↗

(On Diff #42990)

This is better IMO.

[alc]

Change the for () step expression, simplifying the code.

[alc]

vm/vm_fault.c

489 ↗

(On Diff #42990)

I won't argue strongly one way or the other on this point. You choose.

The next thing on my TODO list this weekend is adding an madvise() option that triggers immediate population and mapping of superpages from OBJT_DEFAULT objects at the top of the shadow chain. My intent is to use a patched JVM as the test case. I mention this because I foresee refactoring this code, specifically, this for loop, as a helper function that also gets called near the end of vm_fault_hold().

I'd like to finish that and then return to your proposed object synchronization changes in that same place.