Resolve the devfs deadlock caused by LOR between devfs_mount->dm_lock and

vnode lock in devfs_allocv. Do this by temporary dropping dm_lock around
vnode locking.

For safe operation, add hold counters for both devfs_mount and devfs_dirent,
and DE_DOOMED flag for devfs_dirent. The facilities allow to continue after
dropping of the dm_lock, by making sure that referenced memory does not
disappear.

Reviewed by:	tegge
Tested by:	kris
Approved by:	kan (mentor)
PR:		kern/102335

sys/fs/devfs/devfs.h

@@ -129,6 +129,8 @@ struct devfs_dirent {
 #define	DE_WHITEOUT	0x1
 #define	DE_DOT		0x2
 #define	DE_DOTDOT	0x4
+#define DE_DOOMED	0x8
+	int			de_holdcnt;
 	struct dirent 		*de_dirent;
 	TAILQ_ENTRY(devfs_dirent) de_list;
 	TAILQ_HEAD(, devfs_dirent) de_dlist;
@@ -150,6 +152,7 @@ struct devfs_mount {
 	struct mount		*dm_mount;
 	struct devfs_dirent	*dm_rootdir;
 	unsigned		dm_generation;
+	int			dm_holdcnt;
 	struct sx		dm_lock;
 	devfs_rsnum		dm_ruleset;
 };
@@ -160,13 +163,21 @@ extern unsigned devfs_rule_depth;
 
 #define VFSTODEVFS(mp)	((struct devfs_mount *)((mp)->mnt_data))
 
+#define DEVFS_DE_HOLD(de)	((de)->de_holdcnt++)
+#define DEVFS_DE_DROP(de)	(--(de)->de_holdcnt == 0)
+
+#define DEVFS_DMP_HOLD(dmp)	((dmp)->dm_holdcnt++)
+#define DEVFS_DMP_DROP(dmp)	(--(dmp)->dm_holdcnt == 0)
+
 void devfs_rules_apply(struct devfs_mount *dm, struct devfs_dirent *de);
 void devfs_rules_cleanup (struct devfs_mount *dm);
 int devfs_rules_ioctl(struct devfs_mount *dm, u_long cmd, caddr_t data, struct thread *td);
 int devfs_allocv (struct devfs_dirent *de, struct mount *mp, struct vnode **vpp, struct thread *td);
 void devfs_delete(struct devfs_mount *dm, struct devfs_dirent *de);
+void devfs_dirent_free(struct devfs_dirent *de);
 void devfs_populate (struct devfs_mount *dm);
 void devfs_cleanup (struct devfs_mount *dm);
+void devfs_unmount_final(struct devfs_mount *mp);
 struct devfs_dirent *devfs_newdirent (char *name, int namelen);
 struct devfs_dirent *devfs_vmkdir (struct devfs_mount *, char *name, int namelen, struct devfs_dirent *dotdot, u_int inode);
 struct devfs_dirent *devfs_find (struct devfs_dirent *dd, const char *name, int namelen);

sys/fs/devfs/devfs_devs.c

@@ -179,6 +179,7 @@ devfs_newdirent(char *name, int namelen)
 	vfs_timestamp(&de->de_ctime);
 	de->de_mtime = de->de_atime = de->de_ctime;
 	de->de_links = 1;
+	de->de_holdcnt = 1;
 #ifdef MAC
 	mac_init_devfsdirent(de);
 #endif
@@ -229,10 +230,19 @@ devfs_vmkdir(struct devfs_mount *dmp, char *name, int namelen, struct devfs_dire
 	return (dd);
 }
 
+void
+devfs_dirent_free(struct devfs_dirent *de)
+{
+	free(de, M_DEVFS3);
+}
+
 void
 devfs_delete(struct devfs_mount *dm, struct devfs_dirent *de)
 {
 
+	KASSERT((de->de_flags & DE_DOOMED) == 0,
+		("devfs_delete doomed dirent"));
+	de->de_flags |= DE_DOOMED;
 	if (de->de_symlink) {
 		free(de->de_symlink, M_DEVFS);
 		de->de_symlink = NULL;
@@ -251,7 +261,8 @@ devfs_delete(struct devfs_mount *dm, struct devfs_dirent *de)
 		free_unr(devfs_inos, de->de_inode);
 		de->de_inode = 0;
 	}
-	free(de, M_DEVFS3);
+	if (DEVFS_DE_DROP(de))
+		devfs_dirent_free(de);
 }
 
 /*

sys/fs/devfs/devfs_vfsops.c

@@ -77,6 +77,7 @@ devfs_mount(struct mount *mp, struct thread *td)
 	fmp = malloc(sizeof *fmp, M_DEVFS, M_WAITOK | M_ZERO);
 	fmp->dm_idx = alloc_unr(devfs_unr);
 	sx_init(&fmp->dm_lock, "devfsmount");
+	fmp->dm_holdcnt = 1;
 
 	mp->mnt_flag |= MNT_LOCAL;
 	mp->mnt_kern_flag |= MNTK_MPSAFE;
@@ -104,14 +105,25 @@ devfs_mount(struct mount *mp, struct thread *td)
 	return (0);
 }
 
+void
+devfs_unmount_final(struct devfs_mount *fmp)
+{
+	sx_destroy(&fmp->dm_lock);
+	free(fmp, M_DEVFS);
+}
+
 static int
 devfs_unmount(struct mount *mp, int mntflags, struct thread *td)
 {
 	int error;
 	int flags = 0;
 	struct devfs_mount *fmp;
+	int hold;
+	u_int idx;
 
 	fmp = VFSTODEVFS(mp);
+	KASSERT(fmp->dm_mount != NULL,
+		("devfs_unmount unmounted devfs_mount"));
 	/* There is 1 extra root vnode reference from devfs_mount(). */
 	error = vflush(mp, 1, flags, td);
 	if (error)
@@ -119,11 +131,14 @@ devfs_unmount(struct mount *mp, int mntflags, struct thread *td)
 	sx_xlock(&fmp->dm_lock);
 	devfs_cleanup(fmp);
 	devfs_rules_cleanup(fmp);
-	sx_xunlock(&fmp->dm_lock);
+	fmp->dm_mount = NULL;
+	hold = --fmp->dm_holdcnt;
 	mp->mnt_data = NULL;
-	sx_destroy(&fmp->dm_lock);
+	idx = fmp->dm_idx;
-	free_unr(devfs_unr, fmp->dm_idx);
+	sx_xunlock(&fmp->dm_lock);
-	free(fmp, M_DEVFS);
+	free_unr(devfs_unr, idx);
+	if (hold == 0)
+		devfs_unmount_final(fmp);
 	return 0;
 }
 
@@ -137,6 +152,7 @@ devfs_root(struct mount *mp, int flags, struct vnode **vpp, struct thread *td)
 	struct devfs_mount *dmp;
 
 	dmp = VFSTODEVFS(mp);
+	sx_xlock(&dmp->dm_lock);
 	error = devfs_allocv(dmp->dm_rootdir, mp, &vp, td);
 	if (error)
 		return (error);

sys/fs/devfs/devfs_vnops.c

@@ -125,36 +125,84 @@ devfs_fqpn(char *buf, struct vnode *dvp, struct componentname *cnp)
 	return (buf + i);
 }
 
+static int
+devfs_allocv_drop_refs(int drop_dm_lock, struct devfs_mount *dmp,
+	struct devfs_dirent *de)
+{
+	int not_found;
+
+	not_found = 0;
+	if (de->de_flags & DE_DOOMED)
+		not_found = 1;
+	if (DEVFS_DE_DROP(de)) {
+		KASSERT(not_found == 1, ("DEVFS de dropped but not doomed"));
+		devfs_dirent_free(de);
+	}
+	if (DEVFS_DMP_DROP(dmp)) {
+		KASSERT(not_found == 1,
+			("DEVFS mount struct freed before dirent"));
+		not_found = 2;
+		sx_xunlock(&dmp->dm_lock);
+		devfs_unmount_final(dmp);
+	}
+	if (not_found == 1 || drop_dm_lock)
+		sx_unlock(&dmp->dm_lock);
+	return (not_found);
+}
+
+/*
+ * devfs_allocv shall be entered with dmp->dm_lock held, and it drops
+ * it on return.
+ */
 int
 devfs_allocv(struct devfs_dirent *de, struct mount *mp, struct vnode **vpp, struct thread *td)
 {
 	int error;
 	struct vnode *vp;
 	struct cdev *dev;
+	struct devfs_mount *dmp;
 
 	KASSERT(td == curthread, ("devfs_allocv: td != curthread"));
-loop:
+	dmp = VFSTODEVFS(mp);
-
+	if (de->de_flags & DE_DOOMED) {
+		sx_xunlock(&dmp->dm_lock);
+		return (ENOENT);
+	}
+ loop:
+	DEVFS_DE_HOLD(de);
+	DEVFS_DMP_HOLD(dmp);
 	mtx_lock(&devfs_de_interlock);
 	vp = de->de_vnode;
 	if (vp != NULL) {
 		VI_LOCK(vp);
 		mtx_unlock(&devfs_de_interlock);
-		if (vget(vp, LK_EXCLUSIVE | LK_INTERLOCK, td))
+		sx_xunlock(&dmp->dm_lock);
+		error = vget(vp, LK_EXCLUSIVE | LK_INTERLOCK, td);
+		sx_xlock(&dmp->dm_lock);
+		if (devfs_allocv_drop_refs(0, dmp, de)) {
+			if (error == 0)
+				vput(vp);
+			return (ENOENT);
+		}
+		else if (error)
 			goto loop;
+		sx_xunlock(&dmp->dm_lock);
 		*vpp = vp;
 		return (0);
 	}
 	mtx_unlock(&devfs_de_interlock);
 	if (de->de_dirent->d_type == DT_CHR) {
-		if (!(de->de_cdp->cdp_flags & CDP_ACTIVE))
+		if (!(de->de_cdp->cdp_flags & CDP_ACTIVE)) {
+			devfs_allocv_drop_refs(1, dmp, de);
 			return (ENOENT);
+		}
 		dev = &de->de_cdp->cdp_c;
 	} else {
 		dev = NULL;
 	}
 	error = getnewvnode("devfs", mp, &devfs_vnodeops, &vp);
 	if (error != 0) {
+		devfs_allocv_drop_refs(1, dmp, de);
 		printf("devfs_allocv: failed to allocate new vnode\n");
 		return (error);
 	}
@@ -182,10 +230,17 @@ loop:
 	vp->v_data = de;
 	de->de_vnode = vp;
 	mtx_unlock(&devfs_de_interlock);
+	sx_xunlock(&dmp->dm_lock);
 	vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, td);
+	sx_xlock(&dmp->dm_lock);
+	if (devfs_allocv_drop_refs(0, dmp, de)) {
+		vput(vp);
+		return (ENOENT);
+	}
 #ifdef MAC
 	mac_associate_vnode_devfs(mp, de, vp);
 #endif
+	sx_xunlock(&dmp->dm_lock);
 	*vpp = vp;
 	return (0);
 }
@@ -456,7 +511,7 @@ devfs_kqfilter_f(struct file *fp, struct knote *kn)
 }
 
 static int
-devfs_lookupx(struct vop_lookup_args *ap)
+devfs_lookupx(struct vop_lookup_args *ap, int *dm_unlock)
 {
 	struct componentname *cnp;
 	struct vnode *dvp, **vpp;
@@ -508,6 +563,7 @@ devfs_lookupx(struct vop_lookup_args *ap)
 		de = TAILQ_NEXT(de, de_list);		/* ".." */
 		de = de->de_dir;
 		error = devfs_allocv(de, dvp->v_mount, vpp, td);
+		*dm_unlock = 0;
 		vn_lock(dvp, LK_EXCLUSIVE | LK_RETRY, td);
 		return (error);
 	}
@@ -565,6 +621,7 @@ devfs_lookupx(struct vop_lookup_args *ap)
 		}
 	}
 	error = devfs_allocv(de, dvp->v_mount, vpp, td);
+	*dm_unlock = 0;
 	return (error);
 }
 
@@ -573,11 +630,14 @@ devfs_lookup(struct vop_lookup_args *ap)
 {
 	int j;
 	struct devfs_mount *dmp;
+	int dm_unlock;
 
 	dmp = VFSTODEVFS(ap->a_dvp->v_mount);
+	dm_unlock = 1;
 	sx_xlock(&dmp->dm_lock);
-	j = devfs_lookupx(ap);
+	j = devfs_lookupx(ap, &dm_unlock);
-	sx_xunlock(&dmp->dm_lock);
+	if (dm_unlock == 1)
+		sx_xunlock(&dmp->dm_lock);
 	return (j);
 }
 
@@ -599,7 +659,6 @@ devfs_mknod(struct vop_mknod_args *ap)
 		return (EOPNOTSUPP);
 	dvp = ap->a_dvp;
 	dmp = VFSTODEVFS(dvp->v_mount);
-	sx_xlock(&dmp->dm_lock);
 
 	cnp = ap->a_cnp;
 	vpp = ap->a_vpp;
@@ -607,6 +666,7 @@ devfs_mknod(struct vop_mknod_args *ap)
 	dd = dvp->v_data;
 
 	error = ENOENT;
+	sx_xlock(&dmp->dm_lock);
 	TAILQ_FOREACH(de, &dd->de_dlist, de_list) {
 		if (cnp->cn_namelen != de->de_dirent->d_namlen)
 			continue;
@@ -621,6 +681,7 @@ devfs_mknod(struct vop_mknod_args *ap)
 		goto notfound;
 	de->de_flags &= ~DE_WHITEOUT;
 	error = devfs_allocv(de, dvp->v_mount, vpp, td);
+	return (error);
 notfound:
 	sx_xunlock(&dmp->dm_lock);
 	return (error);
@@ -1124,9 +1185,7 @@ devfs_symlink(struct vop_symlink_args *ap)
 	mac_create_devfs_symlink(ap->a_cnp->cn_cred, dmp->dm_mount, dd, de);
 #endif
 	TAILQ_INSERT_TAIL(&dd->de_dlist, de, de_list);
-	devfs_allocv(de, ap->a_dvp->v_mount, ap->a_vpp, td);
+	return (devfs_allocv(de, ap->a_dvp->v_mount, ap->a_vpp, td));
-	sx_xunlock(&dmp->dm_lock);
-	return (0);
 }
 
 /* ARGSUSED */