From: Ross Zwisler <ross.zwisler@linux.intel.com>
To: Jan Kara <jack@suse.cz>
Cc: linux-nvdimm@lists.01.org, Ted Tso <tytso@mit.edu>,
linux-ext4@vger.kernel.org,
Vishal Verma <vishal.l.verma@intel.com>,
Ross Zwisler <ross.zwisler@linux.intel.com>,
Dan Williams <dan.j.williams@intel.com>,
linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 3/7] ext2: Avoid DAX zeroing to corrupt data
Date: Thu, 12 May 2016 12:45:22 -0600 [thread overview]
Message-ID: <20160512184522.GA19851@linux.intel.com> (raw)
In-Reply-To: <1462960733-29634-4-git-send-email-jack@suse.cz>
On Wed, May 11, 2016 at 11:58:49AM +0200, Jan Kara wrote:
> Currently ext2 zeroes any data blocks allocated for DAX inode however it
> still returns them as BH_New. Thus DAX code zeroes them again in
> dax_insert_mapping() which can possibly overwrite the data that has been
> already stored to those blocks by a racing dax_io(). Avoid marking
> pre-zeroed buffers as new.
>
> Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
> fs/ext2/inode.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
> index 6bd58e6ff038..1f07b758b968 100644
> --- a/fs/ext2/inode.c
> +++ b/fs/ext2/inode.c
> @@ -745,11 +745,11 @@ static int ext2_get_blocks(struct inode *inode,
> mutex_unlock(&ei->truncate_mutex);
> goto cleanup;
> }
> - }
> + } else
> + set_buffer_new(bh_result);
>
> ext2_splice_branch(inode, iblock, partial, indirect_blks, count);
> mutex_unlock(&ei->truncate_mutex);
> - set_buffer_new(bh_result);
> got_it:
> map_bh(bh_result, inode->i_sb, le32_to_cpu(chain[depth-1].key));
> if (count > blocks_to_boundary)
> --
> 2.6.6
Interestingly this change is causing a bunch of xfstests regressions for me
with ext2 + DAX. All of these tests pass without this one change.
# ./check generic/075 generic/091 generic/112 generic/127 generic/231 generic/263
FSTYP -- ext2
PLATFORM -- Linux/x86_64 alara 4.6.0-rc5+
MKFS_OPTIONS -- /dev/pmem0p2
MOUNT_OPTIONS -- -o dax -o context=system_u:object_r:nfs_t:s0 /dev/pmem0p2 /mnt/xfstests_scratch
generic/075 2s ... [failed, exit status 1] - output mismatch (see /root/xfstests/results//generic/075.out.bad)
--- tests/generic/075.out 2015-10-02 10:19:36.798795839 -0600
+++ /root/xfstests/results//generic/075.out.bad 2016-05-12 12:40:06.558706982 -0600
@@ -4,15 +4,5 @@
-----------------------------------------------
fsx.0 : -d -N numops -S 0
-----------------------------------------------
-
------------------------------------------------
-fsx.1 : -d -N numops -S 0 -x
------------------------------------------------
...
(Run 'diff -u tests/generic/075.out /root/xfstests/results//generic/075.out.bad' to see the entire diff)
generic/091 1s ... [failed, exit status 1] - output mismatch (see /root/xfstests/results//generic/091.out.bad)
--- tests/generic/091.out 2015-10-02 10:19:36.800795853 -0600
+++ /root/xfstests/results//generic/091.out.bad 2016-05-12 12:40:07.366712217 -0600
@@ -1,7 +1,110 @@
QA output created by 091
fsx -N 10000 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z -R -W
-fsx -N 10000 -o 8192 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z -R -W
-fsx -N 10000 -o 32768 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z -R -W
-fsx -N 10000 -o 8192 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z -R -W
-fsx -N 10000 -o 32768 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z -R -W
-fsx -N 10000 -o 128000 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z -W
...
(Run 'diff -u tests/generic/091.out /root/xfstests/results//generic/091.out.bad' to see the entire diff)
generic/112 2s ... [failed, exit status 1] - output mismatch (see /root/xfstests/results//generic/112.out.bad)
--- tests/generic/112.out 2015-10-02 10:19:36.802795866 -0600
+++ /root/xfstests/results//generic/112.out.bad 2016-05-12 12:40:08.601720218 -0600
@@ -4,15 +4,5 @@
-----------------------------------------------
fsx.0 : -A -d -N numops -S 0
-----------------------------------------------
-
------------------------------------------------
-fsx.1 : -A -d -N numops -S 0 -x
------------------------------------------------
...
(Run 'diff -u tests/generic/112.out /root/xfstests/results//generic/112.out.bad' to see the entire diff)
generic/127 14s ... - output mismatch (see /root/xfstests/results//generic/127.out.bad)
--- tests/generic/127.out 2016-02-17 16:02:40.110656181 -0700
+++ /root/xfstests/results//generic/127.out.bad 2016-05-12 12:40:16.861773730 -0600
@@ -4,10 +4,905 @@
=== FSX Light Mode, Memory Mapping ===
All 100000 operations completed A-OK!
=== FSX Standard Mode, No Memory Mapping ===
-All 100000 operations completed A-OK!
+ltp/fsx -q -l 262144 -o 65536 -S 191110531 -N 100000 -R -W fsx_std_nommap
+READ BAD DATA: offset = 0xd72d, size = 0x826b, fname = /mnt/xfstests_test/fsx_std_nommap
+OFFSET GOOD BAD RANGE
...
(Run 'diff -u tests/generic/127.out /root/xfstests/results//generic/127.out.bad' to see the entire diff)
generic/231 22s ... [failed, exit status 1] - output mismatch (see /root/xfstests/results//generic/231.out.bad)
--- tests/generic/231.out 2016-02-17 16:02:40.120656239 -0700
+++ /root/xfstests/results//generic/231.out.bad 2016-05-12 12:40:19.266789311 -0600
@@ -1,16 +1,1802 @@
QA output created by 231
=== FSX Standard Mode, Memory Mapping, 1 Tasks ===
-All 20000 operations completed A-OK!
-Comparing user usage
-Comparing group usage
-=== FSX Standard Mode, Memory Mapping, 4 Tasks ===
-All 20000 operations completed A-OK!
...
(Run 'diff -u tests/generic/231.out /root/xfstests/results//generic/231.out.bad' to see the entire diff)
generic/263 1s ... [failed, exit status 1] - output mismatch (see /root/xfstests/results//generic/263.out.bad)
--- tests/generic/263.out 2015-10-02 10:19:36.807795900 -0600
+++ /root/xfstests/results//generic/263.out.bad 2016-05-12 12:40:19.801792777 -0600
@@ -1,3 +1,1429 @@
QA output created by 263
fsx -N 10000 -o 8192 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z
-fsx -N 10000 -o 128000 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z
+main: filesystem does not support fallocate mode 0, disabling!
+main: filesystem does not support fallocate mode FALLOC_FL_KEEP_SIZE, disabling!
+main: filesystem does not support fallocate mode FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, disabling!
+main: filesystem does not support fallocate mode FALLOC_FL_ZERO_RANGE, disabling!
...
(Run 'diff -u tests/generic/263.out /root/xfstests/results//generic/263.out.bad' to see the entire diff)
Ran: generic/075 generic/091 generic/112 generic/127 generic/231 generic/263
Failures: generic/075 generic/091 generic/112 generic/127 generic/231 generic/263
Failed 6 of 6 tests
next prev parent reply other threads:[~2016-05-12 18:45 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-11 9:58 [PATCH 0/7 v4] DAX cleanups and fixes Jan Kara
2016-05-11 9:58 ` [PATCH 1/7] DAX: move RADIX_DAX_ definitions to dax.c Jan Kara
2016-05-11 9:58 ` [PATCH 2/7] dax: Remove complete_unwritten argument Jan Kara
2016-05-11 9:58 ` [PATCH 3/7] ext2: Avoid DAX zeroing to corrupt data Jan Kara
2016-05-12 18:45 ` Ross Zwisler [this message]
2016-05-16 15:22 ` Jan Kara
2016-05-17 6:52 ` Verma, Vishal L
2016-05-17 7:19 ` Jan Kara
2016-05-17 17:56 ` Verma, Vishal L
2016-05-11 9:58 ` [PATCH 4/7] dax: Remove dead zeroing code from fault handlers Jan Kara
2016-05-11 9:58 ` [PATCH 5/7] dax: Remove zeroing from dax_io() Jan Kara
2016-05-11 9:58 ` [PATCH 6/7] dax: Remove pointless writeback from dax_do_io() Jan Kara
2016-05-11 9:58 ` [PATCH 7/7] dax: Remove redundant inode size checks Jan Kara
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160512184522.GA19851@linux.intel.com \
--to=ross.zwisler@linux.intel.com \
--cc=dan.j.williams@intel.com \
--cc=jack@suse.cz \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-nvdimm@lists.01.org \
--cc=tytso@mit.edu \
--cc=vishal.l.verma@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox