From owner-svn-src-all@FreeBSD.ORG Thu Nov 17 04:01:11 2011 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B2A18106564A; Thu, 17 Nov 2011 04:01:10 +0000 (UTC) (envelope-from bright@elvis.mu.org) Received: from elvis.mu.org (elvis.mu.org [192.203.228.196]) by mx1.freebsd.org (Postfix) with ESMTP id 6CC038FC13; Thu, 17 Nov 2011 04:01:10 +0000 (UTC) Received: by elvis.mu.org (Postfix, from userid 1192) id DCDE11A3CA0; Wed, 16 Nov 2011 20:01:09 -0800 (PST) Date: Wed, 16 Nov 2011 20:01:09 -0800 From: Alfred Perlstein To: Rick Macklem Message-ID: <20111117040109.GU1455@elvis.mu.org> References: <201111160505.pAG55D5R032343@svn.freebsd.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201111160505.pAG55D5R032343@svn.freebsd.org> User-Agent: Mutt/1.4.2.3i Cc: svn-src-stable@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, svn-src-stable-7@freebsd.org Subject: Re: svn commit: r227549 - stable/7/sys/nfsclient X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 17 Nov 2011 04:01:11 -0000 Rick, I have a question, what will happen if the nfs_freesillyrename() call happens when the mount is down? Will it block the taskqueue_thread? If so, it might make more sense to make a taskqueue per mount point. If not, excuse me. :-) -Alfred * Rick Macklem [111115 21:05] wrote: > Author: rmacklem > Date: Wed Nov 16 05:05:13 2011 > New Revision: 227549 > URL: http://svn.freebsd.org/changeset/base/227549 > > Log: > MFC: r224604 > Fix a LOR in the NFS client which could cause a deadlock. > This was reported to the mailing list freebsd-net@freebsd.org > on July 21, 2011 under the subject "LOR with nfsclient sillyrename". > The LOR occurred when nfs_inactive() called vrele(sp->s_dvp) > while holding the vnode lock on the file in s_dvp. This patch > modifies the client so that it performs the vrele(sp->s_dvp) > as a separate task to avoid the LOR. This fix was discussed > with jhb@ and kib@, who both proposed variations of it. > > Modified: > stable/7/sys/nfsclient/nfs_node.c > stable/7/sys/nfsclient/nfsnode.h > Directory Properties: > stable/7/sys/ (props changed) > stable/7/sys/cddl/contrib/opensolaris/ (props changed) > stable/7/sys/contrib/dev/acpica/ (props changed) > stable/7/sys/contrib/pf/ (props changed) > > Modified: stable/7/sys/nfsclient/nfs_node.c > ============================================================================== > --- stable/7/sys/nfsclient/nfs_node.c Wed Nov 16 02:52:24 2011 (r227548) > +++ stable/7/sys/nfsclient/nfs_node.c Wed Nov 16 05:05:13 2011 (r227549) > @@ -45,6 +45,7 @@ __FBSDID("$FreeBSD$"); > #include > #include > #include > +#include > #include > > #include > @@ -59,6 +60,8 @@ __FBSDID("$FreeBSD$"); > > static uma_zone_t nfsnode_zone; > > +static void nfs_freesillyrename(void *arg, __unused int pending); > + > #define TRUE 1 > #define FALSE 0 > > @@ -191,6 +194,20 @@ nfs_nget(struct mount *mntp, nfsfh_t *fh > return (0); > } > > +/* > + * Do the vrele(sp->s_dvp) as a separate task in order to avoid a > + * deadlock because of a LOR when vrele() locks the directory vnode. > + */ > +static void > +nfs_freesillyrename(void *arg, __unused int pending) > +{ > + struct sillyrename *sp; > + > + sp = arg; > + vrele(sp->s_dvp); > + free(sp, M_NFSREQ); > +} > + > int > nfs_inactive(struct vop_inactive_args *ap) > { > @@ -213,8 +230,8 @@ nfs_inactive(struct vop_inactive_args *a > */ > (sp->s_removeit)(sp); > crfree(sp->s_cred); > - vrele(sp->s_dvp); > - FREE((caddr_t)sp, M_NFSREQ); > + TASK_INIT(&sp->s_task, 0, nfs_freesillyrename, sp); > + taskqueue_enqueue(taskqueue_thread, &sp->s_task); > } > np->n_flag &= NMODIFIED; > return (0); > > Modified: stable/7/sys/nfsclient/nfsnode.h > ============================================================================== > --- stable/7/sys/nfsclient/nfsnode.h Wed Nov 16 02:52:24 2011 (r227548) > +++ stable/7/sys/nfsclient/nfsnode.h Wed Nov 16 05:05:13 2011 (r227549) > @@ -36,6 +36,7 @@ > #ifndef _NFSCLIENT_NFSNODE_H_ > #define _NFSCLIENT_NFSNODE_H_ > > +#include > #if !defined(_NFSCLIENT_NFS_H_) && !defined(_KERNEL) > #include > #endif > @@ -45,6 +46,7 @@ > * can be removed by nfs_inactive() > */ > struct sillyrename { > + struct task s_task; > struct ucred *s_cred; > struct vnode *s_dvp; > int (*s_removeit)(struct sillyrename *sp); -- - Alfred Perlstein .- VMOA #5191, 03 vmax, 92 gs500, 85 ch250, 07 zx10 .- FreeBSD committer