fix for remove for NFS through nullfs

[Rick Macklem]

> On (18/07/2010 22:40), Rick Macklem wrote:
> > Mikolaj Golub submitted the attached patch that fixes a problem
> > w.r.t.
> > a nullfs mounted NFS mount point for remove. The problem is that,
> > without this patch, NFS does not see that a file is still open
> > (v_usecount > 1) when removed and removes it instead of silly
> > renaming
> > it. This patch increments the v_usecount of the lower level vnode
> > during the remove call, so that silly rename works. kib@ has noted
> > that this may be "racy" and result in silly rename happening when it
> > isn't required but, imho, that is less of a problem than it never
> > working. (I have tested it a bit for NFS and UFS and it seems to
> > work for those file systems under a nullfs mount.)
> >
> > Why I am posting is that I am wondering if anyone knows of a file
> > system type where this extra v_usecount on the vnode at the time of
> > remove
> > will/might cause problems?
> It seems to be easily reproducible only with stacked filesystems. But
> the problem is that v_usecount can be different from number of open
> descriptors for the vnode, and it generally is.
> 
> IMHO using v_usecount is racy for all filesystems. Grep for vref and
> VREF, it's used all over the place.
> 
> The issue was discussed some time ago already:
> http://marc.info/?l=freebsd-hackers&m=125675165319186&w=4
> 
> I think the better solution would be to add a means of getting number
> of
> opened file descriptors, but not misuse v_usecount, e.g. the patch
> looks
> a pure hack for me.
> 
> Thanks,
> Gleb.
>
I tried keeping a count of "opens" in the NFS part of the vnode, but it
doesn't work well. For example, mmap'd files do I/O after the
VOP_CLOSE() { nfs_close() } call, so you can't decrement the count in
nfs_close(). You can set bthe count back to 0 in nfs_inactive(), but
VOP_INACTIVE() isn't always called, so it only happens sometimes.

I grep'd the file systems and the only file systems other than the NFS
clients that look at vrefcnt() in their VOP_REMOVE() calls are smbfs
and nwfs and these 2 file systems only use vrefcnt() when the vnode is
a directory.

As such, if the increment of v_usecount is only done by nullfs in its
VOP_REMOVE() for v_type == VREG, it seems that it will be ok.

Discussions off-list seem to agree that the patch should go in,
so I plan on committing it, although I agree that it is a hack.

rick