[src/trunk]: src/sys When unmounting a file system, acquire the syncer_lock b...

[thorpej <thorpej%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/2bc8bd96ae21
branches:  trunk
changeset: 508656:2bc8bd96ae21
user:      thorpej <thorpej%NetBSD.org@localhost>
date:      Mon Apr 16 22:41:09 2001 +0000

description:
When unmounting a file system, acquire the syncer_lock before
vfs_busy'ing just before the dounmount() call.  This is to avoid
sleeping with the mountlist_slock held -- but we must acquire
syncer_lock before vfs_busy because the syncer itself uses
syncer_lock -> vfs_busy locking order.

diffstat:

 sys/coda/coda_psdev.c    |  13 +++++++++++--
 sys/kern/vfs_subr.c      |  11 +++++++++--
 sys/kern/vfs_syscalls.c  |  32 ++++++++++++++++++++++++--------
 sys/nfs/nfs_nqlease.c    |  14 +++++++++++---
 sys/ufs/mfs/mfs_vfsops.c |  14 +++++++++++---
 5 files changed, 66 insertions(+), 18 deletions(-)

diffs (187 lines):

diff -r ee209edd9858 -r 2bc8bd96ae21 sys/coda/coda_psdev.c
--- a/sys/coda/coda_psdev.c     Mon Apr 16 21:36:58 2001 +0000
+++ b/sys/coda/coda_psdev.c     Mon Apr 16 22:41:09 2001 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: coda_psdev.c,v 1.15 2000/12/27 22:06:07 jdolecek Exp $ */
+/*     $NetBSD: coda_psdev.c,v 1.16 2001/04/16 22:41:09 thorpej Exp $  */
 
 /*
  * 
@@ -70,6 +70,8 @@
 #include <sys/poll.h>
 #include <sys/select.h>
 
+#include <miscfs/syncfs/syncfs.h>
+
 #include <coda/coda.h>
 #include <coda/cnode.h>
 #include <coda/coda_namecache.h>
@@ -184,9 +186,16 @@
     }
 
     /* Let unmount know this is for real */
+    /*
+     * XXX Freeze syncer.  Must do this before locking the
+     * mount point.  See dounmount for details().
+     */
+    lockmgr(&syncer_lock, LK_EXCLUSIVE, NULL);
     VTOC(mi->mi_rootvp)->c_flags |= C_UNMOUNTING;
-    if (vfs_busy(mi->mi_vfsp, 0, 0))
+    if (vfs_busy(mi->mi_vfsp, 0, 0)) {
+       lockmgr(&syncer_lock, LK_RELEASE, NULL);
        return (EBUSY);
+    }
     coda_unmounting(mi->mi_vfsp);
     
     /* Wakeup clients so they can return. */
diff -r ee209edd9858 -r 2bc8bd96ae21 sys/kern/vfs_subr.c
--- a/sys/kern/vfs_subr.c       Mon Apr 16 21:36:58 2001 +0000
+++ b/sys/kern/vfs_subr.c       Mon Apr 16 22:41:09 2001 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: vfs_subr.c,v 1.148 2001/04/09 14:14:10 enami Exp $     */
+/*     $NetBSD: vfs_subr.c,v 1.149 2001/04/16 22:41:10 thorpej Exp $   */
 
 /*-
  * Copyright (c) 1997, 1998 The NetBSD Foundation, Inc.
@@ -2394,8 +2394,15 @@
                printf("unmounting %s (%s)...\n",
                    mp->mnt_stat.f_mntonname, mp->mnt_stat.f_mntfromname);
 #endif
-               if (vfs_busy(mp, 0, 0))
+               /*
+                * XXX Freeze syncer.  Must do this before locking the
+                * mount point.  See dounmount() for details.
+                */
+               lockmgr(&syncer_lock, LK_EXCLUSIVE, NULL);
+               if (vfs_busy(mp, 0, 0)) {
+                       lockmgr(&syncer_lock, LK_RELEASE, NULL);
                        continue;
+               }
                if ((error = dounmount(mp, MNT_FORCE, p)) != 0) {
                        printf("unmount of %s failed with error %d\n",
                            mp->mnt_stat.f_mntonname, error);
diff -r ee209edd9858 -r 2bc8bd96ae21 sys/kern/vfs_syscalls.c
--- a/sys/kern/vfs_syscalls.c   Mon Apr 16 21:36:58 2001 +0000
+++ b/sys/kern/vfs_syscalls.c   Mon Apr 16 22:41:09 2001 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: vfs_syscalls.c,v 1.164 2000/11/27 08:39:44 chs Exp $   */
+/*     $NetBSD: vfs_syscalls.c,v 1.165 2001/04/16 22:41:11 thorpej Exp $       */
 
 /*
  * Copyright (c) 1989, 1993
@@ -459,8 +459,16 @@
        }
        vput(vp);
 
-       if (vfs_busy(mp, 0, 0))
+       /*
+        * XXX Freeze syncer.  Must do this before locking the
+        * mount point.  See dounmount() for details.
+        */
+       lockmgr(&syncer_lock, LK_EXCLUSIVE, NULL);
+
+       if (vfs_busy(mp, 0, 0)) {
+               lockmgr(&syncer_lock, LK_RELEASE, NULL);
                return (EBUSY);
+       }
 
        return (dounmount(mp, SCARG(uap, flags), p));
 }
@@ -485,13 +493,21 @@
        used_syncer = (mp->mnt_syncer != NULL);
 
        /*
-        * XXX Freeze syncer. This should really be done on a mountpoint
-        * basis, but especially the softdep code possibly called from
-        * the syncer doesn't exactly work on a per-mountpoint basis,
-        * so the softdep code would become a maze of vfs_busy calls.
+        * XXX Syncer must be frozen when we get here.  This should really
+        * be done on a per-mountpoint basis, but especially the softdep
+        * code possibly called from the syncer doens't exactly work on a
+        * per-mountpoint basis, so the softdep code would become a maze
+        * of vfs_busy() calls.
+        *
+        * The caller of dounmount() must acquire syncer_lock because
+        * the syncer itself acquires locks in syncer_lock -> vfs_busy
+        * order, and we must preserve that order to avoid deadlock.
+        *
+        * So, if the file system did not use the syncer, now is
+        * the time to release the syncer_lock.
         */
-       if (used_syncer)
-               lockmgr(&syncer_lock, LK_EXCLUSIVE, NULL);
+       if (used_syncer == 0)
+               lockmgr(&syncer_lock, LK_RELEASE, NULL);
 
        mp->mnt_flag |= MNT_UNMOUNT;
        mp->mnt_unmounter = p;
diff -r ee209edd9858 -r 2bc8bd96ae21 sys/nfs/nfs_nqlease.c
--- a/sys/nfs/nfs_nqlease.c     Mon Apr 16 21:36:58 2001 +0000
+++ b/sys/nfs/nfs_nqlease.c     Mon Apr 16 22:41:09 2001 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: nfs_nqlease.c,v 1.37 2001/02/21 21:39:57 jdolecek Exp $        */
+/*     $NetBSD: nfs_nqlease.c,v 1.38 2001/04/16 22:41:11 thorpej Exp $ */
 
 /*
  * Copyright (c) 1992, 1993
@@ -73,6 +73,8 @@
 #include <sys/protosw.h>
 #include <sys/signalvar.h>
 
+#include <miscfs/syncfs/syncfs.h>
+
 #include <netinet/in.h>
 #include <nfs/rpcv2.h>
 #include <nfs/nfsproto.h>
@@ -1049,8 +1051,14 @@
        sleepreturn = 0;
        while ((nmp->nm_iflag & NFSMNT_DISMNT) == 0) {
            if (sleepreturn == EINTR || sleepreturn == ERESTART) {
-               if (vfs_busy(nmp->nm_mountp, LK_NOWAIT, 0) == 0 &&
-                   dounmount(nmp->nm_mountp, 0, p) != 0)
+               /*
+                * XXX Freeze syncer.  Must do this before locking
+                * the mount point.  See dounmount() for details.
+                */
+               lockmgr(&syncer_lock, LK_EXCLUSIVE, NULL);
+               if (vfs_busy(nmp->nm_mountp, LK_NOWAIT, 0) != 0)
+                       lockmgr(&syncer_lock, LK_EXCLUSIVE, NULL);
+               else if (dounmount(nmp->nm_mountp, 0, p) != 0)
                        CLRSIG(p, CURSIG(p));
                sleepreturn = 0;
                continue;
diff -r ee209edd9858 -r 2bc8bd96ae21 sys/ufs/mfs/mfs_vfsops.c
--- a/sys/ufs/mfs/mfs_vfsops.c  Mon Apr 16 21:36:58 2001 +0000
+++ b/sys/ufs/mfs/mfs_vfsops.c  Mon Apr 16 22:41:09 2001 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: mfs_vfsops.c,v 1.32 2001/02/24 00:05:22 cgd Exp $      */
+/*     $NetBSD: mfs_vfsops.c,v 1.33 2001/04/16 22:41:12 thorpej Exp $  */
 
 /*
  * Copyright (c) 1989, 1990, 1993, 1994
@@ -50,6 +50,8 @@
 #include <sys/vnode.h>
 #include <sys/malloc.h>
 
+#include <miscfs/syncfs/syncfs.h>
+
 #include <ufs/ufs/quota.h>
 #include <ufs/ufs/inode.h>
 #include <ufs/ufs/ufsmount.h>
@@ -317,8 +319,14 @@
                 * will always return EINTR/ERESTART.
                 */
                if (sleepreturn != 0) {
-                       if (vfs_busy(mp, LK_NOWAIT, 0) ||
-                           dounmount(mp, 0, p) != 0)
+                       /*
+                        * XXX Freeze syncer.  Must do this before locking
+                        * the mount point.  See dounmount() for details.
+                        */
+                       lockmgr(&syncer_lock, LK_EXCLUSIVE, NULL);
+                       if (vfs_busy(mp, LK_NOWAIT, 0) != 0)
+                               lockmgr(&syncer_lock, LK_RELEASE, NULL);
+                       else if (dounmount(mp, 0, p) != 0)
                                CLRSIG(p, CURSIG(p));
                        sleepreturn = 0;
                        continue;