Mark initial page table entries as wired
ClosedPublic
Actions

Authored by jtl on Feb 8 2018, 5:57 PM.

Details

Reviewers

kib
markj
alc

Commits

rS329171: Mark the pages used for the initial page-table entries as wired. This

Summary

Mark the pages used for the initial page-table entries as wired. This makes them consistent with the way other page-table pages are allocated.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

jtl created this revision.Feb 8 2018, 5:57 PM

Herald added a subscriber: imp. · View Herald TranscriptFeb 8 2018, 5:57 PM

Harbormaster completed remote builds in B14876: Diff 39051.Feb 8 2018, 5:57 PM

Is there any other reason than the consistency with other page table pages for this change ?

sys/amd64/amd64/pmap.c
1241 ↗	(On Diff #39051)	I suggest to directly assign the wire_count to 1, then you do not need the page lock.

In D14269#299107, @kib wrote:

Is there any other reason than the consistency with other page table pages for this change ?

This is not due to a bug. Actually, I noticed this while I was auditing this code to see if it was the cause of an obscure bug. I couldn't find anyplace where this code caused a problem. (And, it appears our obscure bug has another cause.)

However, since r274556, these pages are added to the VM system as physical segments. This means that things in the VM system could be scanning them. For example, vm_phys_scan_contig() may scan these. It may run vm_page_scan_contig() over these to see if there are enough free contiguous pages. Now, vm_page_scan_contig() will reject these because m->order == VM_NFREEORDER. However, it seems like it would be easy to introduce a bug somewhere in one of these scanning routines to make it actually consider these free pages.

For that reason, it seemed like a good idea to mark these as wired.

sys/amd64/amd64/pmap.c
1241 ↗	(On Diff #39051)	OK. I was trying to use the API, but I agree that taking the lock is completely unnecessary here (other than to use the API).

markj added inline comments.Feb 8 2018, 8:22 PM

sys/amd64/amd64/pmap.c
1241 ↗	(On Diff #39051)	v_wire_count ought to be bumped too for consistency.

Address review feedback.

Harbormaster completed remote builds in B14900: Diff 39095.Feb 9 2018, 5:58 PM

jtl marked 3 inline comments as done.Feb 9 2018, 5:59 PM

kib accepted this revision.Feb 9 2018, 6:12 PM

kib added inline comments.

sys/amd64/amd64/pmap.c
1248 ↗	(On Diff #39095)	You can increment v_wire_count by nkpt after the loop. More, I think that the increment does not need to be atomic.

This revision is now accepted and ready to land.Feb 9 2018, 6:12 PM

markj accepted this revision.Feb 9 2018, 8:45 PM

markj added inline comments.

sys/amd64/amd64/pmap.c
1248 ↗	(On Diff #39095)	I'm not sure I'd bother avoiding the locked increment. It's not much more expensive, and the reason for the inconsistency isn't immediately obvious when reading the code.