Page MenuHomeFreeBSD

arm64/pmap: Sparsify pv_table
ClosedPublic

Authored by scottph on Aug 20 2020, 6:55 AM.

Details

Summary

MFC after: 1 week
Sponsored by: Ampere Computing, Inc.

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

I augmented vm_phys_seg with a void *md_first field that is used for the pointer to the starting entry in pv_table. Is void * alright, or should I be making some new md_something type for the field?

I augmented vm_phys_seg with a void *md_first field that is used for the pointer to the starting entry in pv_table. Is void * alright, or should I be making some new md_something type for the field?

I think void * is ok for now.

I do wonder if it'd be a bit nicer to have external arrays of size VM_PHYSSEG_MAX for the pv_table and vm_reserv arrays, instead of embedding external data in the vm_phys segment array. I don't have a strong preference either way.

This revision is now accepted and ready to land.Aug 27 2020, 5:10 PM
sys/arm64/arm64/pmap.c
203 ↗(On Diff #76186)

VM_PAGE_TO_PHYS(m) instead of m->phys_addr

sys/arm64/arm64/pmap.c
182–183 ↗(On Diff #76186)

I don't see any calls to this function.

sys/arm64/arm64/pmap.c
1052 ↗(On Diff #76186)

pvh instead of j?

1124–1128 ↗(On Diff #76186)

The same situation can arise in vm_reserv.c as here. I'm curious why you sought to handle it differently here.

This revision now requires review to proceed.Aug 31 2020, 10:28 PM
scottph added inline comments.
sys/arm64/arm64/pmap.c
182–183 ↗(On Diff #76186)

Some callers of pa_to_pvh() do remain, but the call sites aren't modified in this change so you'll need to go into the context lines to see them.

1124–1128 ↗(On Diff #76186)

That was choice was based on this comment in vm_reserv.c line 216:

This array is analoguous in function to vm_page_array. It differs in the
respect that it may contain a greater number of useful reservation
structures than there are (physical) superpages. These "invalid"
reservation structures exist to trade-off space for time in the
implementation of vm_reserv_from_page(). Invalid reservation structures are
distinguishable from "valid" reservation structures by inspecting the
reservation's "pages" field. Invalid reservation structures have a NULL
"pages" field.

So I didn't add in extra code to ensure the same "invalid" sentinel entry was used. I could see that possibly being surprising to someone in the future though, so I can make the same adjustment there if you'd like.

I wasn't sure what the result would be if partial pages were represented by different entries in this code, but it seemed that they are more than just sentinel values, so I added in this logic.

This revision is now accepted and ready to land.Sep 21 2020, 3:30 PM
This revision was automatically updated to reflect the committed changes.
scottph marked an inline comment as done.