This should be an atomic load.

Don't we potentially need to wake up waiters if we are the last sharer?

vm_page_grab_check() might be a better name. It is consistent with vm_page_alloc_check() and vm_page_grab_asserts sounds like "asserts" is the object, not "vm_page".

You can get rid of the extra quotes here.

For the reader or the machine?

If we are the last sharer we should execute the case below.

ok.

The naming I really don't like is _unlocked after everything. I could probably do a sweep of the tree and convert everything to _unlocked and drop it from the name. Any outside sources doing direct object manipulation will suddenly be wrong though.

If you have suggestions I'm all ears.

Why is it sufficient to hold a wiring? Without the object lock or page busied the page's <obj, pindex> identity is not stable.

I think this is actually harmless because the page in question will have a NULL object pointer and its next pointer will either be clear or point to a page without a NULL object pointer. pindex will remain unchanged. I suppose that could create a spin loop until the NULL pointer makes it to the list. So from that pov I need to be more careful here and perhaps retry less.

Another option would be to pass the expected object pointer in with the page for validation.

Once grab_next fails grab_pages_unlocked will fall back to the locked loop. So the subsequent pages will be valid. I don't think it's possible to get incorrect results this way.

We could actually handle more cases by falling back to a lockless lookup. Then the grab_next_unlocked return would be conclusive.

Although it is obvious for careful reader that the function drops the object lock, it might be surprising to not so careful one. Also we traditionally point that out.

So I think it is useful to note that the function might drop the object lock if owned.

Is this comment still relevant ? I believe it described a release store.
Right now there is nothing preventing observer from seeing updated radix with old m->object.

vm_page_grab_trybusy() is also used by non-grab functions. You could rename it to vm_page_trybusy() and group it with the other functions in this file that deal with page busying.

Why tryacquire instead of trybusy? I don't think anything should be using this function to wire the page. If not, then we should rename the function to e.g., vm_page_acquire().

The use of TAILQ_NEXT here is technically fine but breaks the locking protocol. In the absence of an "atomic" TAILQ_NEXT it would probably be good to note this in the comment.

Maybe use __predict_true here.

"Spurious errors" is kind of ambiguous. "False negatives"?

You might add a two line vm_page_grab_wire() for this, since it gets copied around a lot.

I think "next" should be called "pred" or "prev".

I found no reason to preclude wiring the page although it would be unusual.

The release store is in vm_radix_remove(). The acquire is in vm_radix_lookup_unlocked().

That was the intent of the comment but I can clarify.

This and _valid() don't want to do it via _acquire or tryacquire because they may decide not to keep the page and this simplified error handling. So they should be the only two IIRC.