Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 16 Dec 2008 23:16:10 +0000 (UTC)
From:      Attilio Rao <attilio@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r186197 - in head/sys: kern sys
Message-ID:  <200812162316.mBGNGASN050237@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: attilio
Date: Tue Dec 16 23:16:10 2008
New Revision: 186197
URL: http://svn.freebsd.org/changeset/base/186197

Log:
  1) Fix a deadlock in the VFS:
  - threadA runs vfs_rel(mp1)
  - threadB does unmount the mp1 fs, sets MNTK_UNMOUNT and drop MNT_ILOCK()
  - threadA runs vfs_busy(mp1) and, as long as, MNTK_UNMOUNT is set, sleeps
    waiting for threadB to complete the unmount
  - threadB, in vfs_mount_destroy(), finds mnt_lock > 0 and sleeps waiting
    for the refcount to expire.
  
  Fix the deadlock by adding a flag called MNTK_REFEXPIRE which signals the
  unmounter is waiting for mnt_ref to expire.
  The vfs_busy contenders got awake, fails, and if they retry the
  MNTK_REFEXPIRE won't allow them to sleep again.
  
  2) Simplify significantly the code of vfs_mount_destroy() trimming
     unnecessary codes:
     - as long as any reference exited, it is no-more possible to have
       write-op (primarty and secondary) in progress.
     - it is no needed to drop and reacquire the mount lock.
     - filling the structures with dummy values is unuseful as long as
       it is going to be freed.
  
  Tested by:	pho, Andrea Barberio <insomniac at slackware dot it>
  Discussed with:	kib

Modified:
  head/sys/kern/vfs_mount.c
  head/sys/kern/vfs_subr.c
  head/sys/sys/mount.h

Modified: head/sys/kern/vfs_mount.c
==============================================================================
--- head/sys/kern/vfs_mount.c	Tue Dec 16 23:06:36 2008	(r186196)
+++ head/sys/kern/vfs_mount.c	Tue Dec 16 23:16:10 2008	(r186197)
@@ -507,29 +507,20 @@ vfs_mount_destroy(struct mount *mp)
 {
 
 	MNT_ILOCK(mp);
+	mp->mnt_kern_flag |= MNTK_REFEXPIRE;
+	if (mp->mnt_kern_flag & MNTK_MWAIT) {
+		mp->mnt_kern_flag &= ~MNTK_MWAIT;
+		wakeup(mp);
+	}
 	while (mp->mnt_ref)
 		msleep(mp, MNT_MTX(mp), PVFS, "mntref", 0);
-	if (mp->mnt_writeopcount > 0) {
-		printf("Waiting for mount point write ops\n");
-		while (mp->mnt_writeopcount > 0) {
-			mp->mnt_kern_flag |= MNTK_SUSPEND;
-			msleep(&mp->mnt_writeopcount,
-			       MNT_MTX(mp),
-			       PZERO, "mntdestroy2", 0);
-		}
-		printf("mount point write ops completed\n");
-	}
-	if (mp->mnt_secondary_writes > 0) {
-		printf("Waiting for mount point secondary write ops\n");
-		while (mp->mnt_secondary_writes > 0) {
-			mp->mnt_kern_flag |= MNTK_SUSPEND;
-			msleep(&mp->mnt_secondary_writes,
-			       MNT_MTX(mp),
-			       PZERO, "mntdestroy3", 0);
-		}
-		printf("mount point secondary write ops completed\n");
-	}
-	MNT_IUNLOCK(mp);
+	KASSERT(mp->mnt_ref == 0,
+	    ("%s: invalid refcount in the drain path @ %s:%d", __func__,
+	    __FILE__, __LINE__));
+	if (mp->mnt_writeopcount != 0)
+		panic("vfs_mount_destroy: nonzero writeopcount");
+	if (mp->mnt_secondary_writes != 0)
+		panic("vfs_mount_destroy: nonzero secondary_writes");
 	mp->mnt_vfc->vfc_refcount--;
 	if (!TAILQ_EMPTY(&mp->mnt_nvnodelist)) {
 		struct vnode *vp;
@@ -538,18 +529,10 @@ vfs_mount_destroy(struct mount *mp)
 			vprint("", vp);
 		panic("unmount: dangling vnode");
 	}
-	MNT_ILOCK(mp);
-	if (mp->mnt_kern_flag & MNTK_MWAIT)
-		wakeup(mp);
-	if (mp->mnt_writeopcount != 0)
-		panic("vfs_mount_destroy: nonzero writeopcount");
-	if (mp->mnt_secondary_writes != 0)
-		panic("vfs_mount_destroy: nonzero secondary_writes");
 	if (mp->mnt_nvnodelistsize != 0)
 		panic("vfs_mount_destroy: nonzero nvnodelistsize");
-	mp->mnt_writeopcount = -1000;
-	mp->mnt_nvnodelistsize = -1000;
-	mp->mnt_secondary_writes = -1000;
+	if (mp->mnt_lockref != 0)
+		panic("vfs_mount_destroy: nonzero lock refcount");
 	MNT_IUNLOCK(mp);
 #ifdef MAC
 	mac_mount_destroy(mp);

Modified: head/sys/kern/vfs_subr.c
==============================================================================
--- head/sys/kern/vfs_subr.c	Tue Dec 16 23:06:36 2008	(r186196)
+++ head/sys/kern/vfs_subr.c	Tue Dec 16 23:16:10 2008	(r186197)
@@ -345,7 +345,7 @@ vfs_busy(struct mount *mp, int flags)
 	MNT_ILOCK(mp);
 	MNT_REF(mp);
 	if (mp->mnt_kern_flag & MNTK_UNMOUNT) {
-		if (flags & MBF_NOWAIT) {
+		if (flags & MBF_NOWAIT || mp->mnt_kern_flag & MNTK_REFEXPIRE) {
 			MNT_REL(mp);
 			MNT_IUNLOCK(mp);
 			return (ENOENT);

Modified: head/sys/sys/mount.h
==============================================================================
--- head/sys/sys/mount.h	Tue Dec 16 23:06:36 2008	(r186196)
+++ head/sys/sys/mount.h	Tue Dec 16 23:16:10 2008	(r186197)
@@ -316,6 +316,7 @@ void          __mnt_vnode_markerfree(str
 #define MNTK_SOFTDEP	0x00000004	/* async disabled by softdep */
 #define MNTK_NOINSMNTQ	0x00000008	/* insmntque is not allowed */
 #define	MNTK_DRAINING	0x00000010	/* lock draining is happening */
+#define	MNTK_REFEXPIRE	0x00000020	/* refcount expiring is happening */
 #define MNTK_UNMOUNT	0x01000000	/* unmount in progress */
 #define	MNTK_MWAIT	0x02000000	/* waiting for unmount to finish */
 #define	MNTK_SUSPEND	0x08000000	/* request write suspension */

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