This generally looks good (and much better than D48511) except for what I stated in an inline comment (don't mess with arg2, I insist on that).

In D48825#1114691, @kib wrote:

In D48825#1114667, @olce wrote:

No, because if the lower vnode has actually been unlinked, then the lower layer's VOP_REMOVE() (e.g., ufs_remove()) will return 0,

This is not true, and is not guaranteed.

In D48825#1114666, @kib wrote:

Problem is when upper vnode is reclaimed, but lower is not, although lower was temp unlocked for something. Then despite unlinked, lower vnode is still referenced by other upper vnodes, which prevents inactivation from truncating and clearing the inode.

In D48825#1114649, @kib wrote:

Look at the ERELOOKUP, specifically of the softdep_prelink().

In D48633#1114603, @jhibbits wrote:

I don't think it's possible to have domain 0 unpopulated on powerpc. I do have a patch that implements the NUMAing of mmu_radix_page_array_startup(), but it didn't improve performance any measurable degree when I tested, so never followed through with pushing it.

In D48825#1114446, @kib wrote:

No, with you proposal the call would only propagate to sibling uppers if it succeeded, which is not true when the lower vnode is unlocked.

Please see the new inline comments. Thanks!

In D48634#1109782, @markj wrote:

I don't really see why. The current interface is consistent with other NUMA-aware allocators, e.g., kmem_malloc_domain(), vm_page_alloc_contig_domain(), vm_phys_alloc_pages().

In D48633#1109670, @markj wrote:

So if a system has two CPU sockets and only one has memory installed, we will panic? That is not right, we have to support such configurations.

In D48825#1114388, @kib wrote:

In D48825#1114380, @olce wrote:

You're right. But does it matter?

Yes it is. Second sibling nullfs mount still references the lower vnode, so use count on it is non-zero. For instance, on UFS, this prevents vop_inactive() from truncating the file, clearing the inode, and reclaiming the vnode. Basically it makes the issue fixed in the review, to return, but only in racy way.

Seems Phab did not save my inline comments which consisted of only line removals and no written text. Or I messed up something, but don't know how. The diffs should be complete now.

Whoops... The first inline comment was wrong, I proposed another version.

I think this version increases slightly the already too big amount of spaghetti code in this function.

In D48825#1114098, @kib wrote:

Dropping the lock is the issue. For instance, UFS may successfully unlink but drop the lock, which allows the upper vnode to be reclaimed, and then other upper filesystems are not notified, it the call is moved after the VOP.

In D48825#1113760, @kib wrote:

Well, optimization for the failing cases should not be a necessity. That said, I am not sure that there is no complications with the vnode ref counts (I might be too cautious there, but it is the reason to keep this separate) with the move of upper notification post vop. You can do it after my change is landed.

The approach looks generally OK to me, but please take the opportunity to move notifying the upper layers into the "post" hooks instead of the "pre" ones, under the condition that rc is effectively 0, as it seems wasted work to destroy the upper vnodes if the lower one isn't (as they are probably going to need to be reconstructed later).

I think it makes sense to revise this change to be conditional on VM_PHYSSEG_DENSE (and on the condition: vm_domains == 1 or PMAP_HAS_PAGE_ARRAY absent) in order to avoid disabling the anti-fragmentation mechanism on VM_PHYSSEG_SPARSE.