6526639 Deadlock in fcp code : fp_nexus_enum_tq taskq deadlocks with devfs

2 files changed, 67 insertions, 9 deletions

diff --git a/usr/src/uts/common/fs/devfs/devfs_subr.c b/usr/src/uts/common/fs/devfs/devfs_subr.c
index 819ed9ba56..4aa95bc321 100644
--- a/usr/src/uts/common/fs/devfs/devfs_subr.c
+++ b/usr/src/uts/common/fs/devfs/devfs_subr.c
@@ -19,7 +19,7 @@
  * CDDL HEADER END
  */
 /*
- * Copyright 2006 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2007 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 
@@ -64,7 +64,13 @@ int devfs_debug = 0x0;
 
 const char	dvnm[] = "devfs";
 kmem_cache_t	*dv_node_cache;	/* dv_node cache */
+
+/*
+ * The devfs_clean_key is taken during a devfs_clean operation: it is used to
+ * prevent unnecessary code execution and for detection of potential deadlocks.
+ */
 uint_t		devfs_clean_key;
+
 struct dv_node *dvroot;
 
 /* prototype memory vattrs */
@@ -893,7 +899,7 @@ dv_find(struct dv_node *ddv, char *nm, struct vnode **vpp, struct pathname *pnp,
 {
 	extern int isminiroot;	/* see modctl.c */
 
-	int rv = 0, was_busy = 0, nmlen;
+	int rv = 0, was_busy = 0, nmlen, write_held = 0;
 	struct vnode *vp;
 	struct dv_node *dv, *dup;
 	dev_info_t *pdevi, *devi = NULL;
@@ -947,7 +953,16 @@ start:
 	if ((dv = dv_findbyname(ddv, nm)) != NULL) {
 founddv:
 		ASSERT(RW_LOCK_HELD(&ddv->dv_contents));
-		rw_enter(&dv->dv_contents, RW_READER);
+
+		if (!rw_tryenter(&dv->dv_contents, RW_READER)) {
+			if (tsd_get(devfs_clean_key)) {
+				VN_RELE(DVTOV(dv));
+				rw_exit(&ddv->dv_contents);
+				return (EBUSY);
+			}
+			rw_enter(&dv->dv_contents, RW_READER);
+		}
+
 		vp = DVTOV(dv);
 		if ((dv->dv_attrvp != NULLVP) ||
 		    (vp->v_type != VDIR && dv->dv_attr != NULL)) {
@@ -961,8 +976,30 @@ founddv:
 
 		/*
 		 * No attribute vp, try and build one.
+		 *
+		 * dv_shadow_node() can briefly drop &dv->dv_contents lock
+		 * if it is unable to upgrade it to a write lock. If the
+		 * current thread has come in through the bottom-up device
+		 * configuration devfs_clean() path, we may deadlock against
+		 * a thread performing top-down device configuration if it
+		 * grabs the contents lock. To avoid this, when we are on the
+		 * devfs_clean() path we attempt to upgrade the dv_contents
+		 * lock before we call dv_shadow_node().
 		 */
-		dv_shadow_node(DVTOV(ddv), nm, vp, pnp, rdir, cred, 0);
+		if (tsd_get(devfs_clean_key)) {
+			if (!rw_tryupgrade(&dv->dv_contents)) {
+				VN_RELE(DVTOV(dv));
+				rw_exit(&dv->dv_contents);
+				rw_exit(&ddv->dv_contents);
+				return (EBUSY);
+			}
+
+			write_held = DV_SHADOW_WRITE_HELD;
+		}
+
+		dv_shadow_node(DVTOV(ddv), nm, vp, pnp, rdir, cred,
+		    write_held);
+
 		rw_exit(&dv->dv_contents);
 		rw_exit(&ddv->dv_contents);
 		goto found;
@@ -1242,10 +1279,24 @@ dv_cleandir(struct dv_node *ddv, char *devnm, uint_t flags)
 	struct vnode *vp;
 	int busy = 0;
 
+	/*
+	 * We should always be holding the tsd_clean_key here: dv_cleandir()
+	 * will be called as a result of a devfs_clean request and the
+	 * tsd_clean_key will be set in either in devfs_clean() itself or in
+	 * devfs_clean_vhci().
+	 *
+	 * Since we are on the devfs_clean path, we return EBUSY if we cannot
+	 * get the contents lock: if we blocked here we might deadlock against
+	 * a thread performing top-down device configuration.
+	 */
+	ASSERT(tsd_get(devfs_clean_key));
+
 	dcmn_err3(("dv_cleandir: %s\n", ddv->dv_name));
 
-	if (!(flags & DV_CLEANDIR_LCK))
-		rw_enter(&ddv->dv_contents, RW_WRITER);
+	if (!(flags & DV_CLEANDIR_LCK) &&
+	    !rw_tryenter(&ddv->dv_contents, RW_WRITER))
+		return (EBUSY);
+
 	for (pprev = &ddv->dv_dot, dv = *pprev; dv;
 	    pprev = npprev, dv = *pprev) {
 		npprev = &dv->dv_next;
diff --git a/usr/src/uts/common/fs/devfs/devfs_vfsops.c b/usr/src/uts/common/fs/devfs/devfs_vfsops.c
index 25f8f061f3..ddd006ab70 100644
--- a/usr/src/uts/common/fs/devfs/devfs_vfsops.c
+++ b/usr/src/uts/common/fs/devfs/devfs_vfsops.c
@@ -358,11 +358,11 @@ devfs_clean_vhci(dev_info_t *dip, void *args)
 
 	(void) tsd_set(devfs_clean_key, (void *)1);
 	dvp = devfs_dip_to_dvnode(dip);
-	(void) tsd_set(devfs_clean_key, NULL);
 	if (dvp) {
 		(void) dv_cleandir(dvp, NULL, flags);
 		VN_RELE(DVTOV(dvp));
 	}
+	(void) tsd_set(devfs_clean_key, NULL);
 	return (DDI_WALK_CONTINUE);
 }
 
@@ -380,6 +380,11 @@ devfs_clean_vhci(dev_info_t *dip, void *args)
  * referenced dv_nodes. To enforce this, devfs_clean() always
  * returns success i.e. 0.
  *
+ * devfs_clean() may return before removing all possible nodes if
+ * we cannot acquire locks in areas of the code where potential for
+ * deadlock exists (see comments in dv_find() and dv_cleandir() for
+ * examples of this).
+ *
  * devfs caches unreferenced dv_node to speed by the performance
  * of ls, find, etc. devfs_clean() is invoked to cleanup cached
  * dv_nodes to reclaim memory as well as to facilitate device
@@ -404,11 +409,13 @@ devfs_clean(dev_info_t *dip, char *devnm, uint_t flags)
 	/* avoid recursion back into the device tree */
 	(void) tsd_set(devfs_clean_key, (void *)1);
 	dvp = devfs_dip_to_dvnode(dip);
-	(void) tsd_set(devfs_clean_key, NULL);
-	if (dvp == NULL)
+	if (dvp == NULL) {
+		(void) tsd_set(devfs_clean_key, NULL);
 		return (0);
+	}
 
 	(void) dv_cleandir(dvp, devnm, flags);
+	(void) tsd_set(devfs_clean_key, NULL);
 	VN_RELE(DVTOV(dvp));
 
 	/*