Skip site navigation (1)Skip section navigation (2) 
Date:      Wed, 16 Nov 2011 05:05:13 +0000 (UTC)
From:      Rick Macklem <rmacklem@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-7@freebsd.org
Subject:   svn commit: r227549 - stable/7/sys/nfsclient
Message-ID:  <201111160505.pAG55D5R032343@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
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 <sys/proc.h>
 #include <sys/socket.h>
 #include <sys/sysctl.h>
+#include <sys/taskqueue.h>
 #include <sys/vnode.h>
 
 #include <vm/uma.h>
@@ -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 <sys/_task.h>
 #if !defined(_NFSCLIENT_NFS_H_) && !defined(_KERNEL)
 #include <nfs/nfs.h>
 #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);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201111160505.pAG55D5R032343>