Page MenuHomeFreeBSD

riscv pmap: add a demotion assertion
ClosedPublic

Authored by mhorne on Tue, Jan 12, 1:43 AM.

Details

Summary

Add a KASSERT to pmap_remove_write() to ensure that the page obtained
from the PV list is not a superpage where we don't expect one. An
identical KASSERT already exists in pmap_clear_modify().

This may help with debugging the panic reported in PR 250866, in which
l3 was found to be NULL. Adding this KASSERT will help narrow down the
cause of this panic the next time it occurs.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

I think pmap_page_test_mappings() and pmap_page_wired_mappings() could use a similar treatment.

This revision is now accepted and ready to land.Tue, Jan 12, 1:49 AM
sys/riscv/riscv/pmap.c
3919

Slightly unrelated, but do you know why the fcmpset/retry loop is required here? pmap_clear_modify is structured similarly but performs pmap_clear_bits on the PTE instead.

Also, can it ever advance without updating oldl3?

I think pmap_page_test_mappings() and pmap_page_wired_mappings() could use a similar treatment.

Sure, I can add those as well.

sys/riscv/riscv/pmap.c
3919

Forget the second question, I double checked the semantics of fcmpset.

sys/riscv/riscv/pmap.c
3919

I believe it was done this way to handle races with the hardware setting PTE_D. We want to make sure we don't end up clearing PTE_D without observing that it was set. pmap_clear_modify() doesn't care so it can simply clear the bits.

atomic_fcmpset_* updates the old value upon failure, in contrast with atomic_cmpset_*.

Add asserts to pmap_page_test_mappings() and pmap_page_wired_mappings().

Use __func__ to avoid extra line wrapping.

This revision now requires review to proceed.Tue, Jan 12, 2:07 PM
This revision is now accepted and ready to land.Tue, Jan 12, 2:35 PM
This revision was automatically updated to reflect the committed changes.