void

Does using the assert()'s in test case make sense? Getting descriptive failure reason instead of a core seems better to me.

Not needed anymore?

And here we could check for != NULL and going to line 578, reducing indentation even further which helps readability.

This else isn't really needed as we did 'goto' in other branch, removing it would help reduce indentation.

Takes the lock without being named as such, and keeps it even on error? I'm not a fan! Please just take p_lock before its single call site, then make it assert its held when we call it.

inverting this check would reduce indentation of the rest, and make the common path more obvious

why did we lose the comment about dropping p_lock here? It's unusual behaviour (and I'm not clear why we even do it)

If we're going to document this we should clarify that this is clamped to a minimum of NCPUS * 4 ?

NIT: Why the braces for a single-line return (NULL);?

Same nit here.

NOTE TO SELF:  There are a LOT of holes in "struct proc" that should probably cause a rearrangement.

I suspect the reason is to avoid holding p->p_lock while doing a KM_SLEEP alloc. However, that can be worked around fairly easily.

I can rearrange things here that I think make it easier to follow.

I think the idea is to avoid holding p->p_lock during a sleeping alloc.

Done.

The two nits aren't enough to block this, and I like the post-John rewhack as well.

nit, "have been"

nit, could reformat?

Ship It!

Ship It!