From owner-freebsd-stable@FreeBSD.ORG  Thu Nov 14 12:00:58 2013
Return-Path: <owner-freebsd-stable@FreeBSD.ORG>
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 E841F6BA;
 Thu, 14 Nov 2013 12:00:57 +0000 (UTC)
Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140])
 by mx1.freebsd.org (Postfix) with ESMTP id B6B4624F8;
 Thu, 14 Nov 2013 12:00:56 +0000 (UTC)
Received: from porto.starpoint.kiev.ua (porto-e.starpoint.kiev.ua
 [212.40.38.100])
 by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id OAA08685;
 Thu, 14 Nov 2013 14:00:55 +0200 (EET) (envelope-from avg@FreeBSD.org)
Received: from localhost ([127.0.0.1])
 by porto.starpoint.kiev.ua with esmtp (Exim 4.34 (FreeBSD))
 id 1VgvbS-000OBX-OJ; Thu, 14 Nov 2013 14:00:54 +0200
Message-ID: <5284BB3E.7090802@FreeBSD.org>
Date: Thu, 14 Nov 2013 13:59:58 +0200
From: Andriy Gapon <avg@FreeBSD.org>
User-Agent: Mozilla/5.0 (X11; FreeBSD amd64;
 rv:24.0) Gecko/20100101 Thunderbird/24.1.0
MIME-Version: 1.0
To: Steven Hartland <smh@FreeBSD.org>, hartzell@alerce.com,
 freebsd-stable@FreeBSD.org
Subject: Re: Help with filing a [maybe] ZFS/mmap bug.
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><FEE7BDCF7F494EE1BA0BE9424275AA91@multiplay.co.uk>
 <21111.12085.958991.356982@gargle.gargle.HOWL>
 <4EB902F80CE84DD2BF36C85EF4CE8EF8@multiplay.co.uk>
 <5284B8A5.8040604@FreeBSD.org>
In-Reply-To: <5284B8A5.8040604@FreeBSD.org>
X-Enigmail-Version: 1.6
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Cc: Richard Todd <rmtodd@servalan.servalan.com>
X-BeenThere: freebsd-stable@freebsd.org
X-Mailman-Version: 2.1.16
Precedence: list
List-Id: Production branch of FreeBSD source code <freebsd-stable.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/options/freebsd-stable>,
 <mailto:freebsd-stable-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-stable/>
List-Post: <mailto:freebsd-stable@freebsd.org>
List-Help: <mailto:freebsd-stable-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-stable>,
 <mailto:freebsd-stable-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Thu, 14 Nov 2013 12:00:58 -0000

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.

I should also add that the above information is consistent with the corruption
that George observed and analyzed (thanks!) -- a few bytes at the end of a page.

In fact, I am able to reproduce the bug with the following program.

#include <sys/param.h>
#include <sys/cdefs.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/mman.h>
#include <stdint.h>
#include <fcntl.h>
#include <unistd.h>
#include <stdio.h>

static const off_t len = PAGE_SIZE;
static const off_t len2 = PAGE_SIZE - DEV_BSIZE + 1;

int main (int argc, char *argv[])
{
	char dummy[len2];
        char *p;
        off_t i;
        int fd;

        if (argc < 2) {
                fprintf (stderr, "usage: %s <file>\n", argv[0]);
                return (1);
        }

        fd = open(argv[1], O_CREAT | O_EXCL | O_RDWR, 0660);
        if (fd == -1) {
                perror ("open");
                return (1);
        }

        if (ftruncate(fd, len) == -1) {
                perror ("ftruncate");
                return (1);
        }

        p = mmap(0, len, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
        if (p == MAP_FAILED) {
                perror ("mmap");
                return (1);
        }

        for (i = 0; i < len; i++)
                p[i] = '0';

	if (msync(p, len, MS_SYNC) != 0) {
                perror ("msync");
                return (1);
	}

	printf("file filled with 0s and synced\n");

        for (i = 0; i < len; i++)
                p[i] = '1';

	printf("file filled with 1s\n");

        for (i = 0; i < len2; i++)
		dummy[i] = 'x';

	if (write(fd, dummy, len2) != len2) {
		perror ("write");
		return (1);
	}

	printf("first %ju bytes are overwritten with 'x'\n",
	    (uintmax_t)len2);

        if (munmap(p, len) == -1) {
                perror ("munmap");
                return (1);
        }

        if (close(fd) == -1) {
                perror ("close");
                return (1);
        }

	printf("file is unmapped and closed\n");
	printf("please unmount and remount filesystem and check file content\n");

        return (0);
}


-- 
Andriy Gapon