Tweak softdep_change_linkcnt() so we can specify whether we're willing

to be co-opted by softdep or not, and use this new interface to inform
softdep in ufs_inactive() that the inode mode has changed.

We don't want to be co-opted there as that might lead to undesired
circular dependencies such as a vput() depending on another vput() to
complete, or a process that is trying to free up a vnode being blocked
trying to acquire a new vnode.

Okay tedu@ deraadt@, thanks to all those who tested.

6 files changed, 52 insertions, 35 deletions

diff --git a/sys/ufs/ffs/ffs_softdep.c b/sys/ufs/ffs/ffs_softdep.c
index 17fda288d6b..8cd281c25a7 100644
--- a/sys/ufs/ffs/ffs_softdep.c
+++ b/sys/ufs/ffs/ffs_softdep.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ffs_softdep.c,v 1.56 2005/05/24 04:33:35 pedro Exp $	*/
+/*	$OpenBSD: ffs_softdep.c,v 1.57 2005/06/10 17:37:40 pedro Exp $	*/
 /*
  * Copyright 1998, 2000 Marshall Kirk McKusick. All Rights Reserved.
  *
@@ -2901,18 +2901,30 @@ softdep_setup_directory_change(bp, dp, ip, newinum, isrmdir)
  * inode has been written.
  */
 void
-softdep_change_linkcnt(ip)
+softdep_change_linkcnt(ip, nodelay)
 	struct inode *ip;	/* the inode with the increased link count */
+	int nodelay;		/* do background work or not */
 {
 	struct inodedep *inodedep;
+	int flags;
+
+	/*
+	 * If requested, do not allow background work to happen.
+	 */
+	flags = DEPALLOC;
+	if (nodelay)
+		flags |= NODELAY;
 
 	ACQUIRE_LOCK(&lk);
-	(void) inodedep_lookup(ip->i_fs, ip->i_number, DEPALLOC, &inodedep);
+
+	(void) inodedep_lookup(ip->i_fs, ip->i_number, flags, &inodedep);
 	if (ip->i_ffs_nlink < ip->i_effnlink) {
 		FREE_LOCK(&lk);
 		panic("softdep_change_linkcnt: bad delta");
 	}
+
 	inodedep->id_nlinkdelta = ip->i_ffs_nlink - ip->i_effnlink;
+
 	FREE_LOCK(&lk);
 }
 
diff --git a/sys/ufs/ffs/ffs_softdep_stub.c b/sys/ufs/ffs/ffs_softdep_stub.c
index 6b6b4f57d1b..36d55c1cdf7 100644
--- a/sys/ufs/ffs/ffs_softdep_stub.c
+++ b/sys/ufs/ffs/ffs_softdep_stub.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ffs_softdep_stub.c,v 1.8 2004/12/07 06:06:51 deraadt Exp $	*/
+/*	$OpenBSD: ffs_softdep_stub.c,v 1.9 2005/06/10 17:37:40 pedro Exp $	*/
 
 /*
  * Copyright 1998 Marshall Kirk McKusick. All Rights Reserved.
@@ -202,8 +202,9 @@ softdep_setup_directory_change(bp, dp, ip, newinum, isrmdir)
 }
 
 void
-softdep_change_linkcnt(ip)
+softdep_change_linkcnt(ip, nodelay)
 	struct inode *ip;
+	int nodelay;
 {
 
 	panic("softdep_change_linkcnt called");
diff --git a/sys/ufs/ufs/ufs_extern.h b/sys/ufs/ufs/ufs_extern.h
index c40407d5545..d73eaf6ade1 100644
--- a/sys/ufs/ufs/ufs_extern.h
+++ b/sys/ufs/ufs/ufs_extern.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ufs_extern.h,v 1.23 2005/05/28 02:02:50 pedro Exp $	*/
+/*	$OpenBSD: ufs_extern.h,v 1.24 2005/06/10 17:37:41 pedro Exp $	*/
 /*	$NetBSD: ufs_extern.h,v 1.5 1996/02/09 22:36:03 christos Exp $	*/
 
 /*-
@@ -156,7 +156,7 @@ void  softdep_setup_remove(struct buf *,struct inode *, struct inode *,
           int);
 void  softdep_setup_directory_change(struct buf *, struct inode *,
           struct inode *, long, int);
-void  softdep_change_linkcnt(struct inode *);
+void  softdep_change_linkcnt(struct inode *, int);
 int   softdep_slowdown(struct vnode *);
 
 __END_DECLS
diff --git a/sys/ufs/ufs/ufs_inode.c b/sys/ufs/ufs/ufs_inode.c
index 2be3b167bb8..743e6222b4a 100644
--- a/sys/ufs/ufs/ufs_inode.c
+++ b/sys/ufs/ufs/ufs_inode.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ufs_inode.c,v 1.28 2005/05/28 17:56:28 art Exp $	*/
+/*	$OpenBSD: ufs_inode.c,v 1.29 2005/06/10 17:37:41 pedro Exp $	*/
 /*	$NetBSD: ufs_inode.c,v 1.7 1996/05/11 18:27:52 mycroft Exp $	*/
 
 /*
@@ -96,16 +96,20 @@ ufs_inactive(v)
 		ip->i_ffs_mode = 0;
 		ip->i_flag |= IN_CHANGE | IN_UPDATE;
 
-#if 0
 		/*
 		 * Setting the mode to zero needs to wait for the inode to be
 		 * written just as does a change to the link count. So, rather
 		 * than creating a new entry point to do the same thing, we
-		 * just use softdep_change_linkcnt().
+		 * just use softdep_change_linkcnt(). Also, we can't let
+		 * softdep co-opt us to help on its worklist, as we may end up
+		 * trying to recycle vnodes and getting to this same point a
+		 * couple of times, blowing the kernel stack. However, this
+		 * could be optimized by checking if we are coming from
+		 * vrele(), vput() or vclean() (by checking for VXLOCK) and
+		 * just avoiding the co-opt to happen in the last case.
 		 */
 		if (DOINGSOFTDEP(vp))
-			softdep_change_linkcnt(ip);
-#endif
+			softdep_change_linkcnt(ip, 1);
 
 		UFS_INODE_FREE(ip, ip->i_number, mode);
 	}
diff --git a/sys/ufs/ufs/ufs_lookup.c b/sys/ufs/ufs/ufs_lookup.c
index 14989e52557..206e11534c2 100644
--- a/sys/ufs/ufs/ufs_lookup.c
+++ b/sys/ufs/ufs/ufs_lookup.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ufs_lookup.c,v 1.28 2004/12/07 04:37:28 tedu Exp $	*/
+/*	$OpenBSD: ufs_lookup.c,v 1.29 2005/06/10 17:37:41 pedro Exp $	*/
 /*	$NetBSD: ufs_lookup.c,v 1.7 1996/02/09 22:36:06 christos Exp $	*/
 
 /*
@@ -1037,7 +1037,7 @@ out:
  	if (DOINGSOFTDEP(dvp)) {
 		if (ip) {
 			ip->i_effnlink--;
-			softdep_change_linkcnt(ip);
+			softdep_change_linkcnt(ip, 0);
 			softdep_setup_remove(bp, dp, ip, isrmdir);
 		}
 		if (softdep_slowdown(dvp)) {
@@ -1089,7 +1089,7 @@ ufs_dirrewrite(dp, oip, newinum, newtype, isrmdir)
  		ep->d_type = newtype;
  	oip->i_effnlink--;
  	if (DOINGSOFTDEP(vdp)) {
-		softdep_change_linkcnt(oip);
+		softdep_change_linkcnt(oip, 0);
  		softdep_setup_directory_change(bp, dp, oip, newinum, isrmdir);
  		bdwrite(bp);
  	} else {
diff --git a/sys/ufs/ufs/ufs_vnops.c b/sys/ufs/ufs/ufs_vnops.c
index d5ce9939f29..d1596699ad2 100644
--- a/sys/ufs/ufs/ufs_vnops.c
+++ b/sys/ufs/ufs/ufs_vnops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ufs_vnops.c,v 1.63 2005/05/28 02:02:50 pedro Exp $	*/
+/*	$OpenBSD: ufs_vnops.c,v 1.64 2005/06/10 17:37:41 pedro Exp $	*/
 /*	$NetBSD: ufs_vnops.c,v 1.18 1996/05/11 18:28:04 mycroft Exp $	*/
 
 /*
@@ -720,7 +720,7 @@ ufs_link(v)
 	ip->i_ffs_nlink++;
 	ip->i_flag |= IN_CHANGE;
 	if (DOINGSOFTDEP(vp))
-		softdep_change_linkcnt(ip);
+		softdep_change_linkcnt(ip, 0);
 	if ((error = UFS_UPDATE(ip, !DOINGSOFTDEP(vp))) == 0) {
 		ufs_makedirentry(ip, cnp, &newdir);
 		error = ufs_direnter(dvp, vp, &newdir, cnp, NULL);
@@ -730,7 +730,7 @@ ufs_link(v)
 		ip->i_ffs_nlink--;
 		ip->i_flag |= IN_CHANGE;
 		if (DOINGSOFTDEP(vp))
-			softdep_change_linkcnt(ip);
+			softdep_change_linkcnt(ip, 0);
 	}
 	pool_put(&namei_pool, cnp->cn_pnbuf);
 	VN_KNOTE(vp, NOTE_LINK);
@@ -930,7 +930,7 @@ abortit:
 	ip->i_ffs_nlink++;
 	ip->i_flag |= IN_CHANGE;
 	if (DOINGSOFTDEP(fvp))
-		softdep_change_linkcnt(ip);
+		softdep_change_linkcnt(ip, 0);
 	if ((error = UFS_UPDATE(ip, !DOINGSOFTDEP(fvp))) != 0) {
 		VOP_UNLOCK(fvp, 0, p);
 		goto bad;
@@ -997,14 +997,14 @@ abortit:
 			dp->i_ffs_nlink++;
 			dp->i_flag |= IN_CHANGE;
 			if (DOINGSOFTDEP(tdvp))
-                               softdep_change_linkcnt(dp);
+                               softdep_change_linkcnt(dp, 0);
 			if ((error = UFS_UPDATE(dp, !DOINGSOFTDEP(tdvp))) 
 			    != 0) {
 				dp->i_effnlink--;
 				dp->i_ffs_nlink--;
 				dp->i_flag |= IN_CHANGE;
 				if (DOINGSOFTDEP(tdvp))
-					softdep_change_linkcnt(dp);
+					softdep_change_linkcnt(dp, 0);
 				goto bad;
 			}
 		}
@@ -1015,7 +1015,7 @@ abortit:
 				dp->i_ffs_nlink--;
 				dp->i_flag |= IN_CHANGE;
 				if (DOINGSOFTDEP(tdvp))
-					softdep_change_linkcnt(dp);
+					softdep_change_linkcnt(dp, 0);
 				(void)UFS_UPDATE(dp, 1);
 			}
 			goto bad;
@@ -1071,11 +1071,11 @@ abortit:
 			if (!newparent) {
 				dp->i_effnlink--;
 				if (DOINGSOFTDEP(tdvp))
-					softdep_change_linkcnt(dp);
+					softdep_change_linkcnt(dp, 0);
 			}
 			xp->i_effnlink--;
 			if (DOINGSOFTDEP(tvp))
-				softdep_change_linkcnt(xp);
+				softdep_change_linkcnt(xp, 0);
 		}
 		if (doingdirectory && !DOINGSOFTDEP(tvp)) {
 		       /*
@@ -1182,7 +1182,7 @@ out:
 		ip->i_flag |= IN_CHANGE;
 		ip->i_flag &= ~IN_RENAME;
 		if (DOINGSOFTDEP(fvp))
-			softdep_change_linkcnt(ip);
+			softdep_change_linkcnt(ip, 0);
 		vput(fvp);
 	} else
 		vrele(fvp);
@@ -1249,7 +1249,7 @@ ufs_mkdir(v)
 	ip->i_effnlink = 2;
 	ip->i_ffs_nlink = 2;
 	if (DOINGSOFTDEP(tvp))
-		softdep_change_linkcnt(ip);
+		softdep_change_linkcnt(ip, 0);
 
 	if (cnp->cn_flags & ISWHITEOUT)
 		ip->i_ffs_flags |= UF_OPAQUE;
@@ -1263,7 +1263,7 @@ ufs_mkdir(v)
 	dp->i_ffs_nlink++;
 	dp->i_flag |= IN_CHANGE;
 	if (DOINGSOFTDEP(dvp))
-		softdep_change_linkcnt(dp);
+		softdep_change_linkcnt(dp, 0);
 	if ((error = UFS_UPDATE(dp, !DOINGSOFTDEP(dvp))) != 0)
 		goto bad;
 
@@ -1329,7 +1329,7 @@ bad:
                 dp->i_ffs_nlink--;
                 dp->i_flag |= IN_CHANGE;
 		if (DOINGSOFTDEP(dvp))
-			softdep_change_linkcnt(dp);
+			softdep_change_linkcnt(dp, 0);
                 /*
                  * No need to do an explicit VOP_TRUNCATE here, vrele will
                  * do this for us because we set the link count to 0.
@@ -1338,7 +1338,7 @@ bad:
                 ip->i_ffs_nlink = 0;
                 ip->i_flag |= IN_CHANGE;
 		if (DOINGSOFTDEP(tvp))
-			softdep_change_linkcnt(ip);
+			softdep_change_linkcnt(ip, 0);
 		vput(tvp);
 	}
 out:
@@ -1408,15 +1408,15 @@ ufs_rmdir(v)
 	dp->i_effnlink--;
 	ip->i_effnlink--;
 	if (DOINGSOFTDEP(vp)) {
-		softdep_change_linkcnt(dp);
-		softdep_change_linkcnt(ip);
+		softdep_change_linkcnt(dp, 0);
+		softdep_change_linkcnt(ip, 0);
 	}
 	if ((error = ufs_dirremove(dvp, ip, cnp->cn_flags, 1)) != 0) {
 		dp->i_effnlink++;
 		ip->i_effnlink++;
 		if (DOINGSOFTDEP(vp)) {
-			softdep_change_linkcnt(dp);
-			softdep_change_linkcnt(ip);
+			softdep_change_linkcnt(dp, 0);
+			softdep_change_linkcnt(ip, 0);
 		}
 		goto out;
 	}
@@ -2082,7 +2082,7 @@ ufs_makeinode(mode, dvp, vpp, cnp)
 	ip->i_effnlink = 1;
 	ip->i_ffs_nlink = 1;
 	if (DOINGSOFTDEP(tvp))
-		softdep_change_linkcnt(ip);
+		softdep_change_linkcnt(ip, 0);
 	if ((ip->i_ffs_mode & ISGID) &&
 		!groupmember(ip->i_ffs_gid, cnp->cn_cred) &&
 	    suser_ucred(cnp->cn_cred))
@@ -2118,7 +2118,7 @@ bad:
 	ip->i_ffs_nlink = 0;
 	ip->i_flag |= IN_CHANGE;
 	if (DOINGSOFTDEP(tvp))
-		softdep_change_linkcnt(ip);
+		softdep_change_linkcnt(ip, 0);
 	tvp->v_type = VNON;
 	vput(tvp);