What to do about VOP_INACTIVE?

[Rick Macklem]

Conrad Meyer wrote:
>The nominal return type of the VOP_INACTIVE vnode method is 'int', but
>in practice any error returned is silently discarded.
>
>The only caller is vinactive(), which is also a void routine.
>vinactive ignores the return value of VOP_INACTIVE().  (vinactive
>tends to be called by other void routines, like vput(), so propagating
>an error up the stack is non-trivial.)
>
>In practice, most filesystems in the kernel unconditionally return
>zero for INACTIVE.  The exceptions are: msdosfs, ext2fs, nandfs, and
>(notably) ufs.
>
>The problem (as I see it) is that the return type makes it appear that
>INACTIVE is allowed to fail, but it is not.  One important
>ramification of this is that interruptible sleeps in INACTIVE are
>basically not permitted.
>
>This seems problematic because INACTIVE is invoked as part of
>close(2), and we can potentially block that user process indefinitely
>when the kernel filesystem is stalled on a network resource, or
>something like a FUSE userspace filesystem (which can also access
>network resources).
>
>Can we live with the current behavior (INACTIVE cannot fail)?  In that
>case, I think we should change its return type to void to match.
>
>Thoughts?
Well Kostik is the expert, but my understanding is that a file system cannot
assume that VOP_INACTIVE() will actually be called for a vnode. As such, the
file system needs to do anything it does in its VOP_INACTIVE() in its VOP_RECLAIM()
as well, if it must be done before the vnode is reused.

As such, a failed VOP_INACTIVE() doesn't seem to be meaningful, since it might
not get called at all?

Having said that, making sure file system implementors know the above seems
more important than whether it returns int vs void.
(ie. Returning 0 seems harmless to me and may be useful in the future. Since
 changing the VFS/VOP interface can only be done for major releases and requires
 all file systems to be changed, I leave it, but this is not a strong opinion.)

rick