Date: Tue, 9 Jul 2013 20:49:32 +0000 (UTC) From: Konstantin Belousov <kib@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r253106 - in head/sys: geom/journal kern sys ufs/ffs Message-ID: <201307092049.r69KnWuA034821@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
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 <longwitz@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) {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201307092049.r69KnWuA034821>