From owner-freebsd-hackers Thu Jan 10 19:23:43 2002 Delivered-To: freebsd-hackers@freebsd.org Received: from apollo.backplane.com (apollo.backplane.com [216.240.41.2]) by hub.freebsd.org (Postfix) with ESMTP id 0532C37B400 for ; Thu, 10 Jan 2002 19:23:21 -0800 (PST) Received: (from dillon@localhost) by apollo.backplane.com (8.11.6/8.9.1) id g0B3NDt38028; Thu, 10 Jan 2002 19:23:13 -0800 (PST) (envelope-from dillon) Date: Thu, 10 Jan 2002 19:23:13 -0800 (PST) From: Matthew Dillon Message-Id: <200201110323.g0B3NDt38028@apollo.backplane.com> To: Alfred Perlstein , "Alan L. Cox" Cc: FreeBSD-hackers@FreeBSD.ORG Subject: Need review of NFS patch set for server .. missing/wrong vput() issues Sender: owner-freebsd-hackers@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG Heya Alfred, Alan, hackers. Ok, I've been tracking down a bug with A russian news admin and I believe I may have found it (he's testing it), plus a few other bugs. I would like a review of this (for -stable, but applies to -current too): Patch section 1 Here we were previously vput()ing nd.ni_vp only if error == 0. If error is returned non-zero from namei() this would normally be correct. However, we force error on a number of occassions after namei() succeeds, in which case nd.ni_vp may be non-NULL and we must release it. This fixes it so nd.ni_vp is vput()'d if it is non-NULL whether an error is specified at this point or not. (I believe this may have been Alexey's 'NFS hangs in inode state' problem, which occurs if you are running innd over an NFS filesystem) Patch section's 2 & 3 Here namei() is called only with LOCKPARENT, which means that the leaf is not locked. So when releasing the vnodes we should not have the if (vp == dvp) test, we should just vput() the dvp and vrele the vp. (Normally when you LOCKPARENT|LOCKLEAF and vp == dvp, only one vput should be performed, thus the test. But we aren't locking the leaf here). GURUS: Am I reading this correctly or does LOCKPARENT without a LOCKLEAF do something weird when dvp == vp? Index: nfs/nfs_serv.c =================================================================== RCS file: /home/ncvs/src/sys/nfs/Attic/nfs_serv.c,v retrieving revision 1.93.2.2 diff -u -r1.93.2.2 nfs_serv.c --- nfs/nfs_serv.c 28 Dec 2001 19:57:40 -0000 1.93.2.2 +++ nfs/nfs_serv.c 11 Jan 2002 03:15:51 -0000 @@ -2015,6 +2015,8 @@ error = VFS_VPTOFH(vp, &fhp->fh_fid); if (!error) error = VOP_GETATTR(vp, vap, cred, procp); + } + if (vp) { vput(vp); vp = NULL; nd.ni_vp = NULL; @@ -2467,14 +2469,15 @@ vrele(dirp); if (vp) vrele(vp); - if (nd.ni_dvp) { - if (nd.ni_dvp == nd.ni_vp) - vrele(nd.ni_dvp); - else - vput(nd.ni_dvp); - } + + /* + * since leaf is not locked, we can vput the parent even if + * vp == dvp. + */ if (nd.ni_vp) vrele(nd.ni_vp); + if (nd.ni_dvp) + vput(nd.ni_dvp); return(error); } @@ -2636,10 +2639,11 @@ nfsmout: NDFREE(&nd, NDF_ONLY_PNBUF); if (nd.ni_dvp) { - if (nd.ni_dvp == nd.ni_vp) - vrele(nd.ni_dvp); - else - vput(nd.ni_dvp); + /* + * since leaf is not locked, we can vput the parent even if + * vp == dvp. + */ + vput(nd.ni_dvp); } if (nd.ni_vp) vrele(nd.ni_vp); To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-hackers" in the body of the message