Various changes to pmap_ts_referenced()
ClosedPublic
Actions

Authored by alc on Sep 8 2016, 4:26 PM.

Details

Reviewers

kib
markj

Commits

rS305685: Various changes to pmap_ts_referenced()

Summary

Move PMAP_TS_REFERENCED_MAX out of the various pmap implementations and into vm/pmap.h, and describe what its purpose is. Eliminate the archaic "XXX" comment about its value. I don't believe that its exact value, e.g., 5 versus 6, matters.

Update the arm64 and riscv pmap implementations of pmap_ts_referenced() to opportunistically update the page's dirty field.

On amd64, use the PDE value already cached in a local variable rather than dereferencing a pointer again and again.

I'm primarily looking for feedback on the first point, especially the new comment describing PMAP_TS_REFERENCED_MAX.

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

alc updated this revision to Diff 20202.Sep 8 2016, 4:26 PM

alc retitled this revision from to Various changes to pmap_ts_referenced().

alc updated this object.

alc edited the test plan for this revision. (Show Details)

alc added reviewers: kib, markj.

Herald added subscribers: emaste, andrew. · View Herald TranscriptSep 8 2016, 4:26 PM

May be, it is better to move the comments from MD pmap.c into pmap.h ? I do not see much use in repeating the text for each architecture.

This revision is now accepted and ready to land.Sep 8 2016, 5:15 PM

markj accepted this revision.Sep 8 2016, 8:19 PM

markj edited edge metadata.

markj added inline comments.

vm/pmap.h
111	Perhaps it would be worth adding a kassert to this effect to the active queue scan.

alc added inline comments.Sep 10 2016, 4:27 PM

vm/pmap.h
111	Yes, I agree. However, armv4 and powerpc/aim{32,64} clear all of the bits. (mips has a practically non-functional reference bit emulation mechanism, but it will never trigger an overflow because it never returns a value greater than 1 from pmap_ts_referenced().)

In D7836#162545, @kib wrote:

May be, it is better to move the comments from MD pmap.c into pmap.h ? I do not see much use in repeating the text for each architecture.

Yes, I seriously considered this, but we don't have any similar comments in vm/pmap.h. The hair that I'm splitting here is that we're defining PMAP_TS_REFERENCED_MAX in vm/pmap.h and describing it at the point of definition but pmap_ts_referenced() is defined in pmap.c. I think your proposal would make sense as a part of an overall deduplication that described all of the public pmap functions in vm/pmap.h.

Closed by commit rS305685: Various changes to pmap_ts_referenced() (authored by alc). · Explain WhySep 10 2016, 4:49 PM

This revision was automatically updated to reflect the committed changes.