From owner-freebsd-stable@FreeBSD.ORG Fri Dec 28 21:37:08 2012 Return-Path: Delivered-To: freebsd-stable@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 8163DEFD; Fri, 28 Dec 2012 21:37:08 +0000 (UTC) (envelope-from longwitz@incore.de) Received: from dss.incore.de (dss.incore.de [195.145.1.138]) by mx1.freebsd.org (Postfix) with ESMTP id 21C528FC0A; Fri, 28 Dec 2012 21:37:07 +0000 (UTC) Received: from inetmail.dmz (inetmail.dmz [10.3.0.3]) by dss.incore.de (Postfix) with ESMTP id 04DDD5C3A0; Fri, 28 Dec 2012 22:37:01 +0100 (CET) X-Virus-Scanned: amavisd-new at incore.de Received: from dss.incore.de ([10.3.0.3]) by inetmail.dmz (inetmail.dmz [10.3.0.3]) (amavisd-new, port 10024) with LMTP id s91mKSocLEi5; Fri, 28 Dec 2012 22:36:59 +0100 (CET) Received: from mail.incore (fwintern.dmz [10.0.0.253]) by dss.incore.de (Postfix) with ESMTP id C6BCD5C39D; Fri, 28 Dec 2012 22:36:59 +0100 (CET) Received: from bsdmhs.longwitz (unknown [192.168.99.6]) by mail.incore (Postfix) with ESMTP id 09FEA5083F; Fri, 28 Dec 2012 22:36:58 +0100 (CET) Message-ID: <50DE10FA.30102@incore.de> Date: Fri, 28 Dec 2012 22:36:58 +0100 From: Andreas Longwitz User-Agent: Thunderbird 2.0.0.19 (X11/20090113) MIME-Version: 1.0 To: Konstantin Belousov Subject: Re: FS hang with suspfs when creating snapshot on a UFS + GJOURNAL setup 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> In-Reply-To: <20121228112724.GR82219@kib.kiev.ua> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Cc: pho@freebsd.org, freebsd-stable@freebsd.org, fs@freebsd.org X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 28 Dec 2012 21:37:08 -0000 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