Skip to contentSkip to conversation

xfstest: using extsize cause corruption with multi buffer page

PATCH In review Alain Renaud 3 participants 6 messages Jun 1, 2012 – Jul 13, 2012

lore
Share & tools
lore
#0 lore raw

cover xfstest: using extsize cause corruption with multi buffer page

Hi all,

I just found a problem which cause file corruption with filesystem that
have block size smaller then the page size. The problem occur when using extsize on file and having multiple extents in the same PAGE.

The scenario that I am looking at is a page that look like this:

buffer  content
0       empty  b_state = 0
1       DATA   b_state = 0x1023
2       DATA   b_state = 0x1023
3       empty  b_state = 0
4       empty  b_state = 0
5       DATA   b_state = 0x1023
6       DATA   b_state = 0x1023
7       empty  b_state = 0

At the end the extent should look like this

0 : unwritten extent.
1-2 : real extent
3-4 : unwritten extent.
5-6 : real extent.
7-EOF : unwritten extent.

However the current version of xfs_vm_writepage() will put all the buffer in the page into 1 ioend which cause the extent list to look like this.

0 : unwritten extent.
1-4 : real extent
5-EOF : unwritten extent.

The test generate a 'template' file which is created without using extsize look like this.

# xfs_bmap -vp /xfsqa_scratch/testdir/template 
/xfsqa_scratch/testdir/template:
 EXT: FILE-OFFSET      BLOCK-RANGE      AG AG-OFFSET        TOTAL FLAGS
   0: [0..8]:          hole                                     9
   1: [9..23]:         48..62            0 (48..62)            15 00000
# hexdump  /xfsqa_scratch/testdir/template
0000000 0000 0000 0000 0000 0000 0000 0000 0000
*
0001200 4242 4242 4242 4242 4242 4242 4242 4242
*
0001600 0000 0000 0000 0000 0000 0000 0000 0000
*
0001a00 4343 4343 4343 4343 4343 4343 4343 4343
*
0001e00 0000 0000 0000 0000 0000 0000 0000 0000
*
0003000

The test file which is created with 'extsize == 10 pages' give us this result.
(notice block 13-12 are unwritten but should contain the 'C')

# xfs_bmap -vp /xfsqa_scratch/testdir/zfile-00 
/xfsqa_scratch/testdir/zfile-00:
 EXT: FILE-OFFSET      BLOCK-RANGE      AG AG-OFFSET        TOTAL FLAGS
   0: [0..8]:          167..175          0 (167..175)           9 10000
   1: [9..12]:         176..179          0 (176..179)           4 00000
   2: [13..23]:        180..190          0 (180..190)          11 10000
# hexdump  /xfsqa_scratch/testdir/zfile-00 
0000000 0000 0000 0000 0000 0000 0000 0000 0000
*
0001200 4242 4242 4242 4242 4242 4242 4242 4242
*
0001600 0691 3c24 d01f 3de9 0597 3e65 b312 3ebf
*
0003000

Looking at the xfs_db output I know that the data get correctly written to disk but the coversion from unwritten to real seem broken. I have Isolated the problem to the fact that all buffer in the page are put into a single xfs_ioend instead of 2. I will propose a fix in a separate patch. This test is only to show the problem

Signature · 9 lines
===============================================
Alain Renaud     - Cluster File System Engineer 
[email protected]  - SGI
===============================================

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#1 lore raw

PATCH 1/1 xfstest: using extsize cause corruption with multi buffer page

Test using extsize to create a file with multiple extent in one
PAGE. This show an error in the block conversion from unwritten to real. As a result we tag raw disk block as valid(3-4) and valid data as unwritten(5-6)

On an x86_64 machine the page should look like this.

buffer  content
0       empty  b_state = 0
1       DATA   b_state = 0x1023
2       DATA   b_state = 0x1023
3       empty  b_state = 0
4       empty  b_state = 0
5       DATA   b_state = 0x1023
6       DATA   b_state = 0x1023
7       empty  b_state = 0

---

 .gitignore         |    1 	1 +	0 -	0 !
 287                |   74 	74 +	0 -	0 !
 287.out            |    1 	1 +	0 -	0 !
 group              |    1 	1 +	0 -	0 !
 src/Makefile       |    2 	1 +	1 -	0 !
 src/extsize_page.c |  113 	113 +	0 -	0 !
 6 files changed, 191 insertions(+), 1 deletion(-)
 create mode 100755 287
 create mode 100644 287.out
 create mode 100644 src/extsize_page.c
.gitignore +1 -0
Index: b/.gitignore
===================================================================
@@ -30,6 +30,7 @@
 src/dirperf
 src/dirstress
 src/dmiperf
+src/extsize_page
 src/fault
 src/feature
 src/fiemap-tester
287 new file +74 -0

74 lines not on this page · raw message

287.out new file +1 -0
Index: b/287.out
===================================================================
@@ -0,0 +1 @@
+QA output created by 287
group +1 -0
Index: b/group
===================================================================
@@ -405,3 +405,4 @@
 284 auto
 285 auto rw
 286 other
+287 auto rw
src/Makefile +1 -1
Index: b/src/Makefile
===================================================================
@@ -18,7 +18,7 @@
 	locktest unwritten_mmap bulkstat_unlink_test t_stripealign \
 	bulkstat_unlink_test_modified t_dir_offset t_futimens t_immutable \
 	stale_handle pwrite_mmap_blocked fstrim t_dir_offset2 seek_sanity_test \
-	seek_copy_test
+	seek_copy_test extsize_page
 
 SUBDIRS =
 
src/extsize_page.c new file +113 -0

113 lines not on this page · raw message

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#2 lore raw
Quoted 20 lines · Alain Renaud

Test using extsize to create a file with multiple extent in one
PAGE. This show an error in the block conversion from unwritten to real. As a result we tag raw disk block as valid(3-4) and valid data as unwritten(5-6)

On an x86_64 machine the page should look like this.

buffer  content
0       empty  b_state = 0
1       DATA   b_state = 0x1023
2       DATA   b_state = 0x1023
3       empty  b_state = 0
4       empty  b_state = 0
5       DATA   b_state = 0x1023
6       DATA   b_state = 0x1023
7       empty  b_state = 0

---

Quoted 10 lines
 .gitignore         |    1 	1 +	0 -	0 !
 287                |   74 	74 +	0 -	0 !
 287.out            |    1 	1 +	0 -	0 !
 group              |    1 	1 +	0 -	0 !
 src/Makefile       |    2 	1 +	1 -	0 !
 src/extsize_page.c |  113 	113 +	0 -	0 !
 6 files changed, 191 insertions(+), 1 deletion(-)
 create mode 100755 287
 create mode 100644 287.out
 create mode 100644 src/extsize_page.c

.....

Far too verbose - xfs_io can do just about everything for you in 5 lines. And you don't need a template file to compare to - just dump the hexdump output in the to golden image and the test harness will do the comapre for you.

Also, see test 194 for the way we normally set up such a test:

pgsize=$here/src/feature -s blksize=expr $pgsize / 8
....
_scratch_mkfs_xfs -b size=$blksize >/dev/null 2>&1

and the core of the xfs_io based test is:

mnt=/mnt/scratch
test_file=$mnt/foo

rm -f $test_file

xfs_io -f -c "extsize `expr $pgsize \* 10`" \
        -c "pwrite `expr $pgsize + $blksize` `expr $blksize \* 2`" \
        -c "pwrite `expr $pgsize + $blksize \* 5` `expr $blksize \* 2`" \
        -c "truncate `expr $pgsize \* 3`" \
        -c stat -c "bmap -vp" \
        $test_file

hexdump $test_file

umount $mnt
mount $mnt

hexdump $test_file

--

The output I get on a current 3.5-rc1 kernel is this:

wrote 1024/1024 bytes at offset 4608
1 KiB, 2 ops; 0.0000 sec (15.751 MiB/sec and 32258.0645 ops/sec) wrote 1024/1024 bytes at offset 6656
1 KiB, 2 ops; 0.0000 sec (97.656 MiB/sec and 200000.0000 ops/sec) fd.path = "/mnt/scratch/foo"
fd.flags = non-sync,non-direct,read-write
stat.ino = 35
stat.type = regular file
stat.size = 12288
stat.blocks = 80
fsxattr.xflags = 0x800 [----------e---]
fsxattr.projid = 0
fsxattr.extsize = 40960
fsxattr.nextents = 3
fsxattr.naextents = 0
dioattr.mem = 0x200
dioattr.miniosz = 512
dioattr.maxiosz = 2147483136
/mnt/scratch/foo:

 EXT: FILE-OFFSET      BLOCK-RANGE      AG AG-OFFSET        TOTAL FLAGS
   0: [0..8]:          48..56            0 (48..56)             9 10000
   1: [9..12]:         57..60            0 (57..60)             4 00000
   2: [13..79]:        61..127           0 (61..127)           67 10000

0000000 0000 0000 0000 0000 0000 0000 0000 0000
*
0001200 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
*
0001600 0000 0000 0000 0000 0000 0000 0000 0000
*
0001a00 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
*
0001e00 0000 0000 0000 0000 0000 0000 0000 0000
*
0003000
0000000 0000 0000 0000 0000 0000 0000 0000 0000
*
0001200 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
*
0001600 0000 0000 0000 0000 0000 0000 0000 0000
*
0003000

Which shows that the second region wasn't converted. I'm not sure why you are seeing stale data - you should see zeros because the region is still unwritten. What kernel are you testing on?

Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#3 lore raw

Hi Dave,

The reason I did it the way I did is because if I do not do the unmount it can take 5 to 10 iteration to reproduce the problem. However your version with xfs_io is much simpler I will update the code with your change.

Second for the stale data if you look at buffer 3(11) and 4(12) you see that xfs_bmap show the buffer as REAL but they should be unwritten so what you read is what ever is on disk at the time (aka stale data).

The kernel I used for the testing is 3.4.0-rc2-0.2-default+ it is from oss.sgi.com and the branch head is

14c26c6a05de138a4fd9a0c05ff8e7435a618324 xfs: add trace points for log forces 3ba316037470bbf98c8a16c2179c02794fb8862e xfs: fix memory reclaim deadlock on agi buffer ea562ed6e7df5acd9392d993882c39e855099165 xfs: fix delalloc quota accounting on failure

Again thanks a lot for the feedback I will provide an updated test with your suggested change.

Quoted 117 lines · Dave Chinner
Quoted 27 lines · Alain Renaud

Test using extsize to create a file with multiple extent in one
PAGE. This show an error in the block conversion from unwritten to real. As a result we tag raw disk block as valid(3-4) and valid data as unwritten(5-6)

On an x86_64 machine the page should look like this.

buffer  content
0       empty  b_state = 0
1       DATA   b_state = 0x1023
2       DATA   b_state = 0x1023
3       empty  b_state = 0
4       empty  b_state = 0
5       DATA   b_state = 0x1023
6       DATA   b_state = 0x1023
7       empty  b_state = 0

---

  .gitignore         |    1 	1 +	0 -	0 !
  287                |   74 	74 +	0 -	0 !
  287.out            |    1 	1 +	0 -	0 !
  group              |    1 	1 +	0 -	0 !
  src/Makefile       |    2 	1 +	1 -	0 !
  src/extsize_page.c |  113 	113 +	0 -	0 !
  6 files changed, 191 insertions(+), 1 deletion(-)
Quoted 3 lines

create mode 100755 287
create mode 100644 287.out
create mode 100644 src/extsize_page.c

.....

Far too verbose - xfs_io can do just about everything for you in 5 lines. And you don't need a template file to compare to - just dump the hexdump output in the to golden image and the test harness will do the comapre for you.

Also, see test 194 for the way we normally set up such a test:

pgsize=$here/src/feature -s
blksize=expr $pgsize / 8
....
_scratch_mkfs_xfs -b size=$blksize>/dev/null 2>&1

and the core of the xfs_io based test is:

mnt=/mnt/scratch
test_file=$mnt/foo

rm -f $test_file

xfs_io -f -c "extsize `expr $pgsize \* 10`" \
         -c "pwrite `expr $pgsize + $blksize` `expr $blksize \* 2`" \
         -c "pwrite `expr $pgsize + $blksize \* 5` `expr $blksize \* 2`" \
         -c "truncate `expr $pgsize \* 3`" \
         -c stat -c "bmap -vp" \
         $test_file

hexdump $test_file

umount $mnt
mount $mnt

hexdump $test_file

--

The output I get on a current 3.5-rc1 kernel is this:

wrote 1024/1024 bytes at offset 4608
1 KiB, 2 ops; 0.0000 sec (15.751 MiB/sec and 32258.0645 ops/sec) wrote 1024/1024 bytes at offset 6656
1 KiB, 2 ops; 0.0000 sec (97.656 MiB/sec and 200000.0000 ops/sec) fd.path = "/mnt/scratch/foo"
fd.flags = non-sync,non-direct,read-write
stat.ino = 35
stat.type = regular file
stat.size = 12288
stat.blocks = 80
fsxattr.xflags = 0x800 [----------e---]
fsxattr.projid = 0
fsxattr.extsize = 40960
fsxattr.nextents = 3
fsxattr.naextents = 0
dioattr.mem = 0x200
dioattr.miniosz = 512
dioattr.maxiosz = 2147483136
/mnt/scratch/foo:

  EXT: FILE-OFFSET      BLOCK-RANGE      AG AG-OFFSET        TOTAL FLAGS
    0: [0..8]:          48..56            0 (48..56)             9 10000
    1: [9..12]:         57..60            0 (57..60)             4 00000
    2: [13..79]:        61..127           0 (61..127)           67 10000

0000000 0000 0000 0000 0000 0000 0000 0000 0000
*
0001200 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
*
0001600 0000 0000 0000 0000 0000 0000 0000 0000
*
0001a00 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
*
0001e00 0000 0000 0000 0000 0000 0000 0000 0000
*
0003000
0000000 0000 0000 0000 0000 0000 0000 0000 0000
*
0001200 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
*
0001600 0000 0000 0000 0000 0000 0000 0000 0000
*
0003000

Which shows that the second region wasn't converted. I'm not sure why you are seeing stale data - you should see zeros because the region is still unwritten. What kernel are you testing on?

Signature · 9 lines
===============================================
Alain Renaud     - Cluster File System Engineer
[email protected]  - SGI
===============================================

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#4 lore raw

PATCH v2 1/1 xfstest: test corruption with multi extent in PAGE.

Since I got no feedback on the updated version let me send it again.

xfstest: test corruption with multi extent in PAGE.

Test using extsize/resvspace to create a file with multiple extent in one PAGE. This show an error in the block conversion from unwritten to real. As a result we tag raw disk block as valid(3-4)
and valid data as unwritten(5-6)

On an x86_64 machine the page should look like this.

buffer  content
0       empty  b_state = 0
1       DATA   b_state = 0x1023
2       DATA   b_state = 0x1023
3       empty  b_state = 0
4       empty  b_state = 0
5       DATA   b_state = 0x1023
6       DATA   b_state = 0x1023
7       empty  b_state = 0

---

 287     |  148     148 +    0 -    0 !
 287.out |  102     102 +    0 -    0 !
 group   |    1     1 +    0 -    0 !
 3 files changed, 251 insertions(+)
 create mode 100755 287
 create mode 100644 287.out
287 new file +148 -0
Index: b/287
===================================================================
@@ -0,0 +1,148 @@
+#! /bin/bash
+# FS QA Test No. 287
+#
+# File corruption with multiple extent in a page.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2012 SGI.  All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#
+#-----------------------------------------------------------------------
+#
+# creator
[email protected]
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1    # failure is the default!
+trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# real QA test starts here
+_supported_fs xfs
+_supported_os Linux
+
+_require_scratch
+
+pgsize=$($here/src/feature -s)
+blksize=$(( ${pgsize} / 8))
+
+rm -f $seq.full
+if ! _scratch_mkfs_xfs -b size=${blksize} >> $seq.full 2>&1
+then
+    echo "!!! failed to mkfs on $SCRATCH_DEV"
+    exit ${status}
+fi
+
+_scratch_mount
+
+# Test one create a file using extsize with mix
+# real/unwritten extent in page. assume 8 blocks per page
+# U/R/R/U/U/R/R/U
+echo "Testing page U/R/R/U/U/R/R/U with extsize"
+testfile=${SCRATCH_MNT}/test_extsize_urrurru.data
+rm -f ${testfile}
+xfs_io -f -c "extsize $(( $pgsize * 5))" \
+    -c "pwrite $(( $pgsize + $blksize )) $(( $blksize * 2))" \
+    -c "pwrite $(( $pgsize + $blksize * 5 )) $(( $blksize * 2))" \
+    -c "truncate $(( $pgsize * 3))" \
+    -c stat -c "bmap -vp" \
+    ${testfile} >> $seq.full 2>&1
+
+hexdump ${testfile}
+echo
+
+# Test one create a file using resvsp with mix
+# real/unwritten extent in page. assume 8 blocks per page
+# U/R/R/U/U/R/R/U
+echo "Testing page U/R/R/U/U/R/R/U with resvsp"
+testfile=${SCRATCH_MNT}/test_resvsp_urrurru.data
+rm -f ${testfile}
+xfs_io -f -c "resvsp 0 $(( $pgsize * 5))" \
+    -c "pwrite $(( $pgsize + $blksize )) $(( $blksize * 2))" \
+    -c "pwrite $(( $pgsize + $blksize * 5 )) $(( $blksize * 2))" \
+    -c "truncate $(( $pgsize * 3))" \
+    -c stat -c "bmap -vp" \
+    ${testfile} >> $seq.full 2>&1
+
+hexdump ${testfile}
+echo
+
+
+# Test one create a file using resvsp with mix
+# real/unwritten extent in page. assume 8 blocks per page
+# real overflow resvspace
+# U/R/R/U/U/R/R/R R...
+echo "Testing page U/R/R/U/U/R/R/R with resvsp"
+testfile=${SCRATCH_MNT}/test_resvsp_urrurrr.data
+rm -f ${testfile}
+xfs_io -f -c "resvsp 0 $(( $pgsize * 2))" \
+    -c "pwrite $(( $pgsize + $blksize )) $(( $blksize * 2))" \
+    -c "pwrite $(( $pgsize + $blksize * 5 )) $(( $blksize * 4))" \
+    -c "truncate $(( $pgsize * 5))" \
+    -c stat -c "bmap -vp" \
+   ${testfile} >> $seq.full 2>&1
+
+hexdump ${testfile}
+echo
+
+# Test one create a file using resvsp with mix
+# real/unwritten extent in page. assume 8 blocks per page
+# switch problem to cluster write.
+#  R/R/R/R/R/R/R/R R/R/R/U/U/R/R/U
+echo "Testing page R/R/R/U/U/R/R/U with resvsp"
+testfile=${SCRATCH_MNT}/test_resvsp_rrrurru.data
+rm -f ${testfile}
+xfs_io -f -c "resvsp 0 $(( $pgsize * 10))" \
+    -c "pwrite $(( $pgsize )) $(( $blksize * 10))" \
+    -c "pwrite $(( $pgsize + $blksize * 12 )) $(( $blksize * 2))" \
+    -c "truncate $(( $pgsize * 5))" \
+    -c stat -c "bmap -vp" \
+    ${testfile} >> $seq.full 2>&1
+
+hexdump ${testfile}
+echo
+
+
+_scratch_unmount
+_scratch_mount
+echo
+# Dump all the file again after unmount making sure no
+# pages can be cached.
+
+echo "dumping test_extsize_urrurru.data"
+hexdump ${SCRATCH_MNT}/test_extsize_urrurru.data
+
+echo "dumping test_resvsp_urrurru.data"
+hexdump ${SCRATCH_MNT}/test_resvsp_urrurru.data
+
+echo "dumping test_resvsp_urrurrr.data"
+hexdump ${SCRATCH_MNT}/test_resvsp_urrurrr.data
+
+echo "dumping test_resvsp_rrrurru.data"
+hexdump ${SCRATCH_MNT}/test_resvsp_rrrurru.data
+
+_scratch_unmount
+
+
+# success, all done
+status=0
+exit
287.out new file +102 -0

102 lines not on this page · raw message

group +1 -0
Index: b/group
===================================================================
@@ -405,3 +405,4 @@
 284 auto
 285 auto rw
 286 other
+287 auto rw

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Reviewed-by on v2 1/1
#5 lore raw

Re: [PATCH v2] xfstest: test corruption with multi extent in PAGE.

Quoted 22 lines · Alain Renaud

Since I got no feedback on the updated version let me send it again.

xfstest: test corruption with multi extent in PAGE.

Test using extsize/resvspace to create a file with multiple extent in one PAGE.
This show an error in the block conversion from unwritten to real. As a result we tag raw disk block as valid(3-4)
and valid data as unwritten(5-6)

On an x86_64 machine the page should look like this.

buffer content
0 empty b_state = 0
1 DATA b_state = 0x1023
2 DATA b_state = 0x1023
3 empty b_state = 0
4 empty b_state = 0
5 DATA b_state = 0x1023
6 DATA b_state = 0x1023
7 empty b_state = 0

Quoted 1 line

Tested with and without commit 7d0fa3ecba2f12ceef93fffe615e5dd9b50bb794 Looks good.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs