Date: Sun, 29 Aug 2010 19:34:50 -0400 (EDT) From: Rick Macklem <rmacklem@uoguelph.ca> To: Gleb Kurtsou <gleb.kurtsou@gmail.com> Cc: freebsd-fs@freebsd.org Subject: Re: fix for remove for NFS through nullfs Message-ID: <825702521.248882.1283124890775.JavaMail.root@erie.cs.uoguelph.ca> In-Reply-To: <20100719074050.GA2640@tops>
next in thread | previous in thread | raw e-mail | index | archive | help
> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?825702521.248882.1283124890775.JavaMail.root>