amd64 pmap: demotion changes for kib@
ClosedPublic
Actions

Authored by alc on Mon, Jun 30, 8:15 AM.

Details

Reviewers

kib
markj

Commits

rG88c8cba04395: amd64 pmap: preallocate another page table page in pmap_demote_DMAP()

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

alc requested review of this revision.Mon, Jun 30, 8:15 AM

alc created this revision.

alc mentioned this in D50970: amd64 pmap: eliminate two explicit panics on low mem conditions.Mon, Jun 30, 8:17 AM

kib accepted this revision.Mon, Jun 30, 10:38 AM

This revision is now accepted and ready to land.Mon, Jun 30, 10:38 AM

Or, do you want me to integrate this into the series of patches for D50970?

In D51091#1166112, @kib wrote:

Or, do you want me to integrate this into the series of patches for D50970?

Modulo my latest comments of the last few minutes, I am happy with all of the changes related to pmap_demote_DMAP(). I am still thinking about the other changes relating to pmap_enter_pde(). Can you go ahead and commit the pmap_demote_DMAP() changes? I am indifferent to whether you or I commit the changes in this revision. You can decide. I would happily update this revision after your pmap_demote_DMAP() changes are committed, and commit it.

sys/amd64/amd64/pmap.c
6147	I kept VM_ALLOC_INTERRUPT here, but your patch removed it from pmap_demote_pdpe(). Which do we want?
10061	I've been thinking about adding an assertion that the *pde is not managed to help explain passing NULL for the lockp. Opinion:?

alc added a reviewer: markj.Mon, Jun 30, 5:04 PM

kib added inline comments.Tue, Jul 1, 1:31 AM

sys/amd64/amd64/pmap.c
6147	You mean the allocation in pmap_demote_pdpe()? I removed VM_ALLOC_INTERRUPT on principle that the flag is the last resort to to prevent the fatal consequences, and since the allocation failure no longer panic, we do not need to be so demanding for the page. The intended use for DMAP demotion is some driver load, so at worst we get the inability to attach to some device. On the other hand, pmap_demote_pde() failure could be still fatal. For instance, pmap_unwire() has one more panic.
10061	You mean, MPASS((*pde & PG_MANAGED) == 0); This would still require a comment linking it to NULL lockp, but how can it be opposed?

alc marked 2 inline comments as done.Tue, Jul 1, 8:01 AM

alc added inline comments.

sys/amd64/amd64/pmap.c
6147	You wrote, "You mean the allocation in pmap_demote_pdpe()?" Yes.
10061	Yes, that is what I mean. Alternatively, pmap_demote_pde_mpte() could KASSERT(lockp != NULL \|\| (oldpde & PG_MANAGED) == 0,