Page MenuHomeFreeBSD

Rename pmap_is_write() to pmap_l3_writable().
ClosedPublic

Authored by markj on Thu, Dec 6, 10:00 PM.

Details

Summary

This is consistent with pmap_l3_valid(), for instance. Fix style bugs
along the way. No functional change intended.

Test Plan

Booted under QEMU and spike.

Diff Detail

Repository
rS 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

markj created this revision.Thu, Dec 6, 10:00 PM
markj edited the test plan for this revision. (Show Details)Thu, Dec 6, 10:03 PM
markj added reviewers: br, jhb.
jhb added inline comments.Thu, Dec 6, 10:48 PM
sys/riscv/riscv/pmap.c
360 ↗(On Diff #51686)

If we support superpages eventually then we might not want l3 in this name? That might be true for pmap_l3_valid as well? (Or will we only use this function on leaf PTEs?)

markj marked an inline comment as done.Thu, Dec 6, 11:02 PM
markj added inline comments.
sys/riscv/riscv/pmap.c
360 ↗(On Diff #51686)

I was wondering about that too, as I'm starting to work on superpage support. We'll probably eventually have a use for an is_writable predicate for superpage mappings (ditto for the other predicates below). One wrinkle, though, is that we use distinct types (pd_entry_t vs. pt_entry_t). I'm not sure if that indicates that we should instead use a macro or just test bits inline. I lean somewhat towards the latter, since the flag names are pretty clear.

jhb added inline comments.Thu, Dec 6, 11:13 PM
sys/riscv/riscv/pmap.c
360 ↗(On Diff #51686)

I think it is also probably readable to just test bits inline. What does amd64 do? That's probably the best pmap to be copying from.

markj marked an inline comment as done.Thu, Dec 6, 11:18 PM
markj added inline comments.
sys/riscv/riscv/pmap.c
360 ↗(On Diff #51686)

amd64 is hamstrung by the fact that the bit definitions vary depending on the pmap type, so it's more or less forced to look up the definition once and then use inline tests.

I'll think about it a bit more, but I'm inclined to just get rid of these predicates and inline everything. I'll upload the diff here once that's done.

markj updated this revision to Diff 51742.Fri, Dec 7, 10:00 PM
  • Get rid of the (inconsistently used) predicates and just use inline tests.
jhb accepted this revision.Fri, Dec 7, 11:47 PM
This revision is now accepted and ready to land.Fri, Dec 7, 11:47 PM
This revision was automatically updated to reflect the committed changes.