pctrie: define lookup_readonly

[dougm]

Summary

pctrie_lookup() does not currently modify the pctrie it is examining. Some callers depend on this when they invoke it holding a read lock. Define a readonly version of pctrie_lookup() that promises not to modify the pctrie in the future, and use it in place of pctrie_lookup() in places that hold only a read lock.

No functional change is expected.

[markj]

sys/kern/subr_pctrie.c

343 ↗

(On Diff #157647)

A bit more detail would be helpful, for instance, "Returns the value stored at the index without mutating the trie and assuming that access is synchronized by a read lock or mutex."

sys/vm/vm_page.c

1767

Since a write lock is of course sufficient.

sys/vm/vm_radix.h

84 ↗

(On Diff #157647)

Shouldn't vm_page_lookup() assert that the write lock is held then?

[dougm]

sys/vm/vm_radix.h

84 ↗

(On Diff #157647)

If the first line of vm_page_lookup() is

VM_OBJECT_ASSERT_WLOCKED(object);

then I crash at boot time like this:

panic: Lock vmobject not exclusively locked @ ../../../vm/vm_page.c:1751

cpuid = 13
time = 1750967544
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe014a207a30
vpanic() at vpanic+0x136/frame 0xfffffe014a207b60
panic() at panic+0x43/frame 0xfffffe014a207bc0
__rw_assert() at __rw_assert+0x94/frame 0xfffffe014a207bd0
vm_page_lookup() at vm_page_lookup+0x27/frame 0xfffffe014a207bf0
vm_fault() at vm_fault+0xd81/frame 0xfffffe014a207d60
vm_fault_trap() at vm_fault_trap+0x65/frame 0xfffffe014a207da0
trap_pfault() at trap_pfault+0x27b/frame 0xfffffe014a207e10
trap() at trap+0x5c2/frame 0xfffffe014a207f30
calltrap() at calltrap+0x8/frame 0xfffffe014a207f30
--- trap 0xc, rip = 0x23e896, rsp = 0x8208f46d0, rbp = 0x8208f4730 ---

Is there a different assertion I should be using here?

[dougm]

Update comments, asserts, convert one more lookup to lookup_readonly.

[kib]

sys/vm/vm_radix.h

84 ↗

(On Diff #157647)

The point is that if your intent is to allow vm_radix_lookup() to modify trie, then the exclusive lock must be held.

Otherwise, what is the point of adding _readonly()? Or put it differently, why vm_page_lookup() cannot be switched to vm_radix_lookup_readonly()?

[dougm]

Avoid defining new pctrie methods; just copy the pctrie (root) before the search and search the copy so that only the root of the copy can be modified.

[dougm]

Update to account for call to vm_page_lookup that was dropped from vm_fault.c.