Source-Changes-D archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: CVS commit: src/sys



On Sun, Apr 28, 2013 at 01:50:37PM +0200, J. Hannken-Illjes wrote:

> > The locking order hasn't been changed, but the faulty tryenter has been
> > replaced with an enter.
> 
> Sure -- and that is the problem.  The locking order was wrong and you
> removed the hack/workaround without fixing the lock order.

I now release the mnt_unmounting lock before aquiring the mountlist_lock.
This opens a race condition for a concurrent umount that I prevent by
raising the nest counter. With this change I can no longer provoke
the deadlock.

The only difference should now be that vfs_hooks_unmount() is called
outside the unmounting lock. But since it is operating on an already
detached mountpoint (if it is used at all) before it is destroyed,
that should be harmless.

What do you think?

Index: vfs_mount.c
===================================================================
RCS file: /cvsroot/src/sys/kern/vfs_mount.c,v
retrieving revision 1.18
diff -u -r1.18 vfs_mount.c
--- vfs_mount.c 26 Apr 2013 22:27:16 -0000      1.18
+++ vfs_mount.c 28 Apr 2013 14:30:55 -0000
@@ -866,6 +866,18 @@
        }
        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 busy to prevent further umounts
+        * after mnt_umounting lock is gone
+        */
+       ++mp->mnt_busynest;
+       KASSERT(mp->mnt_busynest != 0);
+       mutex_exit(&mp->mnt_unmounting);
+
        mutex_enter(&mountlist_lock);
        if ((coveredvp = mp->mnt_vnodecovered) != NULLVP)
                coveredvp->v_mountedhere = NULL;
@@ -877,7 +889,7 @@
        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);



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


Home | Main Index | Thread Index | Old Index