Date: Fri, 28 Dec 2012 22:36:58 +0100 From: Andreas Longwitz <longwitz@incore.de> To: Konstantin Belousov <kostikbel@gmail.com> Cc: pho@freebsd.org, freebsd-stable@freebsd.org, fs@freebsd.org Subject: Re: FS hang with suspfs when creating snapshot on a UFS + GJOURNAL setup Message-ID: <50DE10FA.30102@incore.de> In-Reply-To: <20121228112724.GR82219@kib.kiev.ua> References: <50DC30F6.1050904@incore.de> <20121227133355.GI82219@kib.kiev.ua> <50DC8999.8000708@incore.de> <20121227194145.GM82219@kib.kiev.ua> <50DD6423.5090305@incore.de> <20121228112724.GR82219@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
Konstantin Belousov wrote:
>
> Please try the following patch. It is against HEAD, might need some
> adjustments for 8. I do the resume and write accounting atomically,
> not allowing other suspension to intervent between.
>
> diff --git a/sys/kern/vfs_vnops.c b/sys/kern/vfs_vnops.c
> index 3f65b05..cf49ecb 100644
> --- a/sys/kern/vfs_vnops.c
> +++ b/sys/kern/vfs_vnops.c
> @@ -1434,6 +1434,40 @@ vn_closefile(fp, td)
> * proceed. If a suspend request is in progress, we wait until the
> * suspension is over, and then proceed.
> */
> +static int
> +vn_start_write_locked(struct mount *mp, int flags)
> +{
> + int error;
> +
> + mtx_assert(MNT_MTX(mp), MA_OWNED);
> + error = 0;
> +
> + /*
> + * Check on status of suspension.
> + */
> + if ((curthread->td_pflags & TDP_IGNSUSP) == 0 ||
> + mp->mnt_susp_owner != curthread) {
> + while ((mp->mnt_kern_flag & MNTK_SUSPEND) != 0) {
> + if (flags & V_NOWAIT) {
> + error = EWOULDBLOCK;
> + goto unlock;
> + }
> + error = msleep(&mp->mnt_flag, MNT_MTX(mp),
> + (PUSER - 1) | (flags & PCATCH), "suspfs", 0);
> + if (error)
> + goto unlock;
> + }
> + }
> + if (flags & V_XSLEEP)
> + goto unlock;
> + mp->mnt_writeopcount++;
> +unlock:
> + if (error != 0 || (flags & V_XSLEEP) != 0)
> + MNT_REL(mp);
> + MNT_IUNLOCK(mp);
> + return (error);
> +}
> +
> int
> vn_start_write(vp, mpp, flags)
> struct vnode *vp;
> @@ -1470,30 +1504,7 @@ vn_start_write(vp, mpp, flags)
> if (vp == NULL)
> MNT_REF(mp);
>
> - /*
> - * Check on status of suspension.
> - */
> - if ((curthread->td_pflags & TDP_IGNSUSP) == 0 ||
> - mp->mnt_susp_owner != curthread) {
> - while ((mp->mnt_kern_flag & MNTK_SUSPEND) != 0) {
> - if (flags & V_NOWAIT) {
> - error = EWOULDBLOCK;
> - goto unlock;
> - }
> - error = msleep(&mp->mnt_flag, MNT_MTX(mp),
> - (PUSER - 1) | (flags & PCATCH), "suspfs", 0);
> - if (error)
> - goto unlock;
> - }
> - }
> - if (flags & V_XSLEEP)
> - goto unlock;
> - mp->mnt_writeopcount++;
> -unlock:
> - if (error != 0 || (flags & V_XSLEEP) != 0)
> - MNT_REL(mp);
> - MNT_IUNLOCK(mp);
> - return (error);
> + return (vn_start_write_locked(mp, flags));
> }
>
> /*
> @@ -1639,8 +1650,7 @@ vfs_write_suspend(mp)
> * Request a filesystem to resume write operations.
> */
> void
> -vfs_write_resume(mp)
> - struct mount *mp;
> +vfs_write_resume_flags(struct mount *mp, int flags)
> {
>
> MNT_ILOCK(mp);
> @@ -1652,10 +1662,25 @@ vfs_write_resume(mp)
> wakeup(&mp->mnt_writeopcount);
> wakeup(&mp->mnt_flag);
> curthread->td_pflags &= ~TDP_IGNSUSP;
> + if ((flags & VR_START_WRITE) != 0) {
> + MNT_REF(mp);
> + mp->mnt_writeopcount++;
> + }
> MNT_IUNLOCK(mp);
> VFS_SUSP_CLEAN(mp);
> - } else
> + } else if ((flags & VR_START_WRITE) != 0) {
> + MNT_REF(mp);
> + vn_start_write_locked(mp, 0);
> + } else {
> MNT_IUNLOCK(mp);
> + }
> +}
> +
> +void
> +vfs_write_resume(struct mount *mp)
> +{
> +
> + vfs_write_resume_flags(mp, 0);
> }
>
> /*
> diff --git a/sys/sys/vnode.h b/sys/sys/vnode.h
> index 42f9e5f..4371b40 100644
> --- a/sys/sys/vnode.h
> +++ b/sys/sys/vnode.h
> @@ -392,6 +392,8 @@ extern int vttoif_tab[];
> #define V_NOWAIT 0x0002 /* vn_start_write: don't sleep for suspend */
> #define V_XSLEEP 0x0004 /* vn_start_write: just return after sleep */
>
> +#define VR_START_WRITE 0x0001 /* vfs_write_resume: start write atomically */
> +
> #define VREF(vp) vref(vp)
>
> #ifdef DIAGNOSTIC
> @@ -701,6 +703,7 @@ int vn_io_fault_uiomove(char *data, int xfersize, struct uio *uio);
> int vfs_cache_lookup(struct vop_lookup_args *ap);
> void vfs_timestamp(struct timespec *);
> void vfs_write_resume(struct mount *mp);
> +void vfs_write_resume_flags(struct mount *mp, int flags);
> int vfs_write_suspend(struct mount *mp);
> int vop_stdbmap(struct vop_bmap_args *);
> int vop_stdfsync(struct vop_fsync_args *);
> diff --git a/sys/ufs/ffs/ffs_snapshot.c b/sys/ufs/ffs/ffs_snapshot.c
> index e528509..25ad79c 100644
> --- a/sys/ufs/ffs/ffs_snapshot.c
> +++ b/sys/ufs/ffs/ffs_snapshot.c
> @@ -687,8 +687,7 @@ out1:
> /*
> * Resume operation on filesystem.
> */
> - vfs_write_resume(vp->v_mount);
> - vn_start_write(NULL, &wrtmp, V_WAIT);
> + vfs_write_resume_flags(vp->v_mount, VR_START_WRITE);
> if (collectsnapstats && starttime.tv_sec > 0) {
> nanotime(&endtime);
> timespecsub(&endtime, &starttime);
Your patch adjusted to FreeBSD Stable 8 works fine. Creating a snapshot
every minute runs without errors for more than 6 hours now, without the
patch my test machine hangs not later than one hour.
--
Andreas Longwitz
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?50DE10FA.30102>