vm_reserv: Introduce vm_reserv_alloc_npages
Needs ReviewPublic
Actions

Authored by bnovkov on Thu, Oct 10, 9:39 PM.

Details

Reviewers

markj
alc
kib

Summary

This change adds an interface for allocating multiple pages
from a reservation. It is meant to be used instead of invoking
vm_reserv_alloc_page in a loop.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 59894
Build 56779: arc lint + arc unit

Event Timeline

bnovkov created this revision.Thu, Oct 10, 9:39 PM

Herald added a subscriber: imp. · View Herald TranscriptThu, Oct 10, 9:39 PM

bnovkov requested review of this revision.Thu, Oct 10, 9:39 PM

Harbormaster completed remote builds in B59894: Diff 144610.Thu, Oct 10, 9:39 PM

bnovkov added a parent revision: D47047: vm_object: Add wrapper for bumping generation counter.Thu, Oct 10, 9:39 PM

bnovkov added a child revision: D47049: vm_page: Introduce vm_page_radixdone_batch.

bnovkov added a child revision: D47050: vm_page: Add batch page allocation routine.Thu, Oct 10, 9:41 PM

bnovkov removed a parent revision: D47047: vm_object: Add wrapper for bumping generation counter.

bnovkov removed a child revision: D47049: vm_page: Introduce vm_page_radixdone_batch.

markj added inline comments.Mon, Oct 14, 1:40 PM

sys/vm/vm_reserv.c
812	How is it possible for the pages to not be physically contiguous? Don't they always come from a single reservation?

Correct comment pointed out by @markj .

bnovkov added inline comments.Tue, Oct 15, 5:22 PM

sys/vm/vm_reserv.c
812	You are right, after taking a closer look at the reservation lifecycle I see that this cannot happen. I've corrected the comment. We should also probably break from the allocation loop (and possibly even panic after further checks) instead of continuing when encountering an allocated page.

markj added inline comments.Tue, Oct 15, 5:24 PM

sys/vm/vm_reserv.c
922	Can this condition actually arise? That would mean that the page is already allocated, which is presumably due to a bug in the caller.
924	If you assume that the returned run of pages is always contiguous, then you don't need to return a pointer to each page. You can simply return a point to the first page in the run, and a count. Then, it's probably profitable to have a variant of vm_reserv_populate() which adjusts the reservation for `count` pages starting at `index`, e.g., using bit_nset() rather than a loop.

bnovkov added inline comments.Tue, Oct 15, 6:48 PM

sys/vm/vm_reserv.c
922	Other than a caller bug or a `vm_reserv` bug I don't think so. I still think it is useful to check for this, but we can have this check along with the approach you described in the other comment.
924	Right, I'll work that assumption into the code. Callers can "unpack" the returned run into individual 0-order pages if need be.

alc added inline comments.Tue, Oct 15, 8:41 PM

sys/vm/vm_reserv.c
922	It happens due to vm_page_rename()s by, for example, vm_fault(). (I'm incredibly busy this week, when my workload lightens next week, I will elaborate.)

markj added inline comments.Tue, Oct 15, 8:44 PM

sys/vm/vm_reserv.c
922	Ahh, right, I forgot that it's possible for a page to be allocated from a reservation without belonging to the object that "owns" the reservation. But, in this case I believe we have to terminate the loop, not continue.