From owner-svn-src-stable-7@FreeBSD.ORG Wed Nov 16 05:05:14 2011 Return-Path: Delivered-To: svn-src-stable-7@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 018451065675; Wed, 16 Nov 2011 05:05:14 +0000 (UTC) (envelope-from rmacklem@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id CA5A18FC17; Wed, 16 Nov 2011 05:05:13 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.4/8.14.4) with ESMTP id pAG55Dhg032346; Wed, 16 Nov 2011 05:05:13 GMT (envelope-from rmacklem@svn.freebsd.org) Received: (from rmacklem@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id pAG55D5R032343; Wed, 16 Nov 2011 05:05:13 GMT (envelope-from rmacklem@svn.freebsd.org) Message-Id: <201111160505.pAG55D5R032343@svn.freebsd.org> From: Rick Macklem Date: Wed, 16 Nov 2011 05:05:13 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-7@freebsd.org X-SVN-Group: stable-7 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r227549 - stable/7/sys/nfsclient X-BeenThere: svn-src-stable-7@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for only the 7-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Nov 2011 05:05:14 -0000 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);