From owner-freebsd-stable@FreeBSD.ORG Sun Nov 17 20:58:35 2013 Return-Path: Delivered-To: freebsd-stable@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 973088BB; Sun, 17 Nov 2013 20:58:35 +0000 (UTC) Received: from griffon.alerce.com (griffon.alerce.com [206.125.171.162]) (using TLSv1 with cipher ADH-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 71CB022D4; Sun, 17 Nov 2013 20:58:35 +0000 (UTC) Received: from griffon.alerce.com (localhost [127.0.0.1]) by griffon.alerce.com (Postfix) with ESMTP id 8947028435; Sun, 17 Nov 2013 12:58:34 -0800 (PST) Received: from alacrity.alerce.com (75-149-38-78-SFBA.hfc.comcastbusiness.net [75.149.38.78]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by griffon.alerce.com (Postfix) with ESMTPSA id 546B128434; Sun, 17 Nov 2013 12:58:34 -0800 (PST) Received: by alacrity.alerce.com (Postfix, from userid 503) id 5ED561885199; Sun, 17 Nov 2013 12:58:33 -0800 (PST) From: George Hartzell MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <21129.11769.292612.582677@gargle.gargle.HOWL> Date: Sun, 17 Nov 2013 12:58:33 -0800 To: Andriy Gapon Subject: Re: Help with filing a [maybe] ZFS/mmap bug. In-Reply-To: <52889105.7040404@FreeBSD.org> References: <20967.760.95825.310085@gargle.gargle.HOWL> <51E80B30.1090004@FreeBSD.org> <20968.10645.880772.30501@gargle.gargle.HOWL> <520202E5.30300@FreeBSD.org> <20994.55913.93606.436124@gargle.gargle.HOWL> <21111.12085.958991.356982@gargle.gargle.HOWL> <4EB902F80CE84DD2BF36C85EF4CE8EF8@multiplay.co.uk> <5284B8A5.8040604@FreeBSD.org> <52889105.7040404@FreeBSD.org> X-Mailer: VM 8.2.0b under 24.3.1 (x86_64-apple-darwin12.5.0) X-Virus-Scanned: ClamAV using ClamSMTP Cc: freebsd-stable@FreeBSD.org, Steven Hartland , hartzell@alerce.com X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.16 Precedence: list Reply-To: hartzell@alerce.com List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 17 Nov 2013 20:58:35 -0000 Andriy Gapon writes: > on 14/11/2013 13:48 Andriy Gapon said the following: > > HOWEVER. I think that there is a bug that I introduced in r246293. > > Specifically I changed > > vm_page_undirty(pp); > > to > > pmap_remove_write(pp); > > vm_page_clear_dirty(pp, off, nbytes); > > > > vm_page_undirty() would be a very serious (and probably obvious) bug, if it were > > not a NOP in effect. The details are explained in the commit message. > > But when I used vm_page_clear_dirty I completely missed the fact that *extends* > > the range to DEV_BSIZE aligned boundaries[*]. So, given the described behavior > > and that pmap_remove_write clears the page modified bit, it is possible that the > > data dirty data in the extended areas will be marked as clean. > > Here is a patch (for head) that should fix the described above issue: > > diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c > b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c > index 2e2cbd6..4fcd571 100644 > --- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c > +++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c > @@ -328,6 +328,20 @@ page_busy(vnode_t *vp, int64_t start, int64_t off, int64_t > nbytes) > { > vm_object_t obj; > vm_page_t pp; > + int64_t end; > + > + /* > + * At present vm_page_clear_dirty extends the cleared range to DEV_BSIZE > + * aligned boundaries, if the range is not aligned. As a result a > + * DEV_BSIZE subrange with partially dirty data may get marked as clean. > + * It may happen that all DEV_BSIZE subranges are marked clean and thus > + * the whole page would be considred clean despite have some dirty data. > + * For this reason we should shrink the range to DEV_BSIZE aligned > + * boundaries before calling vm_page_clear_dirty. > + */ > + end = rounddown2(off + nbytes, DEV_BSIZE); > + off = roundup2(off, DEV_BSIZE); > + nbytes = end - off; > > obj = vp->v_object; > zfs_vmobject_assert_wlocked(obj); > @@ -362,7 +376,8 @@ page_busy(vnode_t *vp, int64_t start, int64_t off, int64_t > nbytes) > ASSERT3U(pp->valid, ==, VM_PAGE_BITS_ALL); > vm_object_pip_add(obj, 1); > pmap_remove_write(pp); > - vm_page_clear_dirty(pp, off, nbytes); > + if (nbytes != 0) > + vm_page_clear_dirty(pp, off, nbytes); > } > break; > } > > -- > Andriy Gapon This fixes my "test case" (un-automated though it may be). I've successfully run twice through using Picard (which uses Mutagen, which uses mmap) on my 10-ALPHA-2 to tag and transcode a set of tracks that used to consistently but nondeterministically result in failures. It's a bit of a negative result, all that I can see is that I seem to no longer see the problem. But I'm happier than I was and your logic seems to match the reality I was experiencing. Thanks! g.