vm_page_grab_pages: fetch page ranges
ClosedPublic
Actions

Authored by dougm on Thu, May 8, 6:23 AM.

Details

Reviewers

alc
markj
kib

Commits

rG8703b572b9f6: vm_page_grab_pages: fetch page ranges

Summary

Define an iterator based function for reading a range of consecutive, non-NULL leaves from a pctrie. Adapt it to vm pages. Use it in vm_page_grab_pages to fetch more than one page at a time.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

dougm requested review of this revision.Thu, May 8, 6:23 AM

dougm created this revision.

markj accepted this revision.Thu, May 8, 2:20 PM

markj added inline comments.

sys/vm/vm_page.c

5199

Not directly related to this diff, but I believe this zeroing is redundant when the page is freshly allocated. vm_page_alloc_iter(VM_ALLOC_ZERO) will have already zeroed it.

Can we exploit this to simplify control flow a bit? I wonder if the loop body could be written like this:

ahead = vm_radix_iter_lookup_range(&pages, pindex + i, &ma[i], count - i);
if (ahead > 0) {
    for (int j = 0; j < ahead; j++) {
        m = ma[i + j];
        if (!vm_page_tryacquire(...)) {
            ...
        }
        if (vm_page_none_valid(m) && (allocflags & VM_ALLOC_ZERO) != 0) {
            pmap_zero_page(m);
            vm_page_valid(m);
        }
        vm_page_grab_release(m, allocflags);
    }
    i += ahead;
} else {
    if ((allocflags & VM_ALLOC_NOCREAT) != 0)
        break;
    ...
    vm_page_grab_release(m, allocflags);
    i++;
}

This revision is now accepted and ready to land.Thu, May 8, 2:20 PM

markj added inline comments.Thu, May 8, 2:35 PM

sys/vm/vm_page.c
5199	Hrm, sorry, I misremembered. Apparently only the noobj flavour of vm_page_alloc() handles zeroing, so my comment is invalid.

alc added inline comments.Thu, May 8, 9:28 PM

sys/vm/vm_page.c
2142	This goto should no longer exist. It is a leftover from VM_ALLOC_NOOBJ that should have been eliminated when the _noobj functions were introduced. vm_page_alloc_contig*() has this right.

alc added inline comments.Thu, May 8, 9:30 PM

sys/vm/vm_page.c
3556–3575	If this function is being inlined, then I am tempted to suggest that the iterator be passed in and only reset in cases where we actually release the object lock and sleep.