Page MenuHomeFreeBSD

amd64 pmap: explain pteindex
ClosedPublic

Authored by kib on Jun 8 2020, 1:27 PM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 23 2024, 3:42 AM
Unknown Object (File)
Feb 9 2024, 3:06 AM
Unknown Object (File)
Jan 22 2024, 5:40 PM
Unknown Object (File)
Jan 16 2024, 2:02 PM
Unknown Object (File)
Jan 13 2024, 6:52 PM
Unknown Object (File)
Dec 20 2023, 2:23 PM
Unknown Object (File)
Dec 1 2023, 10:59 PM
Unknown Object (File)
Nov 28 2023, 10:20 AM
Subscribers

Details

Summary

Add description of how the page table pages are numbered by pmap_allocpte().

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kib requested review of this revision.Jun 8 2020, 1:27 PM
sys/amd64/amd64/pmap.c
3789 ↗(On Diff #72824)

"The page index of the PTE at address va is defined as follows" would be more clear to me.

3791 ↗(On Diff #72824)

Duplicate '='

3798 ↗(On Diff #72824)

s/is it/it is/

3799 ↗(On Diff #72824)

If I understand this correctly, "height" might be clearer than "rank" in the context of a tree.

kib marked 3 inline comments as done.Jun 25 2020, 7:49 PM
kib added inline comments.
sys/amd64/amd64/pmap.c
3789 ↗(On Diff #72824)

IMO it is too vague. I used the basis of your sentence, but extended it as following:

* The page index of the page containing PTE at address va is defined
* as follows:
3791 ↗(On Diff #72824)

It was a split formula, I reformatted to move formulas to display to avoid line break.

kib marked an inline comment as done.

Handle the first round of comments.

markj added inline comments.
sys/amd64/amd64/pmap.c
3813 ↗(On Diff #73656)

Should it be, "for the page table page (last level)"?

For the other entries, we are missing "the" as well.

3826 ↗(On Diff #73656)

Maybe clarify also that the PML4 page always has pindex 0?

3789 ↗(On Diff #72824)

It should be "the PTE".

"the page" sounds like there is a unique page, but really we are talking about each PTP in the hierarchy. So maybe: "The page indices of the page table pages encountered while translating virtual address va are defined as follows:"

This revision is now accepted and ready to land.Jun 26 2020, 7:36 PM
kib marked 2 inline comments as done.Jun 26 2020, 8:02 PM
kib added inline comments.
sys/amd64/amd64/pmap.c
3826 ↗(On Diff #73656)

PML4 page does not have index for the purpose of _pmap_allocpte() which uses this numbering. The PML4 page is statically allocated at the pinit() time.

In fact, the pteindex 0 is assigned to the pte which maps at zero.

sys/amd64/amd64/pmap.c
3826 ↗(On Diff #73656)

Indeed, I just thought it would be useful to clarify this fact for completeness.

kib marked an inline comment as done.

Change herald sentence as suggested by Mark. Add articles.

This revision now requires review to proceed.Jun 26 2020, 8:05 PM
kib marked an inline comment as done.

Separately mention the root page.

This revision is now accepted and ready to land.Jun 26 2020, 8:42 PM
sys/amd64/amd64/pmap.c
3818–3819 ↗(On Diff #73713)

What follows will be clearer if this sentence explicitly defines ptepindex as the page index of a page table page.

3822 ↗(On Diff #73713)

"... the PDE that maps the page table page."

3831–3833 ↗(On Diff #73713)

I think that this sentence is hard to understand.

kib marked 3 inline comments as done.

Alc suggestions.

This revision now requires review to proceed.Jun 26 2020, 10:02 PM
markj added inline comments.
sys/amd64/amd64/pmap.c
3818 ↗(On Diff #73719)

There should be a comma after "page indices", otherwise it is not really clear what "i.e." refers to.

This revision is now accepted and ready to land.Jun 27 2020, 4:09 AM
kib marked an inline comment as done.

comma

This revision now requires review to proceed.Jun 27 2020, 4:27 AM
This revision was not accepted when it landed; it landed in state Needs Review.Jun 27 2020, 7:29 PM
Closed by commit rS362706: amd64 pmap: explain ptepindex. (authored by kib). · Explain Why
This revision was automatically updated to reflect the committed changes.