Eliminate wasted physical memory in vm_page_startup()

[alc]

Summary

Over the years, the code and comments in vm_page_startup() have diverged in one respect. When determining how many page structures to allocate, contrary to what the comments say, the code does not account for the overhead of a page structure per page of physical memory. This revision changes the code to match the comments in all but one case. (That there exists a case where the overhead of a page structure per page cannot be accounted for is, in fact, why the code and comments diverged.)

For example, on an amd64 machine with 32 GB of physical memory, this revision recovers 21 MB of previously wasted physical memory. In general, the amount of recovered memory will be proportional to the size of physical memory.

Test Plan

If I configure hw.physmem on the same amd64 machine to 5 GB, as expected, vm_cnt.v_page_count does not change, because the biggest physical memory chunk is below the 4 GB mark in the physical address space.

[alc]

Trivially change one snippet of code to better align with a comment.

[alc]

A future improvement would be to calculate a weak upper bound on the amount of physical memory allocated by vm_page_startup(), and determine if the last physical memory chunk is large enough, rather than using the biggest chunk unconditionally.

[kib]

It seems that I understand how the 'saving' formula was obtained, e.g. for the PHYSSEG_DENSE case, the count of the vm_page_t structures should satisfy the inequality

(high_avail - low_avail) - count * sizeof(struct vm_page) <= count * PAGE_SIZE

but it is not clear by only looking at the code. I probably would need much longer time to understand this, if not the patch annotation. Might be, add either the formula above, or some other wording, to the comment about overhead ?

vm/vm_page.c

423 ↗

(On Diff #23712)

The comment right before the function probably needs editing. Use of term 'page cell' seems to be unique for the whole source base. Also, I do not think that the mentioned hash exists for quite some time. The object queue radix tree is hardly that thing.

516 ↗

(On Diff #23712)

I am not sure if the code under this #if block strongly depends on the previous #if block ('Allocate a bitmap' one). Did you considered moving this block under the scope of the previous #if ?

590 ↗

(On Diff #23712)

Am I right that you mean in the updated comment, that for arches where pmap_map() just points to DMAP, the guard page is just wasted ? If yes, would it make sense to e.g. avoid it on amd64 and arm64 ?

[alc]

vm/vm_page.c

423 ↗

(On Diff #23712)

I agree. The use of the term "cell" here is a holdover from the 1980's, when the vm_page size on VAX was 4KB, and the pmap transparently dealt with the underlying VAX page size being 512 bytes.

Would you prefer to separate these proposed changes from this revision or include them?

[alc]

vm/vm_page.c

516 ↗

(On Diff #23712)

It does depend. No, I did not really consider it. This snippet is identical to another snippet below, modulo the comment. I don't feel strongly about this, one way or the other.

590 ↗

(On Diff #23712)

To be clear, we waste a virtual page address, but no physical memory besides a single PTE. That's why I didn't conditionalize this snippet. Again, I don't feel strongly one way or the other. I'm happy to do as you wish.

[kib]

vm/vm_page.c

423 ↗

(On Diff #23712)

It looks logical to include them, since the function gets updates. But it really does not matter, as far as the confusing comment is replaced.

590 ↗

(On Diff #23712)

In my opinion, it is pointless to make this gap, but it adds clutter to the code.

[alc]

Revise some comments.

[markj]

vm/vm_page.c

524 ↗

(On Diff #23747)

Why not move this assignment to after the following #ifdef? It took me a while to spot.