From owner-svn-src-stable-8@FreeBSD.ORG Wed Aug 17 20:55:57 2011 Return-Path: Delivered-To: svn-src-stable-8@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 2E828106566C; Wed, 17 Aug 2011 20:55:57 +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 15C708FC14; Wed, 17 Aug 2011 20:55:57 +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 p7HKtu4R022697; Wed, 17 Aug 2011 20:55:56 GMT (envelope-from rmacklem@svn.freebsd.org) Received: (from rmacklem@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id p7HKtuSC022694; Wed, 17 Aug 2011 20:55:56 GMT (envelope-from rmacklem@svn.freebsd.org) Message-Id: <201108172055.p7HKtuSC022694@svn.freebsd.org> From: Rick Macklem Date: Wed, 17 Aug 2011 20:55:56 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org X-SVN-Group: stable-8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r224951 - stable/8/sys/fs/nfsclient X-BeenThere: svn-src-stable-8@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for only the 8-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 17 Aug 2011 20:55:57 -0000 Author: rmacklem Date: Wed Aug 17 20:55:56 2011 New Revision: 224951 URL: http://svn.freebsd.org/changeset/base/224951 Log: MFC: r224606 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/8/sys/fs/nfsclient/nfs_clnode.c stable/8/sys/fs/nfsclient/nfsnode.h Directory Properties: stable/8/sys/ (props changed) stable/8/sys/amd64/include/xen/ (props changed) stable/8/sys/cddl/contrib/opensolaris/ (props changed) stable/8/sys/contrib/dev/acpica/ (props changed) stable/8/sys/contrib/pf/ (props changed) Modified: stable/8/sys/fs/nfsclient/nfs_clnode.c ============================================================================== --- stable/8/sys/fs/nfsclient/nfs_clnode.c Wed Aug 17 20:43:59 2011 (r224950) +++ stable/8/sys/fs/nfsclient/nfs_clnode.c Wed Aug 17 20:55:56 2011 (r224951) @@ -45,6 +45,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include @@ -62,6 +63,8 @@ MALLOC_DECLARE(M_NEWNFSREQ); uma_zone_t newnfsnode_zone; +static void nfs_freesillyrename(void *arg, __unused int pending); + void ncl_nhinit(void) { @@ -183,6 +186,20 @@ ncl_nget(struct mount *mntp, u_int8_t *f 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_NEWNFSREQ); +} + int ncl_inactive(struct vop_inactive_args *ap) { @@ -217,8 +234,8 @@ ncl_inactive(struct vop_inactive_args *a */ ncl_removeit(sp, vp); crfree(sp->s_cred); - vrele(sp->s_dvp); - FREE((caddr_t)sp, M_NEWNFSREQ); + TASK_INIT(&sp->s_task, 0, nfs_freesillyrename, sp); + taskqueue_enqueue(taskqueue_thread, &sp->s_task); mtx_lock(&np->n_mtx); } np->n_flag &= NMODIFIED; Modified: stable/8/sys/fs/nfsclient/nfsnode.h ============================================================================== --- stable/8/sys/fs/nfsclient/nfsnode.h Wed Aug 17 20:43:59 2011 (r224950) +++ stable/8/sys/fs/nfsclient/nfsnode.h Wed Aug 17 20:55:56 2011 (r224951) @@ -35,11 +35,14 @@ #ifndef _NFSCLIENT_NFSNODE_H_ #define _NFSCLIENT_NFSNODE_H_ +#include + /* * Silly rename structure that hangs off the nfsnode until the name * can be removed by nfs_inactive() */ struct sillyrename { + struct task s_task; struct ucred *s_cred; struct vnode *s_dvp; long s_namlen;