Eliminate redundant KASSERT()s in vm_fault() and its helpers

[alc]

Summary

Near the beginning of vm_fault_hold() we have

if (wired)
        fault_type = prot | (fault_type & VM_PROT_COPY);
else
        KASSERT((fault_flags & VM_FAULT_WIRE) == 0,
            ("!wired && VM_FAULT_WIRE"));

Therefore, I don't see the point of the KASSERT(wired, ...)s removed by this change. They are redundant except in the case that we have to repeat the map lookup, in which case wired may have hypothetically changed.

To be clear about my motivation, when I refactor the loop for enqueueing/wiring pages out of vm_fault_populate() into a helper function, I don't want to have to pass wired to it just for the KASSERT().

[alc]

vm/vm_fault.c

1251 ↗

(On Diff #43034)

Hmm. Could another thread initiate a wiring operation while the thread that got us to this point had the map unlocked? Should we be repeating this check from earlier in vm_fault_hold()?

if (fs.entry->eflags & MAP_ENTRY_IN_TRANSITION &&
    fs.entry->wiring_thread != curthread) {
        ...

[kib]

vm/vm_fault.c

1251 ↗

(On Diff #43034)

I think you are right. We should do it in the if (!fs.lookup_still_valid) block.

But I start thinking that instead of MAP_ENTRY_IN_TRANSITION, vm_map_busy() and other mechanisms which try to avoid some parallelism with wiring, we should much simplify it and make it much more straightforward. I mean, lets add sx lock to them map, take it exclusive around wiring/unwiring, and shared around all other vm_map_XXX() top level entry points().

It may be too naive, because vm_fault() can be entered from some deep stack (vm_map_wire() is an example, but of course there the fault handler must avoid taking the sx at all), but other places like copyouts while owning vnode locks etc. Still, the generic idea is to avoid parallel wire and anything else.

[alc]

vm/vm_fault.c

1251 ↗

(On Diff #43034)

I'm going to commit this change as is and follow up on the MAP_ENTRY_IN_TRANSITION issue later.