Change the locking on the mountpoint slightly. Instead of using mnt_lock

to get shared locks for lookup and get the exclusive lock only with
LK_DRAIN on unmount and do the real exclusive locking with flags in
mnt_flags, we now use shared locks for lookup and an exclusive lock for
unmount.

This is accomplished by slightly changing the semantics of vfs_busy.
Old vfs_busy behavior:
 - with LK_NOWAIT set in flags, a shared lock was obtained if the
   mountpoint wasn't being unmounted, otherwise we just returned an error.
 - with no flags, a shared lock was obtained if the mountpoint was being
   unmounted, otherwise we slept until the unmount was done and returned
   an error.
LK_NOWAIT was used for sync(2) and some statistics code where it isn't really
critical that we get the correct results.
0 was used in fchdir and lookup where it's critical that we get the right
directory vnode for the filesystem root.

After this change vfs_busy keeps the same behavior for no flags and LK_NOWAIT.
But if some other flags are passed into it, they are passed directly
into lockmgr (actually LK_SLEEPFAIL is always added to those flags because
if we sleep for the lock, that means someone was holding the exclusive lock
and the exclusive lock is only held when the filesystem is being unmounted.

More changes:
 dounmount must now be called with the exclusive lock held. (before this
 the caller was supposed to hold the vfs_busy lock, but that wasn't always
 true).
 Zap some (now) unused mount flags.
And the highlight of this change:
 Add some vfs_busy calls to match some vfs_unbusy calls, especially in
 sys_mount. (lockmgr doesn't detect the case where we release a lock noone
 holds (it will do that soon)).

If you've seen hangs on reboot with mfs this should solve it (I repeat this
for the fourth time now, but this time I spent two months fixing and
redesigning this and reading the code so this time I must have gotten
this right).

8 files changed, 60 insertions, 70 deletions

diff --git a/sys/compat/ultrix/ultrix_fs.c b/sys/compat/ultrix/ultrix_fs.c
index a0cfa887030..fa09441a86d 100644
--- a/sys/compat/ultrix/ultrix_fs.c
+++ b/sys/compat/ultrix/ultrix_fs.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ultrix_fs.c,v 1.9 2002/06/03 12:04:08 deraadt Exp $	*/
+/*	$OpenBSD: ultrix_fs.c,v 1.10 2002/07/12 14:02:23 art Exp $	*/
 /*	$NetBSD: ultrix_fs.c,v 1.4 1996/04/07 17:23:06 jonathan Exp $	*/
 
 /*
@@ -244,7 +244,7 @@ ultrix_sys_getmnt(p, v, retval)
 	for (count = 0, mp = mountlist.cqh_first;
 	    mp != (void *)&mountlist && count < maxcount; mp = nmp) {
 		nmp = mp->mnt_list.cqe_next;
-		if (sfsp != NULL && (mp->mnt_flag & MNT_MLOCK) == 0) {
+		if (sfsp != NULL) {
 			struct ultrix_fs_data tem;
 			sp = &mp->mnt_stat;
 
diff --git a/sys/kern/vfs_subr.c b/sys/kern/vfs_subr.c
index 4d899a48ce5..3f70cc8ad1d 100644
--- a/sys/kern/vfs_subr.c
+++ b/sys/kern/vfs_subr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vfs_subr.c,v 1.86 2002/06/16 16:54:25 miod Exp $	*/
+/*	$OpenBSD: vfs_subr.c,v 1.87 2002/07/12 14:02:22 art Exp $	*/
 /*	$NetBSD: vfs_subr.c,v 1.53 1996/04/22 01:39:13 christos Exp $	*/
 
 /*
@@ -146,39 +146,44 @@ vntblinit()
 	vn_initialize_syncerd();
 }
 
-
 /*
  * Mark a mount point as busy. Used to synchronize access and to delay
  * unmounting. Interlock is not released on failure.
+ *
+ * historical behavior:
+ *  - LK_NOWAIT means that we should just ignore the mount point if it's
+ *     being unmounted.
+ *  - no flags means that we should sleep on the mountpoint and then
+ *     fail.
  */
 
 int
-vfs_busy(mp, flags, interlkp, p)
-	struct mount *mp;
-	int flags;
-	struct simplelock *interlkp;
-	struct proc *p;
+vfs_busy(struct mount *mp, int flags, struct simplelock *interlkp,
+    struct proc *p)
 {
 	int lkflags;
 
-	if (mp->mnt_flag & MNT_UNMOUNT) {
-		if (flags & LK_NOWAIT)
-			return (ENOENT);
-		mp->mnt_flag |= MNT_MWAIT;
-		/*
-		 * Since all busy locks are shared except the exclusive
-		 * lock granted when unmounting, the only place that a
-		 * wakeup needs to be done is at the release of the
-		 * exclusive lock at the end of dounmount.
-		 */
-		ltsleep(mp, PVFS, "vfs_bsy", 0, interlkp);
-		return (ENOENT);
+	switch (flags) {
+	case LK_NOWAIT:
+		lkflags = LK_SHARED|LK_NOWAIT;
+		break;
+	case 0:
+		lkflags = LK_SHARED;
+		break;
+	default:
+		lkflags = flags;
 	}
-	lkflags = LK_SHARED;
+
+	/*
+	 * Always sleepfail. We will only sleep for an exclusive lock
+	 * and the exclusive lock will only be acquired when unmounting.
+	 */
+	lkflags |= LK_SLEEPFAIL;
+
 	if (interlkp)
 		lkflags |= LK_INTERLOCK;
 	if (lockmgr(&mp->mnt_lock, lkflags, interlkp, p))
-		panic("vfs_busy: unexpected lock failure");
+		return (ENOENT);
 	return (0);
 }
 
@@ -187,9 +192,7 @@ vfs_busy(mp, flags, interlkp, p)
  * Free a busy file system
  */
 void
-vfs_unbusy(mp, p)
-	struct mount *mp;
-	struct proc *p;
+vfs_unbusy(struct mount *mp, struct proc *p)
 {
 	lockmgr(&mp->mnt_lock, LK_RELEASE, NULL, p);
 }
@@ -1747,16 +1750,19 @@ vaccess(file_mode, uid, gid, acc_mode, cred)
  * will avoid needing to worry about dependencies.
  */
 void
-vfs_unmountall()
+vfs_unmountall(void)
 {
-	register struct mount *mp, *nmp;
+	struct mount *mp, *nmp;
 	int allerror, error, again = 1;
+	struct proc *p = curproc;
 
  retry:
 	allerror = 0;
 	for (mp = CIRCLEQ_LAST(&mountlist); mp != CIRCLEQ_END(&mountlist);
 	    mp = nmp) {
 		nmp = CIRCLEQ_PREV(mp, mnt_list);
+		if ((vfs_busy(mp, LK_EXCLUSIVE|LK_NOWAIT, NULL, p)) != 0)
+			continue;
 		if ((error = dounmount(mp, MNT_FORCE, curproc)) != 0) {
 			printf("unmount of %s failed with error %d\n",
 			    mp->mnt_stat.f_mntonname, error);
diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c
index a95ec82a9c7..68f91738ade 100644
--- a/sys/kern/vfs_syscalls.c
+++ b/sys/kern/vfs_syscalls.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vfs_syscalls.c,v 1.95 2002/03/14 01:27:06 millert Exp $	*/
+/*	$OpenBSD: vfs_syscalls.c,v 1.96 2002/07/12 14:02:22 art Exp $	*/
 /*	$NetBSD: vfs_syscalls.c,v 1.71 1996/04/23 10:29:02 mycroft Exp $	*/
 
 /*
@@ -167,6 +167,10 @@ sys_mount(p, v, retval)
 			}
 			SCARG(uap, flags) |= MNT_NOSUID | MNT_NODEV;
 		}
+		if ((error = vfs_busy(mp, LK_NOWAIT, 0, p)) != 0) {
+			vput(vp);
+			return (error);
+		}
 		VOP_UNLOCK(vp, 0, p);
 		goto update;
 	}
@@ -244,7 +248,9 @@ sys_mount(p, v, retval)
 		M_MOUNT, M_WAITOK);
 	bzero((char *)mp, (u_long)sizeof(struct mount));
 	lockinit(&mp->mnt_lock, PVFS, "vfslock", 0, 0);
-	vfs_busy(mp, LK_NOWAIT, 0, p);
+	/* This error never happens, but it makes auditing easier */
+	if ((error = vfs_busy(mp, LK_NOWAIT, 0, p)))
+		return (error);
 	mp->mnt_op = vfsp->vfc_vfsops;
 	mp->mnt_vfc = vfsp;
 	mp->mnt_flag |= (vfsp->vfc_flags & MNT_VISFLAGMASK);
@@ -421,7 +427,7 @@ sys_unmount(p, v, retval)
 	}
 	vput(vp);
 
-	if (vfs_busy(mp, 0, NULL, p))
+	if (vfs_busy(mp, LK_EXCLUSIVE, NULL, p))
 		return (EBUSY);
 
 	return (dounmount(mp, SCARG(uap, flags), p));
@@ -431,18 +437,11 @@ sys_unmount(p, v, retval)
  * Do the actual file system unmount.
  */
 int
-dounmount(mp, flags, p)
-	register struct mount *mp;
-	int flags;
-	struct proc *p;
+dounmount(struct mount *mp, int flags, struct proc *p)
 {
 	struct vnode *coveredvp;
 	int error;
 
-	simple_lock(&mountlist_slock);
-	mp->mnt_flag |= MNT_UNMOUNT;
-	vfs_unbusy(mp, p);
-	lockmgr(&mp->mnt_lock, LK_DRAIN | LK_INTERLOCK, &mountlist_slock, p);
  	mp->mnt_flag &=~ MNT_ASYNC;
  	cache_purgevfs(mp);	/* remove cache entries for this file sys */
  	if (mp->mnt_syncer != NULL)
@@ -455,12 +454,8 @@ dounmount(mp, flags, p)
  	if (error) {
  		if ((mp->mnt_flag & MNT_RDONLY) == 0 && mp->mnt_syncer == NULL)
  			(void) vfs_allocate_syncvnode(mp);
-		mp->mnt_flag &= ~MNT_UNMOUNT;
-		lockmgr(&mp->mnt_lock, LK_RELEASE | LK_INTERLOCK | LK_REENABLE,
+		lockmgr(&mp->mnt_lock, LK_RELEASE | LK_INTERLOCK,
 		    &mountlist_slock, p);
-		if (mp->mnt_flag & MNT_MWAIT)
-			wakeup((caddr_t)mp);
-		mp->mnt_flag &= ~MNT_MWAIT;
 		return (error);
 	}
 	CIRCLEQ_REMOVE(&mountlist, mp, mnt_list);
@@ -472,8 +467,6 @@ dounmount(mp, flags, p)
 	if (mp->mnt_vnodelist.lh_first != NULL)
 		panic("unmount: dangling vnode");
 	lockmgr(&mp->mnt_lock, LK_RELEASE | LK_INTERLOCK, &mountlist_slock, p);
-	if (mp->mnt_flag & MNT_MWAIT)
-		wakeup((caddr_t)mp);
 	free((caddr_t)mp, M_MOUNT);
 	return (0);
 }
diff --git a/sys/nfs/nfs_syscalls.c b/sys/nfs/nfs_syscalls.c
index 881f8a843d8..565ef94b6d4 100644
--- a/sys/nfs/nfs_syscalls.c
+++ b/sys/nfs/nfs_syscalls.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nfs_syscalls.c,v 1.31 2002/06/11 15:45:44 hin Exp $	*/
+/*	$OpenBSD: nfs_syscalls.c,v 1.32 2002/07/12 14:02:23 art Exp $	*/
 /*	$NetBSD: nfs_syscalls.c,v 1.19 1996/02/18 11:53:52 fvdl Exp $	*/
 
 /*
@@ -177,8 +177,10 @@ nfs_clientd(struct nfsmount *nmp, struct ucred *cred, struct nfsd_cargs *ncd,
 		(nmp->nm_flag & (NFSMNT_WAITAUTH | NFSMNT_HASAUTH))) {
 		    error = tsleep((caddr_t)&nmp->nm_authstr, PSOCK | PCATCH,
 			"nqnfstimr", hz / 3);
-		    if (error == EINTR || error == ERESTART)
-			(void) dounmount(nmp->nm_mountp, MNT_FORCE, p);
+		    if (error == EINTR || error == ERESTART) {
+			if (vfs_busy(nmp->nm_mountp, LK_EXCLUSIVE, NULL, p) == 0)
+			    dounmount(nmp->nm_mountp, MNT_FORCE, p);
+		    }
 	    }
 	}
 
diff --git a/sys/sys/mount.h b/sys/sys/mount.h
index 04eefb10a46..ccb07578bcd 100644
--- a/sys/sys/mount.h
+++ b/sys/sys/mount.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: mount.h,v 1.47 2002/04/23 18:54:12 espie Exp $	*/
+/*	$OpenBSD: mount.h,v 1.48 2002/07/12 14:02:23 art Exp $	*/
 /*	$NetBSD: mount.h,v 1.48 1996/02/18 11:55:47 fvdl Exp $	*/
 
 /*
@@ -384,20 +384,11 @@ struct mount {
 
 /*
  * filesystem control flags.
- *
- * MNT_MLOCK lock the mount entry so that name lookup cannot proceed
- * past the mount point.  This keeps the subtree stable during mounts
- * and unmounts.
  */
 #define	MNT_UPDATE	0x00010000	/* not a real mount, just an update */
 #define	MNT_DELEXPORT	0x00020000	/* delete export host lists */
 #define	MNT_RELOAD	0x00040000	/* reload filesystem data */
 #define	MNT_FORCE	0x00080000	/* force unmount or readonly change */
-#define	MNT_MLOCK	0x00100000	/* lock so that subtree is stable */
-#define	MNT_MWAIT	0x00200000	/* someone is waiting for lock */
-#define MNT_MPBUSY	0x00400000	/* scan of mount point in progress */
-#define MNT_MPWANT	0x00800000	/* waiting for mount point */
-#define MNT_UNMOUNT	0x01000000	/* unmount in progress */
 #define MNT_WANTRDWR	0x02000000	/* want upgrade to read/write */
 #define MNT_SOFTDEP     0x04000000      /* soft dependencies being done */
 /*
diff --git a/sys/ufs/ffs/ffs_softdep.c b/sys/ufs/ffs/ffs_softdep.c
index ae95e681c3c..5851a301437 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.39 2002/06/08 04:51:05 art Exp $	*/
+/*	$OpenBSD: ffs_softdep.c,v 1.40 2002/07/12 14:02:23 art Exp $	*/
 /*
  * Copyright 1998, 2000 Marshall Kirk McKusick. All Rights Reserved.
  *
@@ -921,8 +921,6 @@ softdep_flushfiles(oldmnt, flags, p)
 	 * activity can keep us busy forever, so we just fail with EBUSY.
 	 */
 	if (loopcnt == 0) {
-		if (oldmnt->mnt_flag & MNT_UNMOUNT)
-			panic("softdep_flushfiles: looping");
 		error = EBUSY;
 	}
 	return (error);
diff --git a/sys/ufs/lfs/lfs_bio.c b/sys/ufs/lfs/lfs_bio.c
index 6b18e8b1a03..9fe2bb190e9 100644
--- a/sys/ufs/lfs/lfs_bio.c
+++ b/sys/ufs/lfs/lfs_bio.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: lfs_bio.c,v 1.7 2001/08/19 15:07:34 miod Exp $	*/
+/*	$OpenBSD: lfs_bio.c,v 1.8 2002/07/12 14:02:23 art Exp $	*/
 /*	$NetBSD: lfs_bio.c,v 1.5 1996/02/09 22:28:49 christos Exp $	*/
 
 /*
@@ -164,7 +164,7 @@ lfs_flush()
 	     mp = mp->mnt_list.cqe_next) {
 		/* The lock check below is to avoid races with unmount. */
 		if (!strncmp(&mp->mnt_stat.f_fstypename[0], MOUNT_LFS, MFSNAMELEN) &&
-		    (mp->mnt_flag & (MNT_MLOCK|MNT_RDONLY|MNT_UNMOUNT)) == 0 &&
+		    (mp->mnt_flag & (MNT_RDONLY)) == 0 &&
 		    !((((struct ufsmount *)mp->mnt_data))->ufsmount_u.lfs)->lfs_dirops ) {
 			/*
 			 * We set the queue to 0 here because we are about to
diff --git a/sys/ufs/mfs/mfs_vfsops.c b/sys/ufs/mfs/mfs_vfsops.c
index 9892a6dbade..e9e6a7013e8 100644
--- a/sys/ufs/mfs/mfs_vfsops.c
+++ b/sys/ufs/mfs/mfs_vfsops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: mfs_vfsops.c,v 1.21 2002/06/21 00:40:08 art Exp $	*/
+/*	$OpenBSD: mfs_vfsops.c,v 1.22 2002/07/12 14:02:23 art Exp $	*/
 /*	$NetBSD: mfs_vfsops.c,v 1.10 1996/02/09 22:31:28 christos Exp $	*/
 
 /*
@@ -258,10 +258,10 @@ mfs_start(mp, flags, p)
 	int flags;
 	struct proc *p;
 {
-	register struct vnode *vp = VFSTOUFS(mp)->um_devvp;
-	register struct mfsnode *mfsp = VTOMFS(vp);
-	register struct buf *bp;
-	register caddr_t base;
+	struct vnode *vp = VFSTOUFS(mp)->um_devvp;
+	struct mfsnode *mfsp = VTOMFS(vp);
+	struct buf *bp;
+	caddr_t base;
 	int sleepreturn = 0;
 
 	base = mfsp->mfs_baseoff;
@@ -278,7 +278,7 @@ mfs_start(mp, flags, p)
 		 * EINTR/ERESTART.
 		 */
 		if (sleepreturn != 0) {
-			if (vfs_busy(mp, LK_NOWAIT, NULL, p) ||
+			if (vfs_busy(mp, LK_EXCLUSIVE|LK_NOWAIT, NULL, p) ||
 			    dounmount(mp, 0, p))
 				CLRSIG(p, CURSIG(p));
 			sleepreturn = 0;