vm_page: Add a new page allocator interface for anonymous pages

[markj]

Summary

The diff adds vm_page_alloc_noobj() and vm_page_alloc_noobj_domain().
These mostly correspond to vm_page_alloc() and vm_page_alloc_domain()
when no VM object is specified, with the exception that they handle
VM_ALLOC_ZERO by zeroing the page, rather than by preserving PG_ZERO.

This simplifies callers and makes it harder to accidentally omit a
pmap_zero_page() call. It also permits simplification of the
vm_page_alloc_domain() implementation.

Since the new interface is similar to vm_page_alloc_freelist(),
implement both of them using a common backend allocator function.
No functional change intended.

[kib]

sys/vm/vm_page.c

2398

I would allow VM_ALLOC_NOBUSY

2401

!= 0

2439

!= 0

2545

Why moving this function around?

[kib]

sys/vm/vm_page.c

2442

I wonder if it would be useful to assign pindex some value? E.g. 0 or -1, or even some pattern like deadc0de, so that consumers that need pindex would note it.

[kib]

sys/vm/vm_page.h

545

Do you plan to remove the flag?

[markj]

sys/vm/vm_page.c

2442

I like that idea. Maybe it should be INVARIANTS-only but I don't think there is any harm in having it be unconditional.

2545

I just wanted to group all of the allocator entry points together.

sys/vm/vm_page.h

545

Eventually, yes. For now it is kept since vm_page_alloc(NULL, 0, VM_ALLOC_NOOBJ) is still permitted. I would like to MFC as much as possible to stable/13.

[markj]

Style, initialize pindex, permit _NOBUSY.

[kib]

sys/vm/vm_page.c

2545

Perhaps commit the move separately?

[alc]

sys/vm/vm_page.c

2401–2406

As an aside, I am going to argue that these functions should be jettisoned. They are only used by MIPS, and those uses would be handled efficiently, i.e., without excessive iteration, by vm_page_alloc_noobj_contig().

2404

See my below comment about the _freelist functions.

2547

You can drop the blank line here.

2548–2560

If I am allowed to be truly pedantic for a moment, I would suggest reordering these asserts to match the order of the fields in the struct. Then, a reader can more easily figure out which fields are being checked and which are not.

[imp]

sys/vm/vm_page.c

2401–2406

One could just mark them #ifdef mips. then when the coming mips retirement happens, they will be removed.

[alc]

sys/vm/vm_page.c

2401–2406

How soon will that be? I brought up the elimination of the _freelist functions here because eliminating them would simplify the implementation of the helper function, _vm_page_alloc_noobj_domain(), that is introduced in this change.

[markj]

sys/vm/vm_page.c

2401–2406

It looks like Warner started the discussion, so I guess it would happen sometime before 14.x. We could in the meantime implement a MIPS-private vm_page_alloc_freelist() like this (together with a per-domain variant)?

vm_page_t
pmap_alloc_direct_page(vm_pindex_t index, int req)
{
    vm_page_t m;

    req |= VM_ALLOC_WIRE | VM_ALLOC_ZERO;
#if VM_FREELIST_DIRECT == VM_FREELIST_DEFAULT
    m = vm_page_alloc_noobj(req);
#elif VM_FREELIST_DIRECT == VM_FREELIST_LOWMEM
    m = vm_page_alloc_noobj_contig(req, 1, 0, VM_LOWMEM_BOUNDARY, PAGE_SIZE, 0, VM_MEMATTR_DEFAULT);
#else
#error unhandled
#endif
    m->pindex = index;
    return (m);
}

Alternately, just eliminate the _vm_page_alloc_noobj_domain() helper once MIPS is removed. Or, keep the vm_page_alloc_freelist() implementation separate from the vm_page_alloc_noobj() implementation so that the eventual removal of vm_page_alloc_freelist() needn't touch anything else.

2548–2560

I split the vm_page_alloc_check() move into a separate commit already. This diff shows the modifications you suggested, but I will merge them with the separate commit before pushing.

[markj]

Tidy up vm_page_alloc_check().

[imp]

Mips will be in the tree through end of year. If all the stars align, maybe it will be gone by US thanksgiving.
There's no push back so far in the discussions.

[markj]

Is there any objection to committing this and the rest of the series (the reviews listed under the "stack" tab)? Especially with regards to naming since many consumers are being updated.

[kib]

In D31985#733674, @markj wrote:

Is there any objection to committing this and the rest of the series (the reviews listed under the "stack" tab)? Especially with regards to naming since many consumers are being updated.

Not from me; I believe I clicked "accept" at least once on each review.

[alc]

In D31985#733674, @markj wrote:

Is there any objection to committing this and the rest of the series (the reviews listed under the "stack" tab)? Especially with regards to naming since many consumers are being updated.

I don't think so. I have some free time on my hands today, so I will take one last look at the series.

[alc]

sys/vm/vm_page.h

543

[markj]

Update the VM_ALLOC_ZERO comment.

[alc]

Just to be clear, I believe that I've looked at all of the patches, and unless there is a recent change that you want me to look at, I'm done.

[markj]

In D31985#734942, @alc wrote:

Just to be clear, I believe that I've looked at all of the patches, and unless there is a recent change that you want me to look at, I'm done.

Thank you very much for the reviews!