From owner-freebsd-stable@FreeBSD.ORG Sun Nov 17 15:54:15 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 377A815C; Sun, 17 Nov 2013 15:54:15 +0000 (UTC) Received: from mail1.multiplay.co.uk (mail1.multiplay.co.uk [85.236.96.23]) (using TLSv1 with cipher RC4-MD5 (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id A69C12478; Sun, 17 Nov 2013 15:54:14 +0000 (UTC) Received: from r2d2 ([82.69.141.170]) by mail1.multiplay.co.uk (mail1.multiplay.co.uk [85.236.96.23]) (MDaemon PRO v10.0.4) with ESMTP id md50006747364.msg; Sun, 17 Nov 2013 15:54:06 +0000 X-Spam-Processed: mail1.multiplay.co.uk, Sun, 17 Nov 2013 15:54:06 +0000 (not processed: message from valid local sender) X-MDDKIM-Result: neutral (mail1.multiplay.co.uk) X-MDRemoteIP: 82.69.141.170 X-Return-Path: prvs=10337ef692=killing@multiplay.co.uk X-Envelope-From: killing@multiplay.co.uk Message-ID: <9A58D6B0691A4C1294883AD91F9D4743@multiplay.co.uk> From: "Steven Hartland" To: "Andriy Gapon" , , 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> Subject: Re: Help with filing a [maybe] ZFS/mmap bug. Date: Sun, 17 Nov 2013 15:53:58 -0000 MIME-Version: 1.0 Content-Type: text/plain; format=flowed; charset="iso-8859-1"; reply-type=original Content-Transfer-Encoding: 7bit X-Priority: 3 X-MSMail-Priority: Normal X-Mailer: Microsoft Outlook Express 6.00.2900.5931 X-MimeOLE: Produced By Microsoft MimeOLE V6.00.2900.6157 X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.16 Precedence: list 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 15:54:15 -0000 ----- Original Message ----- From: "Andriy Gapon" To: "Steven Hartland" ; ; Sent: Sunday, November 17, 2013 9:48 AM Subject: Re: Help with filing a [maybe] ZFS/mmap bug. > 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; Thanks Andriy, looking to test this here but 8.3 doesn't have a page_busy method in zfs_vnops.c so I'm not sure how to proceed on this? Regards Steve ================================================ This e.mail is private and confidential between Multiplay (UK) Ltd. and the person or entity to whom it is addressed. In the event of misdirection, the recipient is prohibited from using, copying, printing or otherwise disseminating it or any information contained in it. In the event of misdirection, illegible or incomplete transmission please telephone +44 845 868 1337 or return the E.mail to postmaster@multiplay.co.uk.