[wip] ufs: add missing vgone calls to accomodate unlocked vnode with i_mode == 0

[mjg]

Summary

Still testing.

One place I don't know what to do about is the vfs_hash_insert call. Apart from that all error paths that I see got a vgone call.

[jeff]

Should we make something specific to this pattern? vcancel()? vabandon()? vabort()

[mjg]

Well in that spirit I would argue there should be a (debug) flag where the filesystem can denote that the vnode is ready for use (or ready to no longer be used, after vgone). Then the vfs layer can assert on it in a generic manner. Combining vgone + vput into something in the lines of vabort is fine with me.

[kib]

sys/ufs/ufs/ufs_vnops.c

2084 ↗

(On Diff #66861)

There we might get with i_nlink == 0. In the normal case of no suspension, the vnode was reclaimed by the vput()->inactive from vfs_syscall.c under the same lock ownership.

Same note for ufs_remove().

[mjg]

sys/ufs/ufs/ufs_vnops.c

2084 ↗

(On Diff #66861)

If this is considered a bug, it long predates my changes. Note someone can have a file descriptor for this directory, meaning v_usecount is > 0. In this case vput does not perform inactive.

Since removal was performed, the name should be removed from the namecache and consequently the only new way to get references is through the vnode hash but that's racy regardless of vput relocking or not. One variant is that such a ref got obtained while the vnode still has v_usecount > 0 and then inactive was never considered in the first place. The other variant is that this raced and transitioned usecount to 1, which aborted inactive. But the outcome is the same.

I think it's problematic that such a race exists, but as stated above it predates my work and I consider it orthogonal to the proposed fix.

[mjg]

This got sorted out with r357070 for now, but I think reliance on vput atomicity is an avoidable constraint on what can be done in that primitive and stuff like should go in for all filesystems.

[kib]

I think that this change can/should be committed regardless of other actions in this area. It is correct for the places where you put it.