Page MenuHomeFreeBSD

Use regular stores for updating PTEs.
ClosedPublic

Authored by markj on Jan 2 2019, 6:16 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Mar 12, 2:52 PM
Unknown Object (File)
Thu, Mar 12, 1:28 PM
Unknown Object (File)
Thu, Mar 12, 8:50 AM
Unknown Object (File)
Mon, Mar 9, 4:56 AM
Unknown Object (File)
Sat, Mar 7, 10:32 AM
Unknown Object (File)
Sat, Mar 7, 12:17 AM
Unknown Object (File)
Thu, Mar 5, 8:47 AM
Unknown Object (File)
Tue, Mar 3, 4:07 PM
Subscribers

Details

Summary

We currently use an atomic_swap to atomically store and load a PTE, but
this is excessive. Use regular loads and stores instead when possible.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kib added inline comments.
sys/riscv/riscv/pmap.c
1901 ↗(On Diff #52485)

Could this be expressed by in-place and-ing instead of load/and/store ?

This revision is now accepted and ready to land.Jan 2 2019, 9:46 PM
sys/riscv/riscv/pmap.c
1901 ↗(On Diff #52485)

I think it's more consistent with the current code. However, if we wanted to drop pmap_load/pmap_store entirely and just use regular assignments for simple loads and stores, then replacing this with 'entry &= ~PTE_W' probably would make sense. amd64 uses regular assignments ('*pte = newpte;', etc.) It's not clear to me that the wrappers are useful if you don't really need them.

sys/riscv/riscv/pmap.c
1901 ↗(On Diff #52485)

(Note that this code is changed in D18719.)

I somewhat prefer to use the wrappers since they guard accesses of memory that may be updated concurrently by hardware. In practice they probably aren't necessary; it's hard to see any cases in the amd64 pmap where a compiler optimization might introduce an unexpected side effect, but I don't see why it isn't possible.

This revision was automatically updated to reflect the committed changes.