From owner-freebsd-fs@FreeBSD.ORG Sun Aug 29 23:34:58 2010 Return-Path: Delivered-To: freebsd-fs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id CC1EF106566C for ; Sun, 29 Aug 2010 23:34:58 +0000 (UTC) (envelope-from rmacklem@uoguelph.ca) Received: from esa-annu.mail.uoguelph.ca (esa-annu.mail.uoguelph.ca [131.104.91.36]) by mx1.freebsd.org (Postfix) with ESMTP id 87DFA8FC19 for ; Sun, 29 Aug 2010 23:34:58 +0000 (UTC) X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: ApwEAPuLekyDaFvO/2dsb2JhbACDF54iqHiQfIEigyJzBIoJ X-IronPort-AV: E=Sophos;i="4.56,289,1280721600"; d="scan'208";a="90092542" Received: from erie.cs.uoguelph.ca (HELO zcs3.mail.uoguelph.ca) ([131.104.91.206]) by esa-annu-pri.mail.uoguelph.ca with ESMTP; 29 Aug 2010 19:34:50 -0400 Received: from zcs3.mail.uoguelph.ca (localhost.localdomain [127.0.0.1]) by zcs3.mail.uoguelph.ca (Postfix) with ESMTP id D41F1B3E95; Sun, 29 Aug 2010 19:34:50 -0400 (EDT) Date: Sun, 29 Aug 2010 19:34:50 -0400 (EDT) From: Rick Macklem To: Gleb Kurtsou Message-ID: <825702521.248882.1283124890775.JavaMail.root@erie.cs.uoguelph.ca> In-Reply-To: <20100719074050.GA2640@tops> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [24.65.230.102] X-Mailer: Zimbra 6.0.7_GA_2476.RHEL4 (ZimbraWebClient - SAF3 (Mac)/6.0.7_GA_2473.RHEL4_64) Cc: freebsd-fs@freebsd.org Subject: Re: fix for remove for NFS through nullfs X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 29 Aug 2010 23:34:58 -0000 > 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