Re: CVS commit: src/sys

[Michael van Elst <mlelstv%serpens.de@localhost>]

On Sun, Apr 28, 2013 at 06:36:36PM +0200, Michael van Elst wrote:
> On Sun, Apr 28, 2013 at 06:03:44PM +0200, J. Hannken-Illjes wrote:
> 
> > Unfortunately this also opens a race for do_sys_sync to succeed with
> > vfs_busy() and call ffs_sync() with mp->mnt_data == NULL -> BOMB.
> 
> Setting IMNT_GONE earlier while holding umounting lock would prevent this
> because it lets dounmount() and vfs_busy() fail. But then we have a
> mountpoint in the list with that flag set. Do you know any side effects
> of this?

Here is a diff.

Index: sys/kern/vfs_mount.c
===================================================================
RCS file: /cvsroot/src/sys/kern/vfs_mount.c,v
retrieving revision 1.18
diff -u -r1.18 vfs_mount.c
--- sys/kern/vfs_mount.c        26 Apr 2013 22:27:16 -0000      1.18
+++ sys/kern/vfs_mount.c        28 Apr 2013 16:40:32 -0000
@@ -866,18 +866,29 @@
        }
        mutex_exit(&mp->mnt_updating);
        vfs_scrubvnlist(mp);
+
+       /*
+        * release mnt_umounting lock here, because other code calls
+        * vfs_busy() while holding the mountlist_lock.
+        *
+        * mark filesystem as gone to prevent further umounts
+        * after mnt_umounting lock is gone, this also prevents
+        * vfs_busy() from succeeding.
+        */
+       mp->mnt_iflag |= IMNT_GONE;
+       mutex_exit(&mp->mnt_unmounting);
+
        mutex_enter(&mountlist_lock);
        if ((coveredvp = mp->mnt_vnodecovered) != NULLVP)
                coveredvp->v_mountedhere = NULL;
        CIRCLEQ_REMOVE(&mountlist, mp, mnt_list);
-       mp->mnt_iflag |= IMNT_GONE;
        mutex_exit(&mountlist_lock);
        if (TAILQ_FIRST(&mp->mnt_vnodelist) != NULL)
                panic("unmount: dangling vnode");
        if (used_syncer)
                mutex_exit(&syncer_mutex);
        vfs_hooks_unmount(mp);
-       mutex_exit(&mp->mnt_unmounting);
+
        vfs_destroy(mp);        /* reference from mount() */
        if (coveredvp != NULLVP) {
                vrele(coveredvp);

-- 
                                Michael van Elst
Internet: mlelstv%serpens.de@localhost
                                "A potential Snark may lurk in every tree."