Author: kib
Date: Tue Jul  9 20:49:32 2013
New Revision: 253106
URL: http://svnweb.freebsd.org/changeset/base/253106

Log:
  There are several code sequences like
        vfs_busy(mp);
        vfs_write_suspend(mp);
  which are problematic if other thread starts unmount between two
  calls.  The unmount starts a write, while vfs_write_suspend() drain
  writers.  On the other hand, unmount drains busy references, causing
  the deadlock.
  
  Add a flag argument to vfs_write_suspend and require the callers of it
  to specify VS_SKIP_UNMOUNT flag, when the call is performed not in the
  mount path, i.e. the covered vnode is not locked.  The suspension is
  not attempted if VS_SKIP_UNMOUNT is specified and unmount is in
  progress.
  
  Reported and tested by:       Andreas Longwitz <longw...@incore.de>
  Sponsored by: The FreeBSD Foundation
  MFC after:    3 weeks

Modified:
  head/sys/geom/journal/g_journal.c
  head/sys/kern/vfs_vnops.c
  head/sys/sys/vnode.h
  head/sys/ufs/ffs/ffs_snapshot.c
  head/sys/ufs/ffs/ffs_suspend.c
  head/sys/ufs/ffs/ffs_vfsops.c

Modified: head/sys/geom/journal/g_journal.c
==============================================================================
--- head/sys/geom/journal/g_journal.c   Tue Jul  9 19:12:47 2013        
(r253105)
+++ head/sys/geom/journal/g_journal.c   Tue Jul  9 20:49:32 2013        
(r253106)
@@ -2960,7 +2960,7 @@ g_journal_do_switch(struct g_class *clas
                GJ_TIMER_STOP(1, &bt, "BIO_FLUSH time of %s", sc->sc_name);
 
                GJ_TIMER_START(1, &bt);
-               error = vfs_write_suspend(mp);
+               error = vfs_write_suspend(mp, VS_SKIP_UNMOUNT);
                GJ_TIMER_STOP(1, &bt, "Suspend time of %s", mountpoint);
                if (error != 0) {
                        GJ_DEBUG(0, "Cannot suspend file system %s (error=%d).",

Modified: head/sys/kern/vfs_vnops.c
==============================================================================
--- head/sys/kern/vfs_vnops.c   Tue Jul  9 19:12:47 2013        (r253105)
+++ head/sys/kern/vfs_vnops.c   Tue Jul  9 20:49:32 2013        (r253106)
@@ -1668,8 +1668,7 @@ vn_finished_secondary_write(mp)
  * Request a filesystem to suspend write operations.
  */
 int
-vfs_write_suspend(mp)
-       struct mount *mp;
+vfs_write_suspend(struct mount *mp, int flags)
 {
        int error;
 
@@ -1680,6 +1679,21 @@ vfs_write_suspend(mp)
        }
        while (mp->mnt_kern_flag & MNTK_SUSPEND)
                msleep(&mp->mnt_flag, MNT_MTX(mp), PUSER - 1, "wsuspfs", 0);
+
+       /*
+        * Unmount holds a write reference on the mount point.  If we
+        * own busy reference and drain for writers, we deadlock with
+        * the reference draining in the unmount path.  Callers of
+        * vfs_write_suspend() must specify VS_SKIP_UNMOUNT if
+        * vfs_busy() reference is owned and caller is not in the
+        * unmount context.
+        */
+       if ((flags & VS_SKIP_UNMOUNT) != 0 &&
+           (mp->mnt_kern_flag & MNTK_UNMOUNT) != 0) {
+               MNT_IUNLOCK(mp);
+               return (EBUSY);
+       }
+
        mp->mnt_kern_flag |= MNTK_SUSPEND;
        mp->mnt_susp_owner = curthread;
        if (mp->mnt_writeopcount > 0)

Modified: head/sys/sys/vnode.h
==============================================================================
--- head/sys/sys/vnode.h        Tue Jul  9 19:12:47 2013        (r253105)
+++ head/sys/sys/vnode.h        Tue Jul  9 20:49:32 2013        (r253106)
@@ -398,6 +398,9 @@ extern int          vttoif_tab[];
 #define        VR_START_WRITE  0x0001  /* vfs_write_resume: start write 
atomically */
 #define        VR_NO_SUSPCLR   0x0002  /* vfs_write_resume: do not clear 
suspension */
 
+#define        VS_SKIP_UNMOUNT 0x0001  /* vfs_write_suspend: fail if the
+                                  filesystem is being unmounted */
+
 #define        VREF(vp)        vref(vp)
 
 #ifdef DIAGNOSTIC
@@ -711,7 +714,7 @@ int vn_io_fault_pgmove(vm_page_t ma[], v
 int    vfs_cache_lookup(struct vop_lookup_args *ap);
 void   vfs_timestamp(struct timespec *);
 void   vfs_write_resume(struct mount *mp, int flags);
-int    vfs_write_suspend(struct mount *mp);
+int    vfs_write_suspend(struct mount *mp, int flags);
 int    vop_stdbmap(struct vop_bmap_args *);
 int    vop_stdfsync(struct vop_fsync_args *);
 int    vop_stdgetwritemount(struct vop_getwritemount_args *);

Modified: head/sys/ufs/ffs/ffs_snapshot.c
==============================================================================
--- head/sys/ufs/ffs/ffs_snapshot.c     Tue Jul  9 19:12:47 2013        
(r253105)
+++ head/sys/ufs/ffs/ffs_snapshot.c     Tue Jul  9 20:49:32 2013        
(r253106)
@@ -423,7 +423,7 @@ restart:
         */
        for (;;) {
                vn_finished_write(wrtmp);
-               if ((error = vfs_write_suspend(vp->v_mount)) != 0) {
+               if ((error = vfs_write_suspend(vp->v_mount, 0)) != 0) {
                        vn_start_write(NULL, &wrtmp, V_WAIT);
                        vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
                        goto out;

Modified: head/sys/ufs/ffs/ffs_suspend.c
==============================================================================
--- head/sys/ufs/ffs/ffs_suspend.c      Tue Jul  9 19:12:47 2013        
(r253105)
+++ head/sys/ufs/ffs/ffs_suspend.c      Tue Jul  9 20:49:32 2013        
(r253106)
@@ -206,7 +206,7 @@ ffs_susp_suspend(struct mount *mp)
                return (EPERM);
 #endif
 
-       if ((error = vfs_write_suspend(mp)) != 0)
+       if ((error = vfs_write_suspend(mp, VS_SKIP_UNMOUNT)) != 0)
                return (error);
 
        ump->um_writesuspended = 1;

Modified: head/sys/ufs/ffs/ffs_vfsops.c
==============================================================================
--- head/sys/ufs/ffs/ffs_vfsops.c       Tue Jul  9 19:12:47 2013        
(r253105)
+++ head/sys/ufs/ffs/ffs_vfsops.c       Tue Jul  9 20:49:32 2013        
(r253106)
@@ -257,7 +257,7 @@ ffs_mount(struct mount *mp)
                                return (error);
                        for (;;) {
                                vn_finished_write(mp);
-                               if ((error = vfs_write_suspend(mp)) != 0)
+                               if ((error = vfs_write_suspend(mp, 0)) != 0)
                                        return (error);
                                MNT_ILOCK(mp);
                                if (mp->mnt_kern_flag & MNTK_SUSPENDED) {
@@ -1255,7 +1255,7 @@ ffs_unmount(mp, mntflags)
                 */
                for (;;) {
                        vn_finished_write(mp);
-                       if ((error = vfs_write_suspend(mp)) != 0)
+                       if ((error = vfs_write_suspend(mp, 0)) != 0)
                                return (error);
                        MNT_ILOCK(mp);
                        if (mp->mnt_kern_flag & MNTK_SUSPENDED) {
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to