Re: [PATCH 00/15 v2] xfstests: new btrfs stress test cases
On Thu, Aug 28, 2014 at 09:47:41PM +0800, Eryu Guan wrote: > This patchset add new stress test cases for btrfs by running two > different btrfs operations simultaneously under fsstress to ensure > btrfs doesn't hang or oops in such situations. btrfs scrub and > btrfs check will be run after each test. > > The test matrix is the combination of 6 btrfs operations: > > balance > create/mount/umount/delete subvolume > replace device > scrub > defrag > remount with different compress algorithms > > Short descriptions: > > 059: balance-subvolume > 060: balance-scrub > 061: balance-defrag > 062: balance-remount > 063: balance-replace > 064: subvolume-replace > 065: subvolume-scrub > 066: subvolume-defrag > 067: subvolume-remount > 068: replace-scrub > 069: replace-defrag > 070: replace-remount > 071: scrub-defrag > 072: scrub-remount > 073: defrag-remount Can I get some reviews for btrfs people for this series, please? Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ext4 vs btrfs performance on SSD array
On Tue, Aug 26, 2014 at 07:39:08PM -0400, Nikolai Grigoriev wrote: > Hi, > > This is not exactly a problem - I am trying to understand why BTRFS > demonstrates significantly higher throughput in my environment. > > I am observing something that I cannot explain. I am trying to come up > with a good filesystem configuration using HP P420i controller and > SSDs (Intel S3500). Out of curiosity I have tried BTRFS (still > unstable so I can't really expect to be able to use it) and noticed > that the read speed is about 150% of ext4 - while write speed is > comparable. ... > When I read, I observe different picture: > > Device: rrqm/s wrqm/s r/s w/srMB/swMB/s avgrq-sz > avgqu-sz await svctm %util > (ext4 - reading) > sdb 0.00 0.00 4782.000.00 597.75 0.00 256.00 > 1.570.33 0.18 84.10 > (btrfs - reading) > Device: rrqm/s wrqm/s r/s w/srMB/swMB/s avgrq-sz > avgqu-sz await svctm %util > sdc 207.00 0.00 1794.000.00 886.40 0.00 1011.90 > 10.595.90 0.56 100.00 > (xfs - reading) > Device: rrqm/s wrqm/s r/s w/srMB/swMB/s avgrq-sz > avgqu-sz await svctm %util > sdd 0.00 0.00 4623.000.00 577.88 0.00 256.00 > 1.710.37 0.21 97.00 Pretty obvious difference: avgrq-sz. btrfs is doing 512k IOs, ext4 and XFS are doing is doing 128k IOs because that's the default block device readahead size. 'blockdev --setra 1024 /dev/sdd' before mounting the filesystem will probably fix it. -Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/15] xfstests: new btrfs stress test cases
On Wed, Aug 20, 2014 at 11:24:37AM -0700, Zach Brown wrote:
> On Thu, Aug 21, 2014 at 01:33:48AM +0800, Eryu Guan wrote:
> > This patchset add new stress test cases for btrfs by running two
> > different btrfs operations simultaneously under fsstress to ensure
> > btrfs doesn't hang or oops in such situations. btrfs scrub and
> > btrfs check will be run after each test.
>
> Cool.
>
> > The test matrix is the combination of 6 btrfs operations:
> >
> > balance
> > create/mount/umount/delete subvolume
> > replace device
> > scrub
> > defrag
> > remount with different compress algorithms
> >
> > Short descriptions:
> >
> > 057: balance-subvolume
> > 058: balance-scrub
> > 059: balance-defrag
> > 060: balance-remount
> > 061: balance-replace
> > 062: subvolume-replace
> > 063: subvolume-scrub
> > 064: subvolume-defrag
> > 065: subvolume-remount
> > 066: replace-scrub
> > 067: replace-defrag
> > 068: replace-remount
> > 069: scrub-defrag
> > 070: scrub-remount
> > 071: defrag-remount
>
> But I'm not sure it should be built this way.
>
> At the very least each operation's implementation should be in a shared
> function somewhere instead of being duplicated in each test.
>
> But I don't think there should be a seperate test for each combination.
> With a bit of fiddly bash you can automate generating unique
> combinations of operations that are defined as functions in one test.
>
> btrfs_op_balance()
> {
> echo hi
> }
>
> btrfs_op_scrub()
> {
> echo hi
> }
>
> btrfs_op_defrag()
> {
> echo hi
> }
>
> ops=($(declare -F | awk '/-f btrfs_op_/ {print $3}'))
> nr=${#ops[@]}
>
> for i in $(seq 0 $((nr - 2))); do
> for j in $(seq $((i + 1)) $((nr - 1))); do
> echo ${ops[i]} ${ops[j]}
> done
> done
Yes, it could be done like that, but historically that has proven to
be a bad idea. Multiplexing tens of tests within a single test is
just makes it hard to determine what failed. It might fail one
combination in 3.16, a different combo in 3.17 and yet another in
3.18. But from a reporting point of view, all we see is that a
single test failed, rather than being able to see that there were
three separate problems and that btrfs_op_scrub() was the common
factor in all three failures.
It's trivial to write this as a bunch of helper functions and then
boiler-plate the actual tests themselves. There will be little
difference in terms of run time, but we get much more fine-grained
control of execution and reporting
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] btrfs: new test to run btrfs balance and subvolume test simultaneously
On Thu, Aug 21, 2014 at 05:15:01PM +0800, Qu Wenruo wrote:
>
> Original Message
> Subject: Re: [PATCH 01/15] btrfs: new test to run btrfs balance and
> subvolume test simultaneously
> From: Dave Chinner
> To: Qu Wenruo
> Date: 2014年08月21日 17:01
> >On Thu, Aug 21, 2014 at 10:04:30AM +0800, Qu Wenruo wrote:
> >> Original Message
> >>Subject: [PATCH 01/15] btrfs: new test to run btrfs balance and
> >>subvolume test simultaneously
> >>From: Eryu Guan
> >>To:
> >>Date: 2014年08月21日 01:33
> >>>Run btrfs balance and subvolume create/mount/umount/delete simultaneously,
> >>>with fsstress running in background.
> >>>+# test case array
> >>>+tcs=(
> >>>+ "-m single -d single"
> >>>+ "-m dup -d single"
> >>>+ "-m raid0 -d raid0"
> >>>+ "-m raid1 -d raid0"
> >>>+ "-m raid1 -d raid1"
> >>>+ "-m raid10 -d raid10"
> >>>+ "-m raid5 -d raid5"
> >>>+ "-m raid6 -d raid6"
> >>>+)
> >>I wonder should we add the mkfs options there.
> >>Since xfstests already use environment MKFS_OPTIONS to do mkfs,
> >>if really need to test all mkfs options, IMO it is better to change
> >>MKFS_OPTIONS on each test round.
> >Hmmm - I you didn't read the code, because:
> >
> >>>+run_test()
> >>>+{
> >>>+ local mkfs_opts=$1
> >>>+ local saved_mkfs_opts=$MKFS_OPTIONS
> >>>+ local subvol_mnt=$tmp.mnt
> >>>+
> >>>+ echo "Test $mkfs_opts" >>$seqres.full
> >>>+
> >>>+ MKFS_OPTIONS="$MKFS_OPTIONS $mkfs_opts"
> >>>+ # dup only works on single device
> >it's doing exactly what you suggest.
> I am afraid that you misunderstand what I mean...
> I just mean these mount option should be done by setting environment
> before runing check or set in local.conf.
You can override or append to MKFS_OPTIONS and MOUNT_OPTIONS in
tests if required - lots of tests do exactly that (e.g. any quota
test your care to name). That modification, however, is only valid
for the specific test being run because the modification is to the
environment of the test process, not the environment of check
process that is running the tests
i.e. Running custom mkfs or mount options like this is perfectly
acceptable and I'm just commenting that the implementation of those
custom options could be a lot better.
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] btrfs: new test to run btrfs balance and subvolume test simultaneously
On Thu, Aug 21, 2014 at 10:04:30AM +0800, Qu Wenruo wrote:
>
> Original Message
> Subject: [PATCH 01/15] btrfs: new test to run btrfs balance and
> subvolume test simultaneously
> From: Eryu Guan
> To:
> Date: 2014年08月21日 01:33
> >Run btrfs balance and subvolume create/mount/umount/delete simultaneously,
> >with fsstress running in background.
> >
> >Signed-off-by: Eryu Guan
> >---
> > tests/btrfs/057 | 147
> >
> > tests/btrfs/057.out | 2 +
> > tests/btrfs/group | 1 +
> > 3 files changed, 150 insertions(+)
> > create mode 100755 tests/btrfs/057
> > create mode 100644 tests/btrfs/057.out
> >
> >diff --git a/tests/btrfs/057 b/tests/btrfs/057
> >new file mode 100755
> >index 000..2f507a7
> >--- /dev/null
> >+++ b/tests/btrfs/057
> >@@ -0,0 +1,147 @@
> >+#! /bin/bash
> >+# FSQA Test No. btrfs/057
> >+#
> >+# Run btrfs balance and subvolume create/mount/umount/delete simultaneously,
> >+# with fsstress running in background.
> >+#
> >+#---
> >+# Copyright (C) 2014 Red Hat Inc. 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
> >+#
> >+#---
> >+#
> >+
> >+seq=`basename $0`
> >+seqres=$RESULT_DIR/$seq
> >+echo "QA output created by $seq"
> >+
> >+here=`pwd`
> >+tmp=/tmp/$$
> >+status=1
> >+trap "_cleanup; exit \$status" 0 1 2 3 15
> >+
> >+_cleanup()
> >+{
> >+cd /
> >+rm -fr $tmp.*
> >+}
> >+
> >+# get standard environment, filters and checks
> >+. ./common/rc
> >+. ./common/filter
> >+
> >+# real QA test starts here
> >+_supported_fs btrfs
> >+_supported_os Linux
> >+_require_scratch
> >+_require_scratch_dev_pool 4
> >+
> >+rm -f $seqres.full
> >+
> >+# test case array
> >+tcs=(
> >+"-m single -d single"
> >+"-m dup -d single"
> >+"-m raid0 -d raid0"
> >+"-m raid1 -d raid0"
> >+"-m raid1 -d raid1"
> >+"-m raid10 -d raid10"
> >+"-m raid5 -d raid5"
> >+"-m raid6 -d raid6"
> >+)
> I wonder should we add the mkfs options there.
> Since xfstests already use environment MKFS_OPTIONS to do mkfs,
> if really need to test all mkfs options, IMO it is better to change
> MKFS_OPTIONS on each test round.
Hmmm - I you didn't read the code, because:
> >+run_test()
> >+{
> >+local mkfs_opts=$1
> >+local saved_mkfs_opts=$MKFS_OPTIONS
> >+local subvol_mnt=$tmp.mnt
> >+
> >+echo "Test $mkfs_opts" >>$seqres.full
> >+
> >+MKFS_OPTIONS="$MKFS_OPTIONS $mkfs_opts"
> >+# dup only works on single device
it's doing exactly what you suggest.
And it's wrong. This:
_scratch_mkfs $mkfs_opts
is all that is needed. This wheel does not need reinventing. ;)
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstest: add tests/btrfs/059 to test seed operations
On Fri, Aug 15, 2014 at 11:16:21PM +0800, Anand Jain wrote: > This test contains a set of 5 sub test cases which will tests > the device add and replacement on a seed FS. The kernel messages > are checked at the end of the each of the 5 test cases to see > if there are any kernel warnings or bugs reported. As in general > btrfs do report warning when device counts such as num_devices > and rw_devices appears to be wrong. > > Since the tests has to deal with the replace, it would need > up to five scratch device with same size. Please separate this out into 5 tests using shared infrastructure. Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] common: get fs type again using device canonical name in _fs_type
On Fri, Aug 01, 2014 at 01:02:58PM +0800, Eryu Guan wrote: > On Fri, Aug 01, 2014 at 02:49:10PM +1000, Dave Chinner wrote: > > On Fri, Aug 01, 2014 at 12:02:41PM +0800, Eryu Guan wrote: > > > On Fri, Aug 01, 2014 at 10:21:59AM +1000, Dave Chinner wrote: > > > > On Thu, Jul 31, 2014 at 06:52:37PM +0800, Eryu Guan wrote: > > > > > When testing with lvm, a previous btrfsck run could change df output > > > > > from something like > > > > > > > > > > /dev/mapper/rhel_hp--dl388eg8--01-testlv1 btrfs 15728640 900 13602172 > > > > > 1% /mnt/btrfs > > > > > > > > > > to > > > > > > > > > > /dev/dm-3 btrfs 15728640 900 13602172 1% /mnt/btrfs > > > > > > > > I don't follow you. Why would running btrfsck change the name of the > > > > device? If the filesystem is umounted and mounted again, then the > > > > device could change, but btrfsck should not be not doing the > > > > unmount/mount, and so unless the TEST_DEV/SCRATCH_DEV is changing > > > > the output of df should be identical... > > > > > > > > So before we change the _fs_type() code, can you explain exactly > > > > how, when and why the device name is changing to me? > > > > > > Assume that we have two btrfs filesystems, kernel is 3.16.0-rc4+ > > > > > > [root@hp-dl388eg8-01 btrfs-progs]# btrfs fi show > > > Label: none uuid: 1aba7da5-ce2b-4af0-a716-db732abc60b2 > > > Total devices 1 FS bytes used 384.00KiB > > > devid1 size 15.00GiB used 2.04GiB path > > > /dev/mapper/rhel_hp--dl388eg8--01-testlv1 > > > > > > Label: none uuid: 26ff4f12-f6d9-4cbc-aae2-57febeefde37 > > > Total devices 2 FS bytes used 112.00KiB > > > devid1 size 15.00GiB used 2.03GiB path > > > /dev/mapper/rhel_hp--dl388eg8--01-testlv2 > > > devid2 size 15.00GiB used 2.01GiB path > > > /dev/mapper/rhel_hp--dl388eg8--01-testlv3 > > > > > > Btrfs v3.14.2 > > > > > > And testlv1 was mounted at /mnt/btrfs > > > > > > [root@hp-dl388eg8-01 btrfs-progs]# df -TP /mnt/btrfs > > > FilesystemType 1024-blocks Used > > > Available Capacity Mounted on > > > /dev/mapper/rhel_hp--dl388eg8--01-testlv1 btrfs15728640 512 > > > 13602560 1% /mnt/btrfs > > > > > > Now run btrfsck on testlv2, btrfsck will scan all btrfs devices and > > > somehow change the device name. > > > > > > [root@hp-dl388eg8-01 btrfs-progs]# btrfsck > > > /dev/mapper/rhel_hp--dl388eg8--01-testlv2 >/dev/null 2>&1 > > > > > > # device name changed in df output and btrfs fi show output > > > [root@hp-dl388eg8-01 btrfs-progs]# df -TP /mnt/btrfs > > > Filesystem Type 1024-blocks Used Available Capacity Mounted on > > > /dev/dm-3 btrfs15728640 512 13602560 1% /mnt/btrfs > > > [root@hp-dl388eg8-01 btrfs-progs]# btrfs fi show > > > Label: none uuid: 1aba7da5-ce2b-4af0-a716-db732abc60b2 > > > Total devices 1 FS bytes used 384.00KiB > > > devid1 size 15.00GiB used 2.04GiB path /dev/dm-3 > > > > > > Label: none uuid: 26ff4f12-f6d9-4cbc-aae2-57febeefde37 > > > Total devices 2 FS bytes used 112.00KiB > > > devid1 size 15.00GiB used 2.03GiB path > > > /dev/mapper/rhel_hp--dl388eg8--01-testlv2 > > > devid2 size 15.00GiB used 2.01GiB path > > > /dev/mapper/rhel_hp--dl388eg8--01-testlv3 > > > > > > Btrfs v3.14.2 > > > > > > This only happens when btrfsck a btrfs with multiple devices, so this > > > only affects xfstests run on btrfs with SCRATCH_DEV_POOL set to lvm > > > lvs. > > > > > > Maybe this is a bug of btrfs-progs and we should fix it there? > > > > Yes, that smells of a btrfs-progs bug. If your /etc/mtab a link to > > /proc/mounts? If not, does the contents change when you run btrfsck, > > and does the problem go away when you replace /etc/mtab with a link > > to /proc/mounts? > > /etc/mtab is a symlink to /proc/self/mounts, so does /proc/mounts > > [root@hp-dl388eg8-01 btrfs-progs]# ls -l /etc/mtab > lrwxrwxrwx. 1 root root 17 Sep 22 2013 /etc/mtab -> /proc/self/mounts > [root@hp-dl388eg8-01 btrfs-progs]# ls -l /proc/mounts > lrwxrwxrwx. 1 root root 11 Aug 1 00:59 /proc/mounts -> self/mounts > > And the devic
Re: [PATCH v2] common: get fs type again using device canonical name in _fs_type
On Fri, Aug 01, 2014 at 12:02:41PM +0800, Eryu Guan wrote: > On Fri, Aug 01, 2014 at 10:21:59AM +1000, Dave Chinner wrote: > > On Thu, Jul 31, 2014 at 06:52:37PM +0800, Eryu Guan wrote: > > > When testing with lvm, a previous btrfsck run could change df output > > > from something like > > > > > > /dev/mapper/rhel_hp--dl388eg8--01-testlv1 btrfs 15728640 900 13602172 1% > > > /mnt/btrfs > > > > > > to > > > > > > /dev/dm-3 btrfs 15728640 900 13602172 1% /mnt/btrfs > > > > I don't follow you. Why would running btrfsck change the name of the > > device? If the filesystem is umounted and mounted again, then the > > device could change, but btrfsck should not be not doing the > > unmount/mount, and so unless the TEST_DEV/SCRATCH_DEV is changing > > the output of df should be identical... > > > > So before we change the _fs_type() code, can you explain exactly > > how, when and why the device name is changing to me? > > Assume that we have two btrfs filesystems, kernel is 3.16.0-rc4+ > > [root@hp-dl388eg8-01 btrfs-progs]# btrfs fi show > Label: none uuid: 1aba7da5-ce2b-4af0-a716-db732abc60b2 > Total devices 1 FS bytes used 384.00KiB > devid1 size 15.00GiB used 2.04GiB path > /dev/mapper/rhel_hp--dl388eg8--01-testlv1 > > Label: none uuid: 26ff4f12-f6d9-4cbc-aae2-57febeefde37 > Total devices 2 FS bytes used 112.00KiB > devid1 size 15.00GiB used 2.03GiB path > /dev/mapper/rhel_hp--dl388eg8--01-testlv2 > devid2 size 15.00GiB used 2.01GiB path > /dev/mapper/rhel_hp--dl388eg8--01-testlv3 > > Btrfs v3.14.2 > > And testlv1 was mounted at /mnt/btrfs > > [root@hp-dl388eg8-01 btrfs-progs]# df -TP /mnt/btrfs > FilesystemType 1024-blocks Used Available > Capacity Mounted on > /dev/mapper/rhel_hp--dl388eg8--01-testlv1 btrfs15728640 512 13602560 > 1% /mnt/btrfs > > Now run btrfsck on testlv2, btrfsck will scan all btrfs devices and > somehow change the device name. > > [root@hp-dl388eg8-01 btrfs-progs]# btrfsck > /dev/mapper/rhel_hp--dl388eg8--01-testlv2 >/dev/null 2>&1 > > # device name changed in df output and btrfs fi show output > [root@hp-dl388eg8-01 btrfs-progs]# df -TP /mnt/btrfs > Filesystem Type 1024-blocks Used Available Capacity Mounted on > /dev/dm-3 btrfs15728640 512 13602560 1% /mnt/btrfs > [root@hp-dl388eg8-01 btrfs-progs]# btrfs fi show > Label: none uuid: 1aba7da5-ce2b-4af0-a716-db732abc60b2 > Total devices 1 FS bytes used 384.00KiB > devid1 size 15.00GiB used 2.04GiB path /dev/dm-3 > > Label: none uuid: 26ff4f12-f6d9-4cbc-aae2-57febeefde37 > Total devices 2 FS bytes used 112.00KiB > devid1 size 15.00GiB used 2.03GiB path > /dev/mapper/rhel_hp--dl388eg8--01-testlv2 > devid2 size 15.00GiB used 2.01GiB path > /dev/mapper/rhel_hp--dl388eg8--01-testlv3 > > Btrfs v3.14.2 > > This only happens when btrfsck a btrfs with multiple devices, so this > only affects xfstests run on btrfs with SCRATCH_DEV_POOL set to lvm > lvs. > > Maybe this is a bug of btrfs-progs and we should fix it there? Yes, that smells of a btrfs-progs bug. If your /etc/mtab a link to /proc/mounts? If not, does the contents change when you run btrfsck, and does the problem go away when you replace /etc/mtab with a link to /proc/mounts? Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Remove certain calls for releasing page cache
On 31 July 2014 12:05, Nick Krause wrote: > On Wed, Jul 30, 2014 at 7:30 PM, Dave Airlie wrote: >>> This patch removes the lines for releasing the page cache in certain >>> files as this may aid in perfomance with writes in the compression >>> rountines of btrfs. Please note that this patch has not been tested >>> on my own hardware due to no compression based btrfs volumes of my >>> own. >>> >> >> For all that is sacred, STOP. >> >> Go and do something else, you are wasting people's valuable time, >> >> Don't send any patches you haven't tested ever. If you aren't capable >> of setting up a VM to run compressed btrfs volumes in, what makes you >> think you can patch the code. >> >> This isn't how you learn to be a kernel developer by wasting other >> kernel developers time, if you can't work out why releasing the page cache >> is necessary then don't send the patch until you have spent the time >> understanding what the page cache is. >> >> I know you'll just ignore this, and keep on trucking just like you ignored >> the other messages from Stephen before. >> >> But if you want to work on the kernel, this isn't the way to do it, and >> nobody will ever take a patch from you seriously if you continue in this >> fashion. >> >> Dave. > Dave , > Seems I need to have tested this code first. > Regards Nick No you needed to do a lot more, these one line replies from you are quite stupid, You are quite deliberately missing the point of people trying to help you, Dave. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Remove certain calls for releasing page cache
> This patch removes the lines for releasing the page cache in certain > files as this may aid in perfomance with writes in the compression > rountines of btrfs. Please note that this patch has not been tested > on my own hardware due to no compression based btrfs volumes of my > own. > For all that is sacred, STOP. Go and do something else, you are wasting people's valuable time, Don't send any patches you haven't tested ever. If you aren't capable of setting up a VM to run compressed btrfs volumes in, what makes you think you can patch the code. This isn't how you learn to be a kernel developer by wasting other kernel developers time, if you can't work out why releasing the page cache is necessary then don't send the patch until you have spent the time understanding what the page cache is. I know you'll just ignore this, and keep on trucking just like you ignored the other messages from Stephen before. But if you want to work on the kernel, this isn't the way to do it, and nobody will ever take a patch from you seriously if you continue in this fashion. Dave. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: btrfs kernel workqueues performance regression
On Tue, Jul 15, 2014 at 01:39:11PM -0400, Chris Mason wrote: > On 07/15/2014 11:26 AM, Morten Stevens wrote: > > Hi, > > > > I see that btrfs is using kernel workqueues since linux 3.15. After > > some tests I noticed performance regressions with fs_mark. > > > > mount options: rw,relatime,compress=lzo,space_cache > > > > fs_mark on Kernel 3.14.9: > > > > # fs_mark -d /mnt/btrfs/fsmark -D 512 -t 16 -n 4096 -s 51200 -L5 > > -S0 > > FSUse%Count SizeFiles/sec App Overhead > > 16553651200 17731.4 723894 > > 1 13107251200 16832.6 685444 > > 1 19660851200 19604.5 652294 > > 1 26214451200 18663.6 630067 > > 1 32768051200 20112.2 692769 > > > > The results are really nice! compress=lzo performs very good. > > > > fs_mark after upgrading to Kernel 3.15.4: > > > > # fs_mark -d /mnt/btrfs/fsmark -D 512 -t 16 -n 4096 -s 51200 -L5 > > -S0 > > FSUse%Count SizeFiles/sec App Overhead > > 06553651200 10718.1 749540 > > 0 13107251200 8601.2 853050 > > 0 19660851200 11623.2 558546 > > 0 26214451200 11534.2 536342 > > 0 32768051200 11167.4 578562 > > > > That's really a big performance regression :( > > > > What do you think? It's easy to reproduce with fs_mark. > > I wasn't able to trigger regressions here when we first merged it, but I > was sure that something would pop up. fs_mark is sensitive to a few > different factors outside just the worker threads, so it could easily be > another change as well. > > With 16 threads, the btree locking also has a huge impact, and we've > made change there too. FWIW, I ran my usual 16-way fsmark test last week on my sparse 500TB perf test rig on btrfs. It sucked, big time, much worse than it's sucked in the past. It didn't scale past a single thread - 1 thread got 24,000 files/s, 2 threads got 25,000 files/s 16 threads got 22,000 files/s. $ ./fs_mark -D 1 -S0 -n 10 -s 0 -L 32 -d /mnt/scratch/0 FSUse%Count SizeFiles/sec App Overhead 0 100 24808.8 686583 $ ./fs_mark -D 1 -S0 -n 10 -s 0 -L 32 -d /mnt/scratch/0 -d /mnt/scratch/1 -d /mnt/scratch/2 -d /mnt/scratch/3 -d /mnt/scratch/4 -d /mnt/scratch/5 -d /mnt/scratch/6 -d /mnt/scratch/7 -d /mnt/scratch/8 -d /mnt/scratch/9 -d /mnt/scratch/10 -d /mnt/scratch/11 -d /mnt/scratch/12 -d /mnt/scratch/13 -d /mnt/scratch/14 -d /mnt/scratch/15 FSUse%Count SizeFiles/sec App Overhead 0 1600 23599.7 38047237 Last time I ran this (probably about 3.12 - btrfs was simply too broken when I last tried on 3.14) I got about 80,000 files/s so this is a pretty significant regression. The 16-way run consumed most of the 16 CPUs in the system, and the perf top output showed this: + 44.48% [kernel] [k] _raw_spin_unlock_irqrestore + 28.60% [kernel] [k] queue_read_lock_slowpath + 14.34% [kernel] [k] queue_write_lock_slowpath + 1.91% [kernel] [k] _raw_spin_unlock_irq + 0.85% [kernel] [k] __do_softirq + 0.45% [kernel] [k] do_raw_read_lock + 0.43% [kernel] [k] do_raw_read_unlock + 0.42% [kernel] [k] btrfs_search_slot + 0.40% [kernel] [k] do_raw_spin_lock + 0.35% [kernel] [k] btrfs_tree_read_unlock + 0.33% [kernel] [k] do_raw_write_lock + 0.30% [kernel] [k] btrfs_clear_lock_blocking_rw + 0.29% [kernel] [k] btrfs_tree_read_lock All the CPU time is basically spend in locking functions. Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstests/btrfs: add test for quota groups and drop snapshot
On Thu, Jul 10, 2014 at 10:36:14AM -0700, Mark Fasheh wrote: > Hey Dave, thanks for the patch review! Pretty much all of what you wrote > sounds good to me, there's just one or two items I wanted to clarify - those > comments are inline. Thanks again, > > On Thu, Jul 10, 2014 at 10:43:30AM +1000, Dave Chinner wrote: > > On Wed, Jul 09, 2014 at 03:41:50PM -0700, Mark Fasheh wrote: > > > + > > > +# Enable qgroups now that we have our filesystem prepared. This > > > +# will kick off a scan which we will have to wait for below. > > > +$BTRFS_UTIL_PROG qu en $SCRATCH_MNT > > > +sleep 30 > > > > That seems rather arbitrary. The sleeps you are adding add well over > > a minute to the runtime, and a quota scan of a filesystem with 200 > > files should be almost instantenous. > > Yeah I'll bring that back down to 5 seconds? It's 30 from my testing because > I was being paranoid and neglected to update it for the rest of the world. Be nice to have the btrfs command wait for it to complete. Not being able to query the status of background work or wait for it is somewhat user unfriendly. If you could poll, then a 1s sleep in a poll loop would be fine. Short of that, then I guess sleep 5 is the best we can do. > > > > +_scratch_unmount > > > +_scratch_mount > > > > What is the purpose of this? > > This is kind of 'maximum paranoia' again from my own test script. The idea > was to make _absolutely_ certain that all metadata found it's way to disk > and won't be optimized in layout any more. There's a decent chance it > doesn't do anything but it doesn't seem a huge deal. I wasn't clear though - > do you want it removed or can I comment it for clarity? Comment. If someone reads the test in 2 years time they won't have to ask "wtf?"... > > > +# Ok, delete the snapshot we made previously. Since btrfs drop > > > +# snapshot is a delayed action with no way to force it, we have to > > > +# impose another sleep here. > > > +$BTRFS_UTIL_PROG su de $SCRATCH_MNT/snap1 > > > +sleep 45 > > > > That's indicative of a bug, yes? > > No, that's just how it happens. In fact, if you unmount while a snapshot is > being dropped, progress of the drop will be recorded and it will be > continued on next mount. However, since we *must* have the drop_snapshot > complete for this test I have the large sleep. Unlike the previous sleep I > don't think this can be reduced by much :( Right, again the "can't wait or poll for status of background work" issue comes up. That's the bug in the UI I was refering to. I guess that we'll just have to wait for a long time here. Pretty hacky, though... Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstests/btrfs: add test for quota groups and drop snapshot
On Wed, Jul 09, 2014 at 03:41:50PM -0700, Mark Fasheh wrote: > Test btrfs quota group consistency operations during snapshot delete. Btrfs > has had long standing issues with drop snapshot failing to properly account > for quota groups. This test crafts a snapshot tree with shared and exclusive > elements. The tree is removed and then quota group consistency is checked. > > This issue is fixed by the linux kernel btrfs patch series: >[PATCH 0/3] btrfs: qgroup fixes for btrfs_drop_snapshot >[PATCH 1/3] btrfs: add trace for qgroup accounting >[PATCH 2/3] btrfs: qgroup: account shared subtrees during snapshot delete >[PATCH 3/3] Btrfs: __btrfs_mod_ref should always use no_quota > > The following btrfsprogs patch set is needed for the actual check of qgroup > consistency: >[PATCH 1/5] btrfs-progs: print qgroup excl as unsigned >[PATCH 2/5] btrfs-progs: import ulist >[PATCH 3/5] btrfs-progs: add quota group verify code >[PATCH 4/5] btrfs-progs: show extent state for a subvolume >[PATCH 5/5] btrfs-progs: ignore orphaned qgroups by default > > The btrfsprogs patches can be found in the following repo: > > https://github.com/markfasheh/btrfs-progs-patches/tree/qgroup-verify > > This patch to xfstests can be found in the following repo: > > https://github.com/markfasheh/xfstests-patches/tree/qgroup-drop-snapshot > > +rm -f $seqres.full > + > +_scratch_mkfs >/dev/null 2>&1 > +_scratch_mount > + > +# This always reproduces level 1 trees > +maxfiles=100 > + > +echo "create file set" > + > +# Make a bunch of small files in a directory. This is designed to expand > +# the filesystem tree to something more than zero levels. > +mkdir $SCRATCH_MNT/files > +for i in `seq -w 0 $maxfiles`; > +do > +dd status=none if=/dev/zero of=$SCRATCH_MNT/files/file$i bs=4096 count=4 > +done $XFS_IO_PROG -f -c "pwrite 0 16384" $SCRATCH_MNT/files/file$i > /dev/null > + > +# create a snapshot of what we just did > +$BTRFS_UTIL_PROG fi sy $SCRATCH_MNT > +$BTRFS_UTIL_PROG su sna $SCRATCH_MNT $SCRATCH_MNT/snap1 > +mv $SCRATCH_MNT/snap1/files $SCRATCH_MNT/snap1/old You need to filter the output. i.e. _filter_scratch > +# same thing as before but on the snapshot. this way we can generate > +# some exclusively owned tree nodes. > +echo "create file set on snapshot" > +mkdir $SCRATCH_MNT/snap1/files > +for i in `seq -w 0 $maxfiles`; > +do > +dd status=none if=/dev/zero of=$SCRATCH_MNT/snap1/files/file$i bs=4096 > count=4 > +done Same again. > + > +# Enable qgroups now that we have our filesystem prepared. This > +# will kick off a scan which we will have to wait for below. > +$BTRFS_UTIL_PROG qu en $SCRATCH_MNT > +sleep 30 That seems rather arbitrary. The sleeps you are adding add well over a minute to the runtime, and a quota scan of a filesystem with 200 files should be almost instantenous. > +_scratch_unmount > +_scratch_mount What is the purpose of this? > +# Ok, delete the snapshot we made previously. Since btrfs drop > +# snapshot is a delayed action with no way to force it, we have to > +# impose another sleep here. > +$BTRFS_UTIL_PROG su de $SCRATCH_MNT/snap1 > +sleep 45 That's indicative of a bug, yes? > +_scratch_unmount > + > +# generate a qgroup report and look for inconsistent groups > +$BTRFS_UTIL_PROG check --qgroup-report $SCRATCH_DEV | grep -q "Counts for > qgroup.*different" > +RETVAL=$? > +if [ $RETVAL -eq 0 ]; then > +status=1 > +fi RETVAL! Get your RETVAL here! RETVAL! No need to shout ;) > new file mode 100644 > index 000..b8a146c > --- /dev/null > +++ b/tests/btrfs/057.out > @@ -0,0 +1,7 @@ > +QA output created by 057 > +create file set > +FSSync '/xfstest2' > +Create a snapshot of '/xfstest2' in '/xfstest2/snap1' > +create file set on snapshot > +Transaction commit: none (default) > +Delete subvolume '/xfstest2/snap1' The scratch mountpoint output is what requires filtering - it's different for everyone, and so needs to anonymised to SCRATCH_MNT > diff --git a/tests/btrfs/group b/tests/btrfs/group > index 2da7127..ebc38c5 100644 > --- a/tests/btrfs/group > +++ b/tests/btrfs/group > @@ -59,3 +59,4 @@ > 054 auto quick > 055 auto quick > 056 auto quick > +057 auto quick "quick" means the test takes less than a few seconds to execute. This test takes a couple of minutes, so it should not be in the quick group. Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] generic/017: skip invalid block sizes for btrfs
On Mon, Jun 23, 2014 at 04:09:18PM +0200, Lukáš Czerner wrote: > On Mon, 23 Jun 2014, Lukáš Czerner wrote: > > > Date: Mon, 23 Jun 2014 14:35:50 +0200 (CEST) > > From: Lukáš Czerner > > To: Filipe David Borba Manana > > Cc: fste...@vger.kernel.org, linux-btrfs@vger.kernel.org > > Subject: Re: [PATCH] generic/017: skip invalid block sizes for btrfs > > > > On Mon, 23 Jun 2014, Filipe David Borba Manana wrote: > > > > > Date: Mon, 23 Jun 2014 11:28:00 +0100 > > > From: Filipe David Borba Manana > > > To: fste...@vger.kernel.org > > > Cc: linux-btrfs@vger.kernel.org, > > > Filipe David Borba Manana > > > Subject: [PATCH] generic/017: skip invalid block sizes for btrfs > > > > > > In btrfs the block size (called sector size in btrfs) can not be > > > smaller then the page size. Therefore skip block sizes smaller > > > then page size if the fs is btrfs, so that the test can succeed > > > on btrfs (testing only with block sizes of 4kb on systems with a > > > page size of 4Kb). > > > > The test itself is wrong, it's trying to do _scratch_mkfs with > > different block size, but the block size might already be specified > > by the user (in fact it should be user responsibility to test > > different block sizes). In the case that mkfs can not handle > > multiple of the same option like mkfs.xfs for example it will fail, > > but the test will go on with the original file system. > > > > The test needs to be fixed to just test the file system with options > > specified by the user. Also we should change _scratch_mkfs() to fail > > the test if the mkfs failed (no one is actually testing mkfs_status > > variable anyway. > > Correction, _scratch_mkfs_xfs() is actually testing mkfs_status and > will attempt to re-run mkfs only with provided options if it failed > before. But my point remains the same, block size to test should be > in users hands and we should run all tests with different block > sizes, if supported. Follow that line of reasoning to other options. If we take that argument to it's logical conclusion, no test should be able to set any mount or mkfs option because that's for the user to control. This implies we can't even have quota specific tests because they need to override the mount options (and perhaps mkfs options) the user has specified to be properly tested. Some tests only work on specific configurations and therefore they need to be able to control the execution environment directly. Hence the behaviour of _scratch_mkfs_xfs(), where it will *attempt* to use the user provided options. However, if they conflict with what the test requires it will drop the user options and use what the test requires. It's better that the test overrides user provided options than fail due to incompatible configuration. It makes maintenance of the tests much easier because we don't have to declare and maintain the supported list of user options for every test. Either the test works for all configurations (i.e. whatever the user sets in MKFS/MOUNT_OPTIONS), or it specifically defines the configuration it is testing. Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [systemd-devel] Slow startup of systemd-journal on BTRFS
On Mon, Jun 16, 2014 at 12:13:07AM +0200, Lennart Poettering wrote: > On Sat, 14.06.14 09:52, Goffredo Baroncelli (kreij...@libero.it) wrote: > > > > Which effectively means that by the time the 8 MiB is filled, each 4 KiB > > > block has been rewritten to a new location and is now an extent unto > > > itself. So now that 8 MiB is composed of 2048 new extents, each one a > > > single 4 KiB block in size. > > > > Several people pointed fallocate as the problem. But I don't > > understand the reason. > > BTW, the reason we use fallocate() in journald is not about trying to > optimize anything. It's only used for one reason: to avoid SIGBUS on > disk/quota full, since we actually write everything to the files using > mmap(). FWIW, fallocate() doesn't absolutely guarantee you that. When at ENOSPC, a write into that reserved range can still require un-reserved metadata blocks to be allocated. e.g. splitting a "reserved" data extent into two extents (used and reserved) requires an extra btree record, which can cause a split, which can require allocation. This tends to be pretty damn rare, though, and some filesystems have reserved block pools specifically for handling this sort of ENOSPC corner case. Hence, in practice the filesystems never actually fail with ENOSPC in ranges that have been fallocate()d. > I mean, writing things with mmap() is always problematic, and > handling write errors is awfully difficult, but at least two of the most > common reasons for failure we'd like protect against in advance, under > the assumption that disk/quota full will be reported immediately by the > fallocate(), and the mmap writes later on will then necessarily succeed. > > I am not really following though why this trips up btrfs though. I am > not sure I understand why this breaks btrfs COW behaviour. I mean, > fallocate() isn't necessarily supposed to write anything really, it's > mostly about allocating disk space in advance. I would claim that > journald's usage of it is very much within the entire reason why it > exists... > > Anyway, happy to change these things around if necesary, but first I'd > like to have a very good explanation why fallocate() wouldn't be the > right thing to invoke here, and a suggestion what we should do instead > to cover this usecase... fallocate() of 8MB should be more than sufficient for non-COW filesystems - 1MB would be enough to prevent performance degradation due to fragmentation in most cases. The current problems seem to be with the way btrfs does rewrites, not the use of fallocate() in systemd. Thanks for explanation, Lennart. Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] xfstests/btrfs: add qgroup rescan stress test
On Wed, Jun 18, 2014 at 04:36:22PM +0800, Wang Shilong wrote: > Hello Josef, > > The lastest Qgroup code still break this test sometimes. > > Ps: this test seems not merging into xfstests. Then repost it to fste...@vger.kernel.org. Sometimes patches get missed... Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: R: Re: Slow startup of systemd-journal on BTRFS
On Thu, Jun 12, 2014 at 12:37:13PM +, Duncan wrote: > Goffredo Baroncelli posted on Thu, 12 Jun 2014 > 13:13:26 +0200 as excerpted: > > >>systemd has a very stupid journal write pattern. It checks if there is > >>space in the file for the write, and if not it fallocates the small > >>amount of space it needs (it does *4 byte* fallocate calls!) and then > >>does the write to it. All this does is fragment the crap out of the log > >>files because the filesystems cannot optimise the allocation patterns. > > > > I checked the code, and to me it seems that the fallocate() are done in > > FILE_SIZE_INCREASE unit (actually 8MB). > > FWIW, either 4 byte or 8 MiB fallocate calls would be bad, I think > actually pretty much equally bad without NOCOW set on the file. So maybe it's been fixed in systemd since the last time I looked. Yup: http://cgit.freedesktop.org/systemd/systemd/commit/src/journal/journal-file.c?id=eda4b58b50509dc8ad0428a46e20f6c5cf516d58 The reason it was changed? To "save a syscall per append", not to prevent fragmentation of the file, which was the problem everyone was complaining about... > Why? Because btrfs data blocks are 4 KiB. With COW, the effect for > either 4 byte or 8 MiB file allocations is going to end up being the > same, forcing (repeated until full) rewrite of each 4 KiB block into its > own extent. And that's now a btrfs problem :/ Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Slow startup of systemd-journal on BTRFS
On Thu, Jun 12, 2014 at 11:21:04AM +1000, Dave Chinner wrote: > On Wed, Jun 11, 2014 at 11:28:54PM +0200, Goffredo Baroncelli wrote: > > Hi all, > > > > I would like to share a my experience about a slowness of systemd when used > > on BTRFS. > > > > My boot time was very high (about ~50 seconds); most of time it was due to > > NetworkManager which took about 30-40 seconds to start (this data came from > > "systemd-analyze plot"). > > > > I make several attempts to address this issue. Also I noticed that sometime > > this problem disappeared; but I was never able to understand why. > > > > However this link > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1006386 > > > > suggested me that the problem could be due to a bad interaction between > > systemd and btrfs. NetworkManager was innocent. > > systemd has a very stupid journal write pattern. It checks if there > is space in the file for the write, and if not it fallocates the > small amount of space it needs (it does *4 byte* fallocate calls!) > and then does the write to it. All this does is fragment the crap > out of the log files because the filesystems cannot optimise the > allocation patterns. > > Yup, it fragments journal files on XFS, too. > > http://oss.sgi.com/archives/xfs/2014-03/msg00322.html > > IIRC, the systemd developers consider this a filesystem problem and > so refused to change the systemd code to be nice to the filesystem > allocators, even though they don't actually need to use fallocate... BTW, the systemd list is subscriber only, so thay aren't going to see anything that we comment on from a cross-post to the btrfs list. Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Slow startup of systemd-journal on BTRFS
On Wed, Jun 11, 2014 at 11:28:54PM +0200, Goffredo Baroncelli wrote: > Hi all, > > I would like to share a my experience about a slowness of systemd when used > on BTRFS. > > My boot time was very high (about ~50 seconds); most of time it was due to > NetworkManager which took about 30-40 seconds to start (this data came from > "systemd-analyze plot"). > > I make several attempts to address this issue. Also I noticed that sometime > this problem disappeared; but I was never able to understand why. > > However this link > > https://bugzilla.redhat.com/show_bug.cgi?id=1006386 > > suggested me that the problem could be due to a bad interaction between > systemd and btrfs. NetworkManager was innocent. systemd has a very stupid journal write pattern. It checks if there is space in the file for the write, and if not it fallocates the small amount of space it needs (it does *4 byte* fallocate calls!) and then does the write to it. All this does is fragment the crap out of the log files because the filesystems cannot optimise the allocation patterns. Yup, it fragments journal files on XFS, too. http://oss.sgi.com/archives/xfs/2014-03/msg00322.html IIRC, the systemd developers consider this a filesystem problem and so refused to change the systemd code to be nice to the filesystem allocators, even though they don't actually need to use fallocate... Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] xfstests: add test for btrfs clone + fsync durability
On Mon, Jun 09, 2014 at 03:48:52AM +0100, Filipe David Borba Manana wrote:
> Regression test for btrfs ioctl clone operation + fsync + log
> recovery. The issue was that doing an fsync after cloning into
> a file didn't gave any persistence guarantees as it should.
> What happened was that the in memory metadata (extent maps)
> weren't updated, which made the fsync code not able to detect
> that file data has been changed and must be persisted to the
> log.
>
> This issue is fixed by the following linux kernel btrfs patch:
>
> Btrfs: make fsync work after cloning into a file
>
> Signed-off-by: Filipe David Borba Manana
> ---
>
> V2: Test small files too, consisting of a single inline extent, as
> it triggers different code paths.
>
> tests/btrfs/056 | 150
>
> tests/btrfs/056.out | 129
> tests/btrfs/group | 1 +
> 3 files changed, 280 insertions(+)
> create mode 100755 tests/btrfs/056
> create mode 100644 tests/btrfs/056.out
>
> diff --git a/tests/btrfs/056 b/tests/btrfs/056
> new file mode 100755
> index 000..e066442
> --- /dev/null
> +++ b/tests/btrfs/056
> @@ -0,0 +1,150 @@
> +#! /bin/bash
> +# FS QA Test No. btrfs/056
> +#
> +# Regression test for btrfs ioctl clone operation + fsync + log recovery.
> +# The issue was that doing an fsync after cloning into a file didn't gave any
> +# persistence guarantees as it should. What happened was that the in memory
> +# metadata (extent maps) weren't updated, which made the fsync code not able
> +# to detect that file data has been changed.
> +#
> +# This issue is fixed by the following linux kernel btrfs patch:
> +#
> +#Btrfs: make fsync work after cloning into a file
> +#
> +#---
> +# Copyright (c) 2014 Filipe Manana. 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
> +#---
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +tmp=/tmp/$$
> +status=1 # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> + _cleanup_flakey
> + rm -fr $tmp
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/dmflakey
> +
> +# real QA test starts here
> +_supported_fs btrfs
> +_supported_os Linux
> +_require_scratch
> +_require_btrfs_cloner
> +_require_btrfs_fs_feature "no_holes"
> +_require_btrfs_mkfs_feature "no-holes"
> +_require_dm_flakey
> +_need_to_be_root
> +
> +rm -f $seqres.full
> +
> +test_btrfs_clone_fsync_log_recover()
> +{
> + _scratch_mkfs "$1" >/dev/null 2>&1
> + _init_flakey
> + SAVE_MOUNT_OPTIONS="$MOUNT_OPTIONS"
> + MOUNT_OPTIONS="$MOUNT_OPTIONS $2"
> + _mount_flakey
> +
> + # Create a file with 4 extents and 1 hole, all with a size of 8Kb each.
> + # The hole is in the range [16384, 24576[.
> + $XFS_IO_PROG -f -c "pwrite -S 0x01 -b 8192 0 8192" \
> + -c "fsync" \
> + -c "pwrite -S 0x02 -b 8192 8192 8192" \
> + -c "fsync" \
> + -c "pwrite -S 0x04 -b 8192 24576 8192" \
> + -c "fsync" \
> + -c "pwrite -S 0x05 -b 8192 32768 8192" \
> + -c "fsync" \
> + $SCRATCH_MNT/foo | _filter_xfs_io
> +
> + # Clone destination file, 1 extent of 96kb.
> + $XFS_IO_PROG -f -c "pwrite -S 0xff -b 98304 0 98304" -c "fsync" \
> + $SCRATCH_MNT/bar | _filter_xfs_io
I've seen this pattern before ;)
Use the "-s" option to xfs_io rather than repeated "pwrite/fsync"
pairs.
Otherwise looks fine.
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] xfstests: add test for btrfs cloning with file holes
r $tmp
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs btrfs
> +_supported_os Linux
> +_require_scratch
> +_require_btrfs_cloner
> +_require_btrfs_fs_feature "no_holes"
> +_require_btrfs_mkfs_feature "no-holes"
Nice kernel/userspace consistency in naming there ;)
> +_need_to_be_root
> +
> +rm -f $seqres.full
> +
> +test_btrfs_clone_with_holes()
> +{
> + _scratch_mkfs "$1" >/dev/null 2>&1
> + _scratch_mount
> +
> + # Create a file with 4 extents and 1 hole, all with a size of 8Kb each.
> + $XFS_IO_PROG -f -c "pwrite -S 0x01 -b 8192 0 8192" $SCRATCH_MNT/foo \
> + | _filter_xfs_io
> + sync
> + $XFS_IO_PROG -c "pwrite -S 0x02 -b 8192 8192 8192" $SCRATCH_MNT/foo \
> + | _filter_xfs_io
> + sync
> + # After the following write we get an hole in the range [16384, 24576[
> + $XFS_IO_PROG -c "pwrite -S 0x04 -b 8192 24576 8192" $SCRATCH_MNT/foo \
> + | _filter_xfs_io
> + sync
> + $XFS_IO_PROG -c "pwrite -S 0x05 -b 8192 32768 8192" $SCRATCH_MNT/foo \
> + | _filter_xfs_io
> + sync
> +
> + # Clone destination file, 1 extent of 96kb.
> + $XFS_IO_PROG -f -c "pwrite -S 0xff -b 98304 0 98304" $SCRATCH_MNT/bar \
> + | _filter_xfs_io
> + sync
As Lukas mentioned, this is way too verbose and hard to read.
This:
# Create a file with 4 extents and 1 hole, all with a size of 8Kb each.
$XFS_IO_PROG -fs \
-c "pwrite -S 0x01 -b 8192 0 8192" \
-c "pwrite -S 0x02 -b 8192 8192 8192" \
-c "pwrite -S 0x04 -b 8192 24576 8192" \
-c "pwrite -S 0x05 -b 8192 32768 8192" \
$SCRATCH_MNT/foo | _filter_xfs_io
# Clone destination file, 1 extent of 96kb.
$XFS_IO_PROG -fs -c "pwrite -S 0xff -b 98304 0 98304" \
$SCRATCH_MNT/bar | _filter_xfs_io
is much more compact and far easier to read. It's also the same
style as many of the other tests use for compound operations like
this.
Other than that the test look sgood.
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs: add regression test for remount with thread_pool resized
On Fri, May 30, 2014 at 04:52:51PM +0800, Xing Gu wrote:
> Regression test for resizing 'thread_pool' when remount the fs.
Ping for btrfs test reviewers - is this test useful at all?
> Signed-off-by: Xing Gu
> ---
> tests/btrfs/055 | 55
> +
> tests/btrfs/055.out | 1 +
> tests/btrfs/group | 1 +
> 3 files changed, 57 insertions(+)
> create mode 100755 tests/btrfs/055
> create mode 100644 tests/btrfs/055.out
>
> diff --git a/tests/btrfs/055 b/tests/btrfs/055
> new file mode 100755
> index 000..0a0dd34
> --- /dev/null
> +++ b/tests/btrfs/055
> @@ -0,0 +1,55 @@
> +#!/bin/bash
> +# FS QA Test No. btrfs/055
> +#
> +# Regression test for resizing 'thread_pool' when remount the fs.
> +#
> +#---
> +# Copyright (c) 2014 Fujitsu. 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
> +#
> +#---
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1 # failure is the default!
> +
> +_cleanup()
> +{
> +rm -f $tmp.*
> +}
> +
> +trap "_cleanup ; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs btrfs
> +_supported_os Linux
> +_require_scratch
> +
> +_scratch_mkfs > /dev/null 2>&1
> +
> +_scratch_mount "-o thread_pool=6"
> +
> +_scratch_mount "-o remount,thread_pool=10"
> +
> +status=0 ; exit
> diff --git a/tests/btrfs/055.out b/tests/btrfs/055.out
> new file mode 100644
> index 000..2fdd8f4
> --- /dev/null
> +++ b/tests/btrfs/055.out
> @@ -0,0 +1 @@
> +QA output created by 055
> diff --git a/tests/btrfs/group b/tests/btrfs/group
> index b668485..2c10c5b 100644
> --- a/tests/btrfs/group
> +++ b/tests/btrfs/group
> @@ -57,3 +57,4 @@
> 052 auto quick
> 053 auto quick
> 054 auto quick
> +055 auto quick
> --
> 1.9.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 00/32] making inode time stamps y2038 ready
On Tue, Jun 03, 2014 at 04:22:19PM +0200, Arnd Bergmann wrote: > On Monday 02 June 2014 14:57:26 H. Peter Anvin wrote: > > On 06/02/2014 12:55 PM, Arnd Bergmann wrote: > The possible uses I can see for non-ktime_t types in the kernel are: > * inodes need 96 bit timestamps to represent the full range of values > that can be stored in a file system, you made a convincing argument > for that. Almost everything else can fit into 64 bit on a 32-bit > kernel, in theory also on a 64-bit kernel if we want that. Just ot be pedantic, inodes don't *need* 96 bit timestamps - some filesystems can *support up to* 96 bit timestamps. If the kernel only supports 64 bit timestamps and that's all the kernel can represent, then the upper bits of the 96 bit on-disk inode timestamps simply remain zero. If you move the filesystem between kernels with different time ranges, then the filesystem needs to be able to tell the kernel what it's supported range is. This is where having the VFS limit the range of supported timestamps is important: the limit is the min(kernel range, filesystem range). This allows the filesystems to be indepenent of the kernel time representation, and the kernel to be independent of the physical filesystem time encoding Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] xfstests: master branch updated to 45d1fac
Hi folks, The xfstests repository at git://oss.sgi.com/xfs/cmds/xfstests has just been updated. Patches often get missed, so please check if your outstanding patches were in this update. If they have not been in this update, please resubmit them to fste...@vger.kernel.org so they can be picked up in the next update. I've sent this announcement to fstests, linux-fsdevel and the XFS, ext4 and btrfs lists. Future fstests announcements will only be directed to the fstests and linux-fsdevel lists. The new head of the master branch is commit: 45d1fac btrfs: test for btrfs send when nested subvols/snapshots exist New Commits: Eryu Guan (2): [82afae2] generic: new ENOSPC regression test [caca581] xfs/005: filter _scratch_mount output to match golden image Filipe David Borba Manana (5): [e7dd9f1] btrfs: add test for btrfs send with long paths [86da66c] common: add helper require function _require_btrfs_cloner [d140277] btrfs: add test for clone operation [3318629] btrfs: add test for btrfs send with large xattrs [45d1fac] btrfs: test for btrfs send when nested subvols/snapshots exist Code Diffstat: common/rc | 7 + tests/btrfs/035 | 4 +- tests/btrfs/051 | 85 + tests/btrfs/051.out | 1 + tests/btrfs/052 | 171 + tests/btrfs/052.out | 499 ++ tests/btrfs/053 | 109 +++ tests/btrfs/053.out | 1 + tests/btrfs/054 | 109 +++ tests/btrfs/054.out | 1 + tests/btrfs/group | 4 + tests/generic/027 | 107 +++ tests/generic/027.out | 2 + tests/generic/group | 1 + tests/xfs/005 | 7 +- 15 files changed, 1104 insertions(+), 4 deletions(-) create mode 100755 tests/btrfs/051 create mode 100644 tests/btrfs/051.out create mode 100755 tests/btrfs/052 create mode 100644 tests/btrfs/052.out create mode 100755 tests/btrfs/053 create mode 100644 tests/btrfs/053.out create mode 100755 tests/btrfs/054 create mode 100644 tests/btrfs/054.out create mode 100755 tests/generic/027 create mode 100644 tests/generic/027.out -- Dave Chinner da...@fromorbit.com signature.asc Description: Digital signature
Re: [ANNOUNCE] xfstests: new mailing list
On Mon, May 19, 2014 at 07:55:41AM -0700, Christoph Hellwig wrote: > On Sat, May 17, 2014 at 08:19:30AM +1000, Dave Chinner wrote: > > Renaming the test suite take a lot more work - .e.g renaming/moving > > source trees and a fixing all the documentation that points to it... > > In that case please call the list xfstests - a name different by a > single character is utterly confusing. And I defintively see some merit > to the suggestion that we'll just keep the x and allow people to come up > with a nice backronym for it if they care enough. What is important is that we have a separate list for the filesystem test suite we use, not whether the name has an "x" in or not. Arguing about whether there should or should not be an 'x' in the mailing list name is just a waste of time - it's not going to make me change the name of the list Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] xfstests: new mailing list
On Fri, May 16, 2014 at 01:53:20AM -0700, Christoph Hellwig wrote: > On Fri, May 16, 2014 at 10:48:42AM +0200, Luk?? Czerner wrote: > > > > As requested I've created a new mailing list for xfstests > > > > development and discussion. Reflecting the fact that the test > > > > harness is not really XFS specific anymore, the list is: > > > > > > > > fste...@vger.kernel.org > > > > > > Isn't there an "x" missing somewhere? > > > > It's intentional and it is "Reflecting the fact that the test > > harness is not really XFS specific anymore", even though the test > > suite itself keep the name xfstests. > > > > This way it's more obvious to people that this is in fact not xfs > > specific. > > Having the name different from the project it is for is stupid. Either > rename the test suite, or use the same name for the mailing list. Renaming the test suite take a lot more work - .e.g renaming/moving source trees and a fixing all the documentation that points to it... So, small steps. Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] xfstests: new mailing list
Hi folks, As requested I've created a new mailing list for xfstests development and discussion. Reflecting the fact that the test harness is not really XFS specific anymore, the list is: fste...@vger.kernel.org I have not confirmed an archiving location yet (in the process of doing so through marc.info), but that should be set up real soon. I'll still be listening in on the XFS list for xfstests patches, bug reports and questions, so don't worry if you forget this new list exists... :) Cheers, Dave. -- Dave Chinner da...@fromorbit.com signature.asc Description: Digital signature
[ANNOUNCE] xfstests: master branch updated to 10fd79a
Hi folks, The xfstests repository at git://oss.sgi.com/xfs/cmds/xfstests has just been updated. Patches often get missed, so please check if your outstanding patches were in this update. If they have not been in this update, please resubmit them to x...@oss.sgi.com so they can be picked up in the next update. The new head of the master branch is commit: 10fd79a locktest: add a F_GETLK vs. openmode test New Commits: Brian Foster (5): [2a52b8f] xfs/030: filter out extra repair noise for finobt enabled fs' [9d5cb63] common: add _require_xfs_[mkfs_]finobt() checks for finobt tests [01fbf44] repair: filter agno/ino repair output for finobt [d95ecb0] xfs/010: test repair for finobt corruption [c141570] xfs/013: stress the free inode btree Eryu Guan (1): [ec08236] common: new function to get real device path name and basename Jeff Layton (4): [2baab36] locktest: don't assume that F_OPEN should use O_RDWR [ece564a] locktest: set f_fd to INVALID_HANDLE on close [fe7056d] locktest: consolidate do_lock and do_unlock, and add ability to F_GETLK [10fd79a] locktest: add a F_GETLK vs. openmode test Josef Bacik (2): [0fd4705] common: fix flink check [b6689ad] config: fix selinux context handling Namjae Jeon (1): [995a459] fsstress: fix incorrect if condition check for collapse range mode Theodore Ts'o (1): [9f35155] check: add support for an external file containing tests to exclude Wang Shilong (1): [49398f8] btrfs: add regression test for inode cache vs tree log h...@infradead.org (1): [7f82a5f] common: use a relative path to fsstress Code Diffstat: check | 6 ++ common/config | 18 +++--- common/rc | 36 common/repair | 2 + ltp/fsstress.c | 2 +- src/locktest.c | 125 +++--- tests/btrfs/049 | 109 tests/btrfs/049.out | 1 + tests/btrfs/group | 1 + tests/ext4/305 | 2 +- tests/generic/009 | 2 +- tests/generic/019 | 4 +- tests/generic/285 | 2 +- tests/generic/312 | 2 +- tests/xfs/010 | 133 tests/xfs/010.out | 57 +++ tests/xfs/013 | 155 tests/xfs/013.out | 7 +++ tests/xfs/030 | 3 +- tests/xfs/group | 2 + 20 files changed, 582 insertions(+), 87 deletions(-) create mode 100644 tests/btrfs/049 create mode 100644 tests/btrfs/049.out create mode 100755 tests/xfs/010 create mode 100644 tests/xfs/010.out create mode 100755 tests/xfs/013 create mode 100644 tests/xfs/013.out -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstests: fix flink test
On Wed, May 07, 2014 at 04:54:47PM -0400, Josef Bacik wrote: > I don't have flink support in my xfsprogs, but it doesn't fail with "command > not > found" or whatever, it fails because I don't have the -T option. So fix > _require_xfs_io_command to check for an invalid option and not run. This way > I > get notrun instead of a failure. Thanks, > > Signed-off-by: Josef Bacik > --- > common/rc | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/common/rc b/common/rc > index 5c13db5..4fa7e63 100644 > --- a/common/rc > +++ b/common/rc > @@ -1258,6 +1258,8 @@ _require_xfs_io_command() > _notrun "xfs_io $command support is missing" > echo $testio | grep -q "Operation not supported" && \ > _notrun "xfs_io $command failed (old kernel/wrong fs?)" > + echo $testio | grep -q "invalid option" && \ > + _notrun "xfs_io $command support is missing" > } Yeah, looks like it throws a different error - it treats -T as an option, not a command. Rather than multiple checks that result in the same error, why not just: - echo $testio | grep -q "not found" && \ + echo $testio | egrep -q 'not found|invalid option' && \ _notrun "xfs_io $command support is missing" Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] xfstests: master branch updated to ce0aa2b
Hi folks, The xfstests repository at git://oss.sgi.com/xfs/cmds/xfstests has just been updated. Patches often get missed, so please check if your outstanding patches were in this update. If they have not been in this update, please resubmit them to x...@oss.sgi.com so they can be picked up in the next update. The new head of the master branch is commit: ce0aa2b common: Use _require_xfs_io_command() instead of helpers New Commits: Dave Chinner (6): [31a50c7] generic/204: tweak reserve pool size [ca93123] generic: cleanup space after test in TESTDIR [9d7807d] xfs: remove dmapi tests from the auto group [03227f9] filter: xfs_repair emits new corruption messagse [a12a56f] shared/051: remove ACL count subtest [a841a6d] generic: introduce new large ACL test Lukas Czerner (1): [ce0aa2b] common: Use _require_xfs_io_command() instead of helpers Code Diffstat: common/attr | 31 common/defrag | 2 +- common/rc | 41 --- common/repair | 3 ++ tests/btrfs/026 | 2 +- tests/btrfs/027 | 2 +- tests/btrfs/028 | 2 +- tests/btrfs/047 | 2 +- tests/ext4/001| 2 +- tests/ext4/002| 2 +- tests/generic/009 | 2 +- tests/generic/012 | 8 +-- tests/generic/016 | 8 +-- tests/generic/017 | 4 +- tests/generic/021 | 8 +-- tests/generic/022 | 8 +-- tests/generic/026 | 132 tests/generic/026.out | 9 tests/generic/070 | 1 + tests/generic/204 | 2 - tests/generic/213 | 2 +- tests/generic/214 | 2 +- tests/generic/223 | 2 +- tests/generic/228 | 2 +- tests/generic/255 | 6 +-- tests/generic/256 | 2 +- tests/generic/274 | 2 +- tests/generic/300 | 4 +- tests/generic/311 | 2 +- tests/generic/312 | 2 +- tests/generic/316 | 4 +- tests/generic/group | 1 + tests/shared/051 | 45 - tests/shared/051.out | 135 -- tests/shared/298 | 2 +- tests/xfs/252 | 4 +- tests/xfs/290 | 2 +- tests/xfs/292 | 2 + tests/xfs/group | 50 +-- 39 files changed, 249 insertions(+), 293 deletions(-) create mode 100644 tests/generic/026 create mode 100644 tests/generic/026.out -- Dave Chinner da...@fromorbit.com signature.asc Description: Digital signature
Re: [PATCH] xfstests: btrfs, test send's ability to punch holes and prealloc extents
On Wed, Apr 16, 2014 at 03:39:18PM +0100, Filipe David Manana wrote:
> On Wed, Apr 16, 2014 at 1:23 AM, Dave Chinner wrote:
> > On Tue, Apr 15, 2014 at 05:43:21PM +0100, Filipe David Borba Manana wrote:
> >> This test verifies that after an incremental btrfs send the replicated
> >> file has
> >> the same exact hole and data structure as in the origin filesystem. This
> >> didn't
> >> use to be the case before the send stream version 2 - holes were sent as
> >> write
> >> operations of 0 valued bytes instead of punching holes with the fallocate
> >> system
> >> call, and pre-allocated extents were sent as well as write operations of 0
> >> valued
> >> bytes instead of intructions for the receiver to use the fallocate system
> >> call.
> >> Also checks that prealloc extents that lie beyond the file's size are
> >> replicated
> >> by an incremental send.
> >
> > Can you wrap commit messages at 68 columns?
> >
> >
> >> +md5sum $SCRATCH_MNT/mysnap2/foo | _filter_scratch
> >> +# List all hole and data segments.
> >> +$XFS_IO_PROG -r -c "seek -r -a 0" $SCRATCH_MNT/mysnap2/foo
> >> +# List all extents, we're interested here in prealloc extents that lie
> >> beyond
> >> +# the file's size.
> >> +$XFS_IO_PROG -r -c "fiemap -l" $SCRATCH_MNT/mysnap2/foo | _filter_scratch
> >
> > That dumps raw block numbers into the golden output. _filter_fiemap
> > is probably needed here.
>
> Hum, just tried it and uploaded a v2.
> However I'm now noticing it doesn't do everything I had in mind.
> _filter_fiemap is not showing the extents falloc -k created, only a
> collapsed range of holes.
>
> So my intention is to verify not just holes, but also the extents
> created by 'falloc -k'. The following filter I just made locally gives
> me that:
>
> _filter_all_fiemap()
> {
> awk --posix '
> $3 ~ /hole/ {
> print $1, $2, $3;
> next;
> }
> $3 ~ /[[:xdigit:]]*..[[:xdigit:]]/ {
> print $1, $2, "extent";
> next;
> }'
> }
Which is effectively _filter_hole_fiemap(), except it coalesces
adjacent extents into a single range.
I'd suggest moving the _filter_* functions from common/punch to
common/filter, and using _filter_hole_fiemap() as there's no
guarantee that you'll get individual extents for each falloc -k
range - they coul dbe allocated contiguously, and hence the number
of extents reported can change from run to run. That's the reason
why the filters coalesce adjacent file offsets of the same type - we
care whether the range of the file contains the correct extent type,
not how fragmented the range is
> (nicely printed/indented at
> https://friendpaste.com/1JtG5bts2Sz0LWhUutCpzE, as e-mail is not good
> for code pasting)
Pasting code works fine for me ;)
> Which gives me:
>
> 0: [0..191]: extent
> 1: [192..199]: extent
> 2: [200..231]: extent
> 3: [232..239]: extent
> 4: [240..287]: extent
> 5: [288..295]: extent
> 6: [296..487]: extent
> 7: [488..495]: extent
> 8: [496..519]: hole
> 9: [520..527]: extent
> 10: [528..583]: extent
> 11: [584..591]: extent
> 12: [592..2543]: extent
> 13: [2544..17575]: hole
> 14: [17576..21487]: extent
Also, you're trimming of the block count, so you can drop the "-l"
option to the fiemap command
> Versus only (from _filter_fiemap):
>
> 0: [496..17575]: hole
Maybe the "-l" option is confusing the filter, it should be giving:
0: [0..495]: data
1: [496..519]: hole
2: [520..2543]: data
3: [2544..17575]: hole
4: [17576..21487]: data
Though if there are unwritten extents, it will say "unwritten"
rather than "data". _filter_hole_fiemap should give:
0: [0..495]: extent
1: [496..519]: hole
2: [520..2543]: extent
3: [2544..17575]: hole
4: [17576..21487]: extent
Which tells you that everything you asked for was allocated...
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstests: btrfs, add test for btrfs properties
On Tue, Apr 15, 2014 at 08:50:24PM +0100, Filipe David Borba Manana wrote:
> This test case verifies the btrfs properties feature, a new feature
> introduced in the linux kernel version 3.14.
>
> Signed-off-by: Filipe David Borba Manana
...
> ---
> common/rc | 9 +++
> tests/btrfs/048 | 220
>
> tests/btrfs/048.out | 78 +++
> tests/btrfs/group | 1 +
> 4 files changed, 308 insertions(+)
> create mode 100755 tests/btrfs/048
> create mode 100644 tests/btrfs/048.out
>
> diff --git a/common/rc b/common/rc
> index acf419b..d4ba74f 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2262,6 +2262,15 @@ _run_btrfs_util_prog()
> run_check $BTRFS_UTIL_PROG $*
> }
>
> +_require_btrfs_properties()
> +{
> + $BTRFS_UTIL_PROG | grep 'btrfs property ' > /dev/null 2>&1
> + if [ $? -ne 0 ]
> + then
> + _notrun "Missing btrfs-progs with properties support, skipped
> this test"
> + fi
if [ ... ]; then
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here="`pwd`"
> +tmp=/tmp/$$
> +
> +status=1 # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +send_files_dir=$TEST_DIR/btrfs-test-$seq
You should define this after including the common/* files.
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstests: btrfs, test send's ability to punch holes and prealloc extents
On Tue, Apr 15, 2014 at 05:43:21PM +0100, Filipe David Borba Manana wrote: > This test verifies that after an incremental btrfs send the replicated file > has > the same exact hole and data structure as in the origin filesystem. This > didn't > use to be the case before the send stream version 2 - holes were sent as write > operations of 0 valued bytes instead of punching holes with the fallocate > system > call, and pre-allocated extents were sent as well as write operations of 0 > valued > bytes instead of intructions for the receiver to use the fallocate system > call. > Also checks that prealloc extents that lie beyond the file's size are > replicated > by an incremental send. Can you wrap commit messages at 68 columns? > +md5sum $SCRATCH_MNT/mysnap2/foo | _filter_scratch > +# List all hole and data segments. > +$XFS_IO_PROG -r -c "seek -r -a 0" $SCRATCH_MNT/mysnap2/foo > +# List all extents, we're interested here in prealloc extents that lie beyond > +# the file's size. > +$XFS_IO_PROG -r -c "fiemap -l" $SCRATCH_MNT/mysnap2/foo | _filter_scratch That dumps raw block numbers into the golden output. _filter_fiemap is probably needed here. > +md5sum $SCRATCH_MNT/mysnap2/foo | _filter_scratch > +# List all hole and data segments. > +$XFS_IO_PROG -r -c "seek -r -a 0" $SCRATCH_MNT/mysnap2/foo > +# List all extents, we're interested here in prealloc extents that lie beyond > +# the file's size. > +$XFS_IO_PROG -r -c "fiemap -l" $SCRATCH_MNT/mysnap2/foo | _filter_scratch Same here. Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] xfstests: master branch updated to 8874560
On Mon, Apr 14, 2014 at 10:49:38AM +1000, Dave Chinner wrote: > Hi folks, > > The xfstests repository at git://oss.sgi.com/xfs/cmds/xfstests has > just been updated. Patches often get missed, so please check if your > outstanding patches were in this update. If they have not been in > this update, please resubmit them to x...@oss.sgi.com so they can be > picked up in the next update. > > This update includes tests for the new renameat2 syscall, and a > bunch of fixes for the new config file format infrastructure. Actaully, it's now updated to: 8874560 generic/024: fix output number because it missed a test number update in an output file again. That test doesn't run yet on XFS, so I missed it until I ran some ext4 tests... Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] xfstests: master branch updated to 249cc51
Hi folks, The xfstests repository at git://oss.sgi.com/xfs/cmds/xfstests has just been updated. Patches often get missed, so please check if your outstanding patches were in this update. If they have not been in this update, please resubmit them to x...@oss.sgi.com so they can be picked up in the next update. This update includes tests for the new renameat2 syscall, and a bunch of fixes for the new config file format infrastructure. The new head of the master branch is commit: 249cc51 generic: add renameat2 system call number for i386 New Commits: Alexander Tsvetkov (1): [7e07c4b] generic/204: correct log size for XFS Eric Whitney (2): [2e9061f] generic/300: add fallocate() checks [f729616] generic/311: add fallocate() check Eryu Guan (1): [9f1a8e7] ext4/306: disable 64bit feature too Lukas Czerner (6): [f00a444] config: Fix SCRATCH_DEV_POOL handling [7407717] config: Unset SCRATCH_DEV when deduced from SCRATCH_DEV_POOL [e0f5daf] config: Fix setting FSTYP automatically [5138e74] config: fix specifying configuration value with equality sign [ed6d096] generic/022: Fix output file [a0e2d8e] config: Add -s option to run only specified sections Miklos Szeredi (4): [413f501] common: add infrastructure for renameat2 syscall tests [bdf2150] generic: check plain renameat2 syscall [1f98dd0] generic: check noreplace renameat2 syscall [3c9cd13] generic: check cross renameat2 syscall Theodore Ts'o (2): [dd8556c] generic/004: fix filtering of expected error message [249cc51] generic: add renameat2 system call number for i386 Wanlong Gao (1): [610e44c] check: fix wallclock wrapping problem Code Diffstat: .gitignore | 1 + README.config-sections | 30 check | 26 ++ common/config | 35 +++--- common/renameat2 | 129 + configure.ac | 2 + src/Makefile | 3 +- src/renameat2.c| 106 tests/ext4/306 | 2 +- tests/generic/004 | 2 +- tests/generic/004.out | 2 +- tests/generic/022.out | 2 +- tests/generic/023 | 57 ++ tests/generic/023.out | 51 +++ tests/generic/024 | 64 tests/generic/024.out | 51 +++ tests/generic/025 | 64 tests/generic/025.out | 51 +++ tests/generic/204 | 4 +- tests/generic/300 | 7 +++ tests/generic/311 | 6 ++- tests/generic/group| 3 ++ 22 files changed, 672 insertions(+), 26 deletions(-) create mode 100644 common/renameat2 create mode 100644 src/renameat2.c create mode 100755 tests/generic/023 create mode 100644 tests/generic/023.out create mode 100755 tests/generic/024 create mode 100644 tests/generic/024.out create mode 100755 tests/generic/025 create mode 100644 tests/generic/025.out mode change 100644 => 100755 tests/generic/311 -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] xfstests: master branch updated to 610e44c
Hi folks, The xfstests repository at git://oss.sgi.com/xfs/cmds/xfstests has just been updated. Patches often get missed, so please check if your outstanding patches were in this update. If they have not been in this update, please resubmit them to x...@oss.sgi.com so they can be picked up in the next update. This update fixes the SCRATCH_DEV_POOL config file regression. The new head of the master branch is commit: 610e44c check: fix wallclock wrapping problem New Commits: Lukas Czerner (3): [f00a444] config: Fix SCRATCH_DEV_POOL handling [7407717] config: Unset SCRATCH_DEV when deduced from SCRATCH_DEV_POOL [e0f5daf] config: Fix setting FSTYP automatically Wanlong Gao (1): [610e44c] check: fix wallclock wrapping problem Code Diffstat: check | 10 +- common/config | 33 ++--- 2 files changed, 27 insertions(+), 16 deletions(-) -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] xfstests: updated to cf1ed54
On Fri, Apr 04, 2014 at 02:07:16PM +0100, Filipe David Manana wrote: > On Fri, Apr 4, 2014 at 10:03 AM, Dave Chinner wrote: > > Hi folks, > > > > The xfstests repository at git://oss.sgi.com/xfs/cmds/xfstests has > > just been updated. Patches often get missed, so please check if your > > outstanding patches were in this update. If they have not been in > > this update, please resubmit them to x...@oss.sgi.com so they can be > > picked up in the next update. > > > > The new head of the master branch is commit: > > > > cf1ed54 check: fix RESULT_BASE typo in check script > > > > The major new functionality worth mentioning in this update is the > > new config file format from Lukas. The existing format config files > > should continue to work without change, but the new format is much > > richer and allows specification of multiple different configurations > > to run test under. Hence testing multiple mount an dmkfs > > configurations becomes as simple as iterating the configurations > > in the config file. > > Hi, > > I might be missing something, but after checking out these changes, I > am no longer able to run btrfs tests. Example: > > $ ./check btrfs/041 > common/config: Error: $SCRATCH_DEV should be unset when $SCRATCH_DEV_POOL is > set > Passed all 0 tests > > $ cat local.config > export TEST_DEV=/dev/sdb > export TEST_DIR=/home/fdmanana/btrfs-tests/dev > export SCRATCH_MNT="/home/fdmanana/btrfs-tests/scratch_1" > export SCRATCH_DEV_POOL="/dev/sdc /dev/sdd" Are you sure that's the config file that is being picked up? I can't test btrfs at the moment because it appears to be completely screwed in a TOT kernel right now - it doesn't even show up in /proc/filesystems and doesn't emit anything on dmesg to indicate that initialisation of the built in btrfs code has failed or even been attempted. It's simply MIA However, using that same SCRATCH_DEV_POOL config for xfs or ext4 works just fine on my test machines with the current TOT xfstests and kernel code. $ cat configs/test2.config TEST_DIR=/mnt/test SCRATCH_MNT=/mnt/scratch TEST_DEV=/dev/vda SCRATCH_DEV_POOL="/dev/vdc /dev/vdd" $ sudo MKFS_OPTIONS="-m crc=1" ./check generic/001 FSTYP -- xfs (debug) PLATFORM -- Linux/x86_64 test2 3.14.0-rc1-dgc+ MKFS_OPTIONS -- -f -m crc=1 /dev/vdc MOUNT_OPTIONS -- /dev/vdc /mnt/scratch generic/001 4s ... 3s Ran: generic/001 Passed all 1 tests $ So, as you can see I can't reproduce your problem myself right now. Could you add a "set -x" line to the start of check and post the output? Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] xfstests: updated to cf1ed54
On Fri, Apr 04, 2014 at 02:07:16PM +0100, Filipe David Manana wrote: > On Fri, Apr 4, 2014 at 10:03 AM, Dave Chinner wrote: > > Hi folks, > > > > The xfstests repository at git://oss.sgi.com/xfs/cmds/xfstests has > > just been updated. Patches often get missed, so please check if your > > outstanding patches were in this update. If they have not been in > > this update, please resubmit them to x...@oss.sgi.com so they can be > > picked up in the next update. > > > > The new head of the master branch is commit: > > > > cf1ed54 check: fix RESULT_BASE typo in check script > > > > The major new functionality worth mentioning in this update is the > > new config file format from Lukas. The existing format config files > > should continue to work without change, but the new format is much > > richer and allows specification of multiple different configurations > > to run test under. Hence testing multiple mount an dmkfs > > configurations becomes as simple as iterating the configurations > > in the config file. > > Hi, > > I might be missing something, but after checking out these changes, I > am no longer able to run btrfs tests. Example: > > $ ./check btrfs/041 > common/config: Error: $SCRATCH_DEV should be unset when $SCRATCH_DEV_POOL is > set > Passed all 0 tests > > $ cat local.config > export TEST_DEV=/dev/sdb > export TEST_DIR=/home/fdmanana/btrfs-tests/dev > export SCRATCH_MNT="/home/fdmanana/btrfs-tests/scratch_1" > export SCRATCH_DEV_POOL="/dev/sdc /dev/sdd" OK, that'll be a bug in the new config file parsing code. Lukas, can you have a look at this? Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] xfstests: updated to cf1ed54
eate mode 100644 tests/btrfs/043
create mode 100644 tests/btrfs/043.out
create mode 100644 tests/btrfs/044
create mode 100644 tests/btrfs/044.out
create mode 100755 tests/btrfs/045
create mode 100644 tests/btrfs/045.out
create mode 100644 tests/btrfs/046
create mode 100644 tests/btrfs/046.out
rename tests/{shared/243 => ext4/002} (99%)
rename tests/{shared/243.out => ext4/002.out} (95%)
create mode 100755 tests/generic/004
create mode 100644 tests/generic/004.out
create mode 100644 tests/generic/009
create mode 100644 tests/generic/009.out
rename tests/{shared/003 => generic/012} (93%)
mode change 100644 => 100755
rename tests/{shared/003.out => generic/012.out} (97%)
rename tests/{shared/004 => generic/016} (93%)
mode change 100644 => 100755
rename tests/{shared/004.out => generic/016.out} (97%)
rename tests/{shared/005 => generic/017} (97%)
mode change 100644 => 100755
create mode 100644 tests/generic/017.out
rename tests/{shared/218 => generic/018} (98%)
rename tests/{shared/218.out => generic/018.out} (93%)
rename tests/{shared/305 => generic/019} (98%)
rename tests/{shared/305.out => generic/019.out} (88%)
rename tests/{shared/001 => generic/021} (93%)
rename tests/{shared/001.out => generic/021.out} (97%)
rename tests/{shared/002 => generic/022} (93%)
mode change 100644 => 100755
rename tests/{shared/002.out => generic/022.out} (97%)
delete mode 100644 tests/shared/005.out
create mode 100755 tests/shared/006
create mode 100644 tests/shared/006.out
create mode 100755 tests/xfs/006
create mode 100644 tests/xfs/006.out
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] xfstests: master branch updated to 3948694
Hi folks, The xfstests repository at git://oss.sgi.com/xfs/cmds/xfstests has just been updated. Patches often get missed, so please check if your outstanding patches were in this update. If they have not been in this update, please resubmit them to x...@oss.sgi.com so they can be picked up in the next update. I know I skipped a couple of posted tests because they hadn't obviously been reviewed and I didn't know what the status of them was, so reposts (and reviews!) would be appreciated. The new head of the master branch is commit: 3948694 xfs/300: fix golden output New Commits: Dave Chinner (5): [763b46f] xfs/033: add golden output for CRC enabled filesystems [97a665b] xfs/189: noattr2 invalid for CRC enabled filesystems [84edd7a] generic/003: ensure time changes between stat calls [4a74700] xfs/217: prevent enospc failures on small test devices [b50473c] xfs/167: need at least 10GB of scratch space to run Dmitry Monakhov (2): [3365817] fsstress: add verifiable logging mode [33e02a8] generic/280: use waidpid instead of ugly sleep Eric Sandeen (2): [9642986] aio-dio: fix error msg in aio-dio-subblock-eof-read.c [3948694] xfs/300: fix golden output Filipe David Borba Manana (2): [2dd0dbe] btrfs: add function _require_fssum() [5f1cd20] btrfs: add test for btrfs-progs restore feature Jie Liu (1): [5f6be5c] shared/051: add filter to match the golden output for large acls Lukas Czerner (8): [212f48f] common: Create single function for testing xfs_io commands [9589e15] common: create _test_block_boundaries in common/punch [613cb30] generic/008: Add test for fallocate zero range at block boundary [2dc43e0] build: Move fallocate include into global.h [2d2d853] fsstress: Add fallocate zero range operation [4544179] fsstress: translate flags in fiemap_f [f074613] fsx: Add fallocate zero range operation [4d46e47] ext4/001: Add ext4 specific test for fallocate zero range Wang Shilong (1): [8acf172] btrfs: add basic functional test for btrfs quota groups ZhangZhen (1): [f7f9492] btrfs: cleanup tests 004, 007, 022 and 025 Code Diffstat: check | 2 +- common/attr | 10 + common/filter | 3 + common/punch| 106 ++ common/rc | 78 +++-- configure.ac| 3 +- ltp/fsstress.c | 222 ltp/fsx.c | 146 +--- src/aio-dio-regress/aio-dio-subblock-eof-read.c | 4 +- src/global.h| 25 ++ tests/btrfs/004 | 2 +- tests/btrfs/007 | 11 +- tests/btrfs/016 | 5 +- tests/btrfs/022 | 24 +- tests/btrfs/025 | 20 +- tests/btrfs/030 | 5 +- tests/btrfs/038 | 5 +- tests/btrfs/039 | 5 +- tests/btrfs/040 | 5 +- tests/btrfs/041 | 113 +++ tests/btrfs/041.out | 40 +++ tests/btrfs/042 | 95 ++ tests/btrfs/042.out | 2 + tests/btrfs/group | 2 + tests/ext4/001 | 64 tests/ext4/001.out | 337 ++ tests/ext4/group| 1 + tests/generic/003 | 110 +++--- tests/generic/008 | 57 tests/generic/008.out | 433 tests/generic/280 | 3 +- tests/generic/group | 1 + tests/shared/051| 10 +- tests/shared/051.out| 2 +- tests/xfs/033 | 10 +- tests/xfs/033.crc.out.linux | 197 +++ tests/xfs/167 | 6 + tests/xfs/189 | 2 + tests/xfs/217 | 4 + tests/xfs/290 | 40 +-- tests/xfs/290.out | 13 +- tests/xfs/300.out | 2 +- 42 files changed, 1930 insertions(+), 295 deletions(-) mode change 100644 => 100755 tests/btrfs/016 create mode 100755 tests/btrfs/041 create mode 100644 tests/btrfs/041.out create mode 100755 tests/btrfs/042 create mode 100644 tests/btrfs/042.out creat
Re: [PATCH v2 3/3] xfstests/btrfs: add stress test for btrfs quota operations
On Mon, Mar 10, 2014 at 03:48:43PM -0400, Josef Bacik wrote:
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
> On 03/09/2014 11:44 PM, Wang Shilong wrote:
> > So this is a stress test for btrfs quota operations. it can also
> > detect the following commit fixed problem:
> >
> > 4082bd3d73(Btrfs: fix oops when writting dirty qgroups to disk)
> >
> > Signed-off-by: Wang Shilong ---
> > v1->v2: switch into new helper _run_btrfs_util_prog() ---
> > tests/btrfs/043 | 76
> > +
> > tests/btrfs/043.out | 2 ++ tests/btrfs/group | 1 + 3 files
> > changed, 79 insertions(+) create mode 100755 tests/btrfs/043 create
> > mode 100644 tests/btrfs/043.out
> >
> > diff --git a/tests/btrfs/043 b/tests/btrfs/043 new file mode
> > 100755 index 000..d6c4bf3 --- /dev/null +++ b/tests/btrfs/043
> > @@ -0,0 +1,76 @@ +#! /bin/bash +# FS QA Test No. 043 +# +#
> > stresstest for btrfs quota operations. we run fsstress and quota +#
> > operations concurrently. +#
> > +#---
> >
> >
> +# Copyright (c) 2014 Fujitsu. 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
> > +#---
> >
> >
> +#
> > + +seq=`basename $0` +seqres=$RESULT_DIR/$seq +echo "QA output
> > created by $seq" + +here=`pwd` +tmp=/tmp/$$ +status=1 # failure is
> > the default! +trap "_cleanup; exit \$status" 0 1 2 3 15 +
> > +_cleanup() +{ +cd / + rm -f $tmp.* +} + +# get standard
> > environment, filters and checks +. ./common/rc +. ./common/filter
> > + +_supported_fs btrfs +_supported_os Linux +_require_scratch + +rm
> > -f $seqres.full + +_quota_enabled_background() +{ + i=1 + while [
> > $i -le 5 ] +do +_run_btrfs_util_prog quota enable
> > $SCRATCH_MNT +
> > _run_btrfs_util_prog quota disable $SCRATCH_MNT + i=$(($i+1)) +
> > sleep 1 + done +} + +MKFS_SIZE=$((1024 * 1024 * 1024)) +run_check
> > _scratch_mkfs_sized $MKFS_SIZE +run_check _scratch_mount +
> > +_quota_enabled_background & +run_check $FSSTRESS_PROG -d
> > $SCRATCH_MNT -w -p 5 -n 1000 \ +$FSSTRESS_AVOID + +run_check
> > _scratch_unmount +_check_scratch_fs +
>
> You should probably be doing something to make sure the background
> quota stuff exits properly before your script exits, my fio box can
> run the fsstress in way less than 5 seconds. Thanks,
josef - you might want to have a look at what your mailer is doing
to quoted email ad fix it... ;)
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: BUG: >16TB Btrfs volumes are mountable on 32 bit kernels
On Thu, Feb 27, 2014 at 04:07:06PM -0500, Josef Bacik wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA1 > > On 02/27/2014 04:05 PM, Chris Murphy wrote: > > User reports successfully formatting and using an ~18TB Btrfs > > volume on hardware raid5 using i686 kernel for over a year, and > > then suddenly the file system starts behaving weirdly: > > > > https://urldefense.proofpoint.com/v1/url?u=http://www.mail-archive.com/linux-btrfs%40vger.kernel.org/msg31856.html&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=cKCbChRKsMpTX8ybrSkonQ%3D%3D%0A&m=6eUt5RgBggFh930oFrH19iR4z%2BFVzT%2F0%2F4dYPt3g48U%3D%0A&s=5ac126734d7fa1d3238ab09a2ddc021a8dcc8fff7b022560a4d068be2de37c00 > > > > > > > > I think this is due to the kernel page cache address space being > > 16TB limited on 32-bit kernels, as mentioned by Dave Chinner in > > this thread: > > > > https://urldefense.proofpoint.com/v1/url?u=http://oss.sgi.com/pipermail/xfs/2014-February/034588.html&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=cKCbChRKsMpTX8ybrSkonQ%3D%3D%0A&m=6eUt5RgBggFh930oFrH19iR4z%2BFVzT%2F0%2F4dYPt3g48U%3D%0A&s=3e45f9288e6a77bc1a24dded368802c2ab46b812bf59953f74d4ee1d4141f7d2 > > > > So it sounds like it shouldn't be possible to mount a Btrfs volume > > larger than 16TB on 32-bit kernels. This is consistent with ext4 > > and XFS which refuse to mount large file systems. > > > > > > Well that's not good, I'll fix this up. Thanks, Well, don't go assuming there's a problem just because I made an off-hand comment. i.e my comment was simply "maybe it hasn't been tested", and not an assertion that there is a bug or a problem Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Help with space
On Thu, Feb 27, 2014 at 05:27:48PM -0700, Chris Murphy wrote: > > On Feb 27, 2014, at 5:12 PM, Dave Chinner > wrote: > > > On Thu, Feb 27, 2014 at 02:11:19PM -0700, Chris Murphy wrote: > >> > >> On Feb 27, 2014, at 1:49 PM, otakujunct...@gmail.com wrote: > >> > >>> Yes it's an ancient 32 bit machine. There must be a complex > >>> bug involved as the system, when originally mounted, claimed > >>> the correct free space and only as used over time did the > >>> discrepancy between used and free grow. I'm afraid I chose > >>> btrfs because it appeared capable of breaking the 16 tera > >>> limit on a 32 bit system. If this isn't the case then it's > >>> incredible that I've been using this file system for about a > >>> year without difficulty until now. > >> > >> Yep, it's not a good bug. This happened some years ago on XFS > >> too, where people would use the file system for a long time and > >> then at 16TB+1byte written to the volume, kablewy! And then it > >> wasn't usable at all, until put on a 64-bit kernel. > >> > >> http://oss.sgi.com/pipermail/xfs/2014-February/034588.html > > > > Well, no, that's not what I said. > > What are you thinking I said you said? I wasn't quoting or > paraphrasing anything you've said above. I had done a google > search on this early and found some rather old threads where some > people had this experience of making a large file system on a > 32-bit kernel, and only after filling it beyond 16TB did they run > into the problem. Here is one of them: > > http://lists.centos.org/pipermail/centos/2011-April/109142.html No, he didn't fill it with 16TB of data and then have it fail. He made a new filesystem *larger* than 16TB and tried to mount it: | On a CentOS 32-bit backup server with a 17TB LVM logical volume on | EMC storage. Worked great, until it rolled 16TB. Then it quit | working. Altogether. /var/log/messages told me that the | filesystem was too large to be mounted. Had to re-image the VM as | a 64-bit CentOS, and then re-attached the RDM's to the LUNs | holding the PV's for the LV, and it mounted instantly, and we | kept on trucking. This just backs up what I told you originally - that XFS has always refused to mount >16TB filesystems on 32 bit systems. > > I said that it was limited on XFS, not that the limit was a > > result of a user making a filesystem too large and then finding > > out it didn't work. Indeed, you can't do that on XFS - mkfs will > > refuse to run on a block device it can't access the last block > > on, and the kernel has the same "can I access the last block of > > the filesystem" sanity checks that are run at mount and growfs > > time. > > Nope. What I reported on the XFS list, I had used mkfs.xfs while > running 32bit kernel on a 20TB virtual disk. It did not fail to > make the file system, it failed only to mount it. You said no such thing. All you said was you couldn't mount a filesystem > 16TB - you made no mention of how you made the fs, what the block device was or any other details. > It was the same > booted virtual machine, I created the file system and immediately > mounted it. If you want the specifics, I'll post on the XFS list > with versions and reproduce steps. Did you check to see whether the block device silently wrapped at 16TB? There's a real good chance it did - but you might have got lucky because mkfs.xfs uses direct IO and *maybe* that works correctly on block devices on 32 bit systems. I wouldn't bet on it, though, given it's something we don't support and therefore never test Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Help with space
On Thu, Feb 27, 2014 at 02:11:19PM -0700, Chris Murphy wrote:
>
> On Feb 27, 2014, at 1:49 PM, otakujunct...@gmail.com wrote:
>
> > Yes it's an ancient 32 bit machine. There must be a complex bug
> > involved as the system, when originally mounted, claimed the
> > correct free space and only as used over time did the
> > discrepancy between used and free grow. I'm afraid I chose
> > btrfs because it appeared capable of breaking the 16 tera limit
> > on a 32 bit system. If this isn't the case then it's incredible
> > that I've been using this file system for about a year without
> > difficulty until now.
>
> Yep, it's not a good bug. This happened some years ago on XFS too,
> where people would use the file system for a long time and then at
> 16TB+1byte written to the volume, kablewy! And then it wasn't
> usable at all, until put on a 64-bit kernel.
>
> http://oss.sgi.com/pipermail/xfs/2014-February/034588.html
Well, no, that's not what I said. I said that it was limited on XFS,
not that the limit was a result of a user making a filesystem too
large and then finding out it didn't work. Indeed, you can't do that
on XFS - mkfs will refuse to run on a block device it can't access the
last block on, and the kernel has the same "can I access the last
block of the filesystem" sanity checks that are run at mount and
growfs time.
IOWs, XFS has *never* allowed >16TB on 32 bit systems on Linux. And,
historically speaking, it didn't even allow it on Irix. Irix on 32
bit systems was limited to 1TB (2^31 sectors of 2^9 bytes = 1TB),
and only as Linux gained sufficient capability on 32 bit systems
(e.g. CONFIG_LBD) was the limit increased. The limit we are now at
is the address space index being 32 bits, so the size is limited by
2^32 * PAGE_SIZE = 2^44 = 16TB
i.e Back when XFS was still being ported to Linux from Irix in 2000:
203 #if !XFS_BIG_FILESYSTEMS
204 if (sbp->sb_dblocks > INT_MAX || sbp->sb_rblocks > INT_MAX) {
205 cmn_err(CE_WARN,
206 "XFS: File systems greater than 1TB not supported on this system.\n");
207 return XFS_ERROR(E2BIG);
208 }
209 #endif
(http://oss.sgi.com/cgi-bin/gitweb.cgi?p=archive/xfs-import.git;a=blob;f=fs/xfs/xfs_mount.c;hb=60a4726a60437654e2af369ccc8458376e1657b9)
So, good story, but is not true.
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] xfstests: add test for btrfs-progs restore feature
On Tue, Feb 25, 2014 at 10:34:07PM +, Filipe David Manana wrote: > On Tue, Feb 25, 2014 at 10:11 PM, Dave Chinner wrote: > > On Tue, Feb 25, 2014 at 09:02:43PM +, Filipe David Manana wrote: > >> On Tue, Feb 25, 2014 at 7:54 PM, Dave Chinner wrote: > >> > On Tue, Feb 25, 2014 at 06:44:08PM +, Filipe David Borba Manana > >> > wrote: > >> >> This is a regression test to verify that the restore feature of > >> >> btrfs-progs > >> >> is able to correctly recover files that have compressed extents, > >> >> specially when > >> >> the respective file extent items have a non-zero data offset field. > >> >> > >> >> This issue is fixed by the following btrfs-progs patch: > >> >> > >> >> Btrfs-progs: fix restore of files with compressed extents > >> >> > >> >> Signed-off-by: Filipe David Borba Manana > >> > > >> >> +seq=`basename $0` > >> >> +seqres=$RESULT_DIR/$seq > >> >> +echo "QA output created by $seq" > >> >> + > >> >> +tmp=/tmp/$$ > >> >> +status=1 # failure is the default! > >> >> +trap "_cleanup; exit \$status" 0 1 2 3 15 > >> > > >> > here=`pwd` > >> > >> Didn't we agree before, for a previous path, to export "here" from the > >> main control skip and then cleanup tests to not redefine it? > >> I am confused now :) > > > > Yes, we did, but there's no patch to do that yet. Hence tests need > > to define it until the infrastructure is changed. > > There's a patch flying around that adds the _require_fssum() and then > removes definition of "here" for all btrfs tests that use fssum. changing how $here is defined needs to be in a patch of it's own, and that patch needs to remove it from every single test in the xfstests code base that declares it. test harness infrastructure changes should not be buried in an unrelated btrfs test changes > >> >> + | _filter_xfs_io > >> >> + $XFS_IO_PROG -c "pwrite -S 0xd0 -b 11 33000 11" $SCRATCH_MNT/foo \ > >> >> + | _filter_xfs_io > >> >> + $XFS_IO_PROG -c "pwrite -S 0xbc -b 100 99000 100" > >> >> $SCRATCH_MNT/foo \ > >> >> + | _filter_xfs_io > >> >> + > >> >> + md5sum $SCRATCH_MNT/foo | _filter_scratch > >> > > >> > So you are doing this with first having "persisted" the new extents. > >> > Seems kind of strange that you need to persist some and not > >> > others... > > All I want is to have different file extent items. Yes, I get that. What is not clear is where you expect the failure to be detected - in memory or on disk? > >> I need to make sure there's fragmentation (i.e. several file extent > >> items in the fs btree with data offset fields > 0). > > > > Right, but my question is why you haven't ensured that btree is on > > disk at the time you run the md5sum. > > Because it's not needed. > The sync is only to make sure the first 2 extent items aren't merged > together. And that is needed to trigger the failure. Yes, it's a pre-condition. That's not the answer to the question I've been asking, though. > > That seems important to me > > because the above sync commands indicate that having the extents on > > disk rather than in memory is important here. e.g. are you expecting > > the md5sum to be correct before the data is synced to disk, and then > > incorrect after the data is synced to disk by the unmount? > > Again what's important is having multiple extent items after unmounting. Ah, so syncing the data after the second set of writes is important, and that's what you are testing. So, why aren't you testing this with sync and fiemap? Or is it the unmount that matters, rather than the data writeback? Do you see what I'm trying to understand? If it's data writeback that triggers the bug or enables it to be detected, then that's what the test should use as the trigger. If unmount is necessary, then a comment saying "data writeback is not sufficient to trigger or detect the corruption - we need can only detect it via unmount and a fsck pass" > > code in it to check and unmount the scratch device, so if that is > > not happening then there's something broken that needs to be fixed. > > So _check_btrfs_filesystem() will u
Re: [PATCH v2] xfstests: add test for btrfs-progs restore feature
On Tue, Feb 25, 2014 at 09:02:43PM +, Filipe David Manana wrote: > On Tue, Feb 25, 2014 at 7:54 PM, Dave Chinner wrote: > > On Tue, Feb 25, 2014 at 06:44:08PM +, Filipe David Borba Manana wrote: > >> This is a regression test to verify that the restore feature of btrfs-progs > >> is able to correctly recover files that have compressed extents, specially > >> when > >> the respective file extent items have a non-zero data offset field. > >> > >> This issue is fixed by the following btrfs-progs patch: > >> > >> Btrfs-progs: fix restore of files with compressed extents > >> > >> Signed-off-by: Filipe David Borba Manana > > > >> +seq=`basename $0` > >> +seqres=$RESULT_DIR/$seq > >> +echo "QA output created by $seq" > >> + > >> +tmp=/tmp/$$ > >> +status=1 # failure is the default! > >> +trap "_cleanup; exit \$status" 0 1 2 3 15 > > > > here=`pwd` > > Didn't we agree before, for a previous path, to export "here" from the > main control skip and then cleanup tests to not redefine it? > I am confused now :) Yes, we did, but there's no patch to do that yet. Hence tests need to define it until the infrastructure is changed. > >> + _scratch_mount $OPTIONS > >> + > >> + $XFS_IO_PROG -f -c "pwrite -S 0xff -b 10 0 10" \ > >> + $SCRATCH_MNT/foo | _filter_xfs_io > >> + > >> + # Ensure a single file extent item is persisted. > >> + _run_btrfs_util_prog filesystem sync $SCRATCH_MNT > > > > What's the difference here between "sync" and the command run above? > > Unless there's some specific reason for using the above command (and > > that needs to be commented), I think that sync(1) should be used > > instead in all tests. > > I want to force a btrfs transaction commit. Plain old 'sync' would do > it as well for sure, but that applies against all mounted FSs, while > btrfs filesystem sync is applied against a single fs. And the problem with syncing all the filesystems is what? > Plus, since this is a btrfs specific test, is it non sense to exercise > commands from btrfs-progs? At the expense of testing the command that almost every user in the world uses to sync their filesystems? > >> + | _filter_xfs_io > >> + $XFS_IO_PROG -c "pwrite -S 0xd0 -b 11 33000 11" $SCRATCH_MNT/foo \ > >> + | _filter_xfs_io > >> + $XFS_IO_PROG -c "pwrite -S 0xbc -b 100 99000 100" $SCRATCH_MNT/foo \ > >> + | _filter_xfs_io > >> + > >> + md5sum $SCRATCH_MNT/foo | _filter_scratch > > > > So you are doing this with first having "persisted" the new extents. > > Seems kind of strange that you need to persist some and not > > others... > > I need to make sure there's fragmentation (i.e. several file extent > items in the fs btree with data offset fields > 0). Right, but my question is why you haven't ensured that btree is on disk at the time you run the md5sum. That seems important to me because the above sync commands indicate that having the extents on disk rather than in memory is important here. e.g. are you expecting the md5sum to be correct before the data is synced to disk, and then incorrect after the data is synced to disk by the unmount? Will you remember this detail in five years time when this test detects a regression? Indeed, will you even be around to explain why the test does this in five years time? These regression tests are going to be around for the entire life of btrfs, so we better make sure that there's enough information in them to be able to maintain them in 10-15 years time > > > > >> + _scratch_unmount > >> + _check_scratch_fs > > > > _check_scratch_fs should be unmounting the SCRATCH_DEV itself > > internally. If it's not doing that for btrfs, then the btrfs check > > code needs fixing. ;) > > No it doesn't unmount. Then _check_btrfs_filesystem() needs fixing. It certainly does have code in it to check and unmount the scratch device, so if that is not happening then there's something broken that needs to be fixed. Fix the infrastructure bug, don't work around it. > >> + > >> + _run_btrfs_util_prog restore $SCRATCH_DEV $tmp > >> + md5sum $tmp/foo | cut -d ' ' -f 1 > > > > What, exactly, are you restoring to /tmp/$$? Does this assume that > > /tmp is a btrfs filesystem? If so, that is an invalid assumption - > > /tmp can be any ty
Re: [PATCH v2] xfstests: add test for btrfs-progs restore feature
On Tue, Feb 25, 2014 at 06:44:08PM +, Filipe David Borba Manana wrote:
> This is a regression test to verify that the restore feature of btrfs-progs
> is able to correctly recover files that have compressed extents, specially
> when
> the respective file extent items have a non-zero data offset field.
>
> This issue is fixed by the following btrfs-progs patch:
>
> Btrfs-progs: fix restore of files with compressed extents
>
> Signed-off-by: Filipe David Borba Manana
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +tmp=/tmp/$$
> +status=1 # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
here=`pwd`
> +
> +_cleanup()
> +{
> +rm -fr $tmp
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs btrfs
> +_supported_os Linux
> +_require_scratch
> +_need_to_be_root
> +
> +rm -f $seqres.full
> +
> +test_btrfs_restore()
> +{
> + if [ -z $1 ]
> + then
> + OPTIONS=""
> + else
> + OPTIONS="-o compress-force=$1"
> + fi
> + _scratch_mkfs >/dev/null 2>&1
> + _scratch_mount $OPTIONS
> +
> + $XFS_IO_PROG -f -c "pwrite -S 0xff -b 10 0 10" \
> + $SCRATCH_MNT/foo | _filter_xfs_io
> +
> + # Ensure a single file extent item is persisted.
> + _run_btrfs_util_prog filesystem sync $SCRATCH_MNT
What's the difference here between "sync" and the command run above?
Unless there's some specific reason for using the above command (and
that needs to be commented), I think that sync(1) should be used
instead in all tests.
Indeed, why a separate command - just adding a "-c fsync" to the
xfs_io command, or even -s to make it open the file O_SYNC should do
what you need without needing a specific sync command
> +
> + $XFS_IO_PROG -c "pwrite -S 0xaa -b 10 10 10" \
> + $SCRATCH_MNT/foo | _filter_xfs_io
> +
> + # Now ensure a second one is created (and not merged with previous one).
> + _run_btrfs_util_prog filesystem sync $SCRATCH_MNT
> +
> + # Make the extent item be split into several ones, each with a data
> + # offset field != 0
> + $XFS_IO_PROG -c "pwrite -S 0x1e -b 2 1 2" $SCRATCH_MNT/foo \
> + | _filter_xfs_io
> + $XFS_IO_PROG -c "pwrite -S 0xd0 -b 11 33000 11" $SCRATCH_MNT/foo \
> + | _filter_xfs_io
> + $XFS_IO_PROG -c "pwrite -S 0xbc -b 100 99000 100" $SCRATCH_MNT/foo \
> + | _filter_xfs_io
> +
> + md5sum $SCRATCH_MNT/foo | _filter_scratch
So you are doing this with first having "persisted" the new extents.
Seems kind of strange that you need to persist some and not
others...
> + _scratch_unmount
> + _check_scratch_fs
_check_scratch_fs should be unmounting the SCRATCH_DEV itself
internally. If it's not doing that for btrfs, then the btrfs check
code needs fixing. ;)
> +
> + _run_btrfs_util_prog restore $SCRATCH_DEV $tmp
> + md5sum $tmp/foo | cut -d ' ' -f 1
What, exactly, are you restoring to /tmp/$$? Does this assume that
/tmp is a btrfs filesystem? If so, that is an invalid assumption -
/tmp can be any type of filesystem at all.
It's also wrong to use $tmp like this
> +}
> +
> +mkdir $tmp
> +echo "Testing restore of file compressed with lzo"
> +test_btrfs_restore "lzo"
> +echo "Testing restore of file compressed with zlib"
> +test_btrfs_restore "zlib"
> +echo "Testing restore of file without any compression"
> +test_btrfs_restore
Yup, using $tmp like this is definitely wrong. $tmp is really for test
harness files and test logs, not for *test data*. TEST_DIR is what you
should be using here, not $tmp.
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstests: add function _require_fssum()
On Mon, Feb 24, 2014 at 11:08:27PM +, Filipe David Manana wrote: > On Mon, Feb 24, 2014 at 10:22 PM, Dave Chinner wrote: > > On Mon, Feb 24, 2014 at 01:22:36PM +, Filipe David Manana wrote: > >> On Mon, Feb 24, 2014 at 12:23 PM, Dave Chinner wrote: > >> >> diff --git a/tests/btrfs/007 b/tests/btrfs/007 > >> >> index 5df9ccb..5430613 100755 > >> >> --- a/tests/btrfs/007 > >> >> +++ b/tests/btrfs/007 > >> >> @@ -31,7 +31,6 @@ seq=`basename $0` > >> >> seqres=$RESULT_DIR/$seq > >> >> echo "QA output created by $seq" > >> >> > >> >> -here=`pwd` > >> >> tmp=`mktemp -d` > >> >> status=1 > >> > > >> > Yeah, redefining $here is a bad thing to do :/ > > > > Right, my mistake. It needs to be defined for the entire test, not > > in a requires function. Hence I think we should just export it from > > check > > Hum, ok. So the decision is to let the tests explicitly define the > variable "here", and not export "here" from the main check script. Well, that's the status quo right now. What I'm suggesting is that we should just export it from check and get rid of it from each test as a separate cleanup. i.e. It always needs to be set to the same value (i.e. the root of the xfstests tree) so there is no reason why it should not be set up once in a central location. Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstests: add function _require_fssum()
On Mon, Feb 24, 2014 at 01:22:36PM +, Filipe David Manana wrote:
> On Mon, Feb 24, 2014 at 12:23 PM, Dave Chinner wrote:
> > On Mon, Feb 24, 2014 at 11:56:23AM +, Filipe David Borba Manana wrote:
> >> To avoid repeating detection of fssum presence in many btrfs tests, as
> >> suggested by Dave Chinner.
> >>
> >> Signed-off-by: Filipe David Borba Manana
> >> ---
> >> common/rc |7 +++
> >> tests/btrfs/007 |5 +
> >> tests/btrfs/016 |5 +
> >> tests/btrfs/030 |5 +
> >> tests/btrfs/038 |5 +
> >> tests/btrfs/039 |5 +
> >> tests/btrfs/040 |5 +
> >> tests/btrfs/041 |5 +
> >> tests/btrfs/042 |5 +
> >> 9 files changed, 15 insertions(+), 32 deletions(-)
> >> mode change 100644 => 100755 tests/btrfs/016
> >>
> >> diff --git a/common/rc b/common/rc
> >> index 5df504c..cce05cc 100644
> >> --- a/common/rc
> >> +++ b/common/rc
> >> @@ -2144,6 +2144,13 @@ _require_cp_reflink()
> >> _notrun "This test requires a cp with --reflink support."
> >> }
> >>
> >> +_require_fssum()
> >> +{
> >> + HERE=`pwd`
> >> + FSSUM_PROG=$HERE/src/fssum
> >> + [ -x $FSSUM_PROG ] || _notrun "fssum not built"
> >> +}
> >
> > $here is defined by check to be the root of the xfstests instance
> > that is running. There's 60+ tests that already us it. Hence:
> >
> > _require_fssum()
> > {
> > FSSUM_PROG=$here/src/fssum
> > [ -x $FSSUM_PROG ] || _notrun "fssum not built"
> > }
> >
> > Is all you need here.
>
> Hum, doesn't work unless the test file defines $here. At least when
> running a single only (.e.g. ./check btrfs/041).
Right, I realised that it isn't exported from check, and the template
for new tests defines it as:
here=\`pwd\`
which is essentially the same thing. Almost every test does it.
$ git grep here=\`pwd\` |wc -l
364
And it works just fine when running a test as you describe above.
Many of the _requires* functions just use $here directly, and does a
lot of the common/* infrastructure. It is assumed that $here is set
and valid and so if it wasn't set, lots of different things would
break.
Oh, I note that btrfs/034 and btrfs/037 don't set it, so they need
fixing, too.
Looking at it, though, we shoul djust export the value from check
and remove the assignment that occurs in every test as they end up
being the same value:
$ sudo ./check generic/001
check_here=/home/dave/src/xfstests-dev
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 test2 3.14.0-rc3-dgc+
MKFS_OPTIONS -- -f -bsize=4096 /dev/vdb
MOUNT_OPTIONS -- /dev/vdb /mnt/scratch
generic/001 5s ... [failed, exit status 1] - output mismatch (see
/home/dave/src/xfstests-dev/results//generic/001.out.bad)
.
$ cat /home/dave/src/xfstests-dev/results//generic/001.out.bad
QA output created by 001
here=/home/dave/src/xfstests-dev
$
> >> diff --git a/tests/btrfs/007 b/tests/btrfs/007
> >> index 5df9ccb..5430613 100755
> >> --- a/tests/btrfs/007
> >> +++ b/tests/btrfs/007
> >> @@ -31,7 +31,6 @@ seq=`basename $0`
> >> seqres=$RESULT_DIR/$seq
> >> echo "QA output created by $seq"
> >>
> >> -here=`pwd`
> >> tmp=`mktemp -d`
> >> status=1
> >
> > Yeah, redefining $here is a bad thing to do :/
Right, my mistake. It needs to be defined for the entire test, not
in a requires function. Hence I think we should just export it from
check
> > It also points out that the btrfs tests are using a non-standard
> > $tmp directory - one that is in the xfstests source directory.
> > That's a bad thing, too - tests should be using:
> >
> > tmp=/tmp/$$
> >
> > to store small temporary files.
> >
> > If /tmp is too small for what a test needs, then the test should be
> > using $TEST_DIR as the store for the temporary files to exercise
> > the filesystem under test as much as possible. e.g. send image
> > files build form snapshots of SCRATCH_DEV should be stored on TEST_DIR,
> > not in $tmp; filesystem image files that are mounted by loopback
> > should be stored on TEST_DIR or SCRATCH_MNT, not $tmp. And so on.
> >
> > i.e. the idea is that you direct as much of the IO to the test_DIR
> > and SCRATCH_MNT as possible, not to the filesystem that is hosting
> > $tmp or the xfstests source directory....
>
> Right. Sounds like a separate patch (to use TEST_DIR/$$ for e.g. as a
> place to st
Re: [PATCH] xfstests: add function _require_fssum()
On Mon, Feb 24, 2014 at 11:56:23AM +, Filipe David Borba Manana wrote:
> To avoid repeating detection of fssum presence in many btrfs tests, as
> suggested by Dave Chinner.
>
> Signed-off-by: Filipe David Borba Manana
> ---
> common/rc |7 +++
> tests/btrfs/007 |5 +
> tests/btrfs/016 |5 +
> tests/btrfs/030 |5 +
> tests/btrfs/038 |5 +
> tests/btrfs/039 |5 +
> tests/btrfs/040 |5 +
> tests/btrfs/041 |5 +
> tests/btrfs/042 |5 +
> 9 files changed, 15 insertions(+), 32 deletions(-)
> mode change 100644 => 100755 tests/btrfs/016
>
> diff --git a/common/rc b/common/rc
> index 5df504c..cce05cc 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2144,6 +2144,13 @@ _require_cp_reflink()
> _notrun "This test requires a cp with --reflink support."
> }
>
> +_require_fssum()
> +{
> + HERE=`pwd`
> + FSSUM_PROG=$HERE/src/fssum
> + [ -x $FSSUM_PROG ] || _notrun "fssum not built"
> +}
$here is defined by check to be the root of the xfstests instance
that is running. There's 60+ tests that already us it. Hence:
_require_fssum()
{
FSSUM_PROG=$here/src/fssum
[ -x $FSSUM_PROG ] || _notrun "fssum not built"
}
Is all you need here.
> +
> # Given 2 files, verify that they have the same mapping but different
> # inodes - i.e. an undisturbed reflink
> # Silent if so, make noise if not
> diff --git a/tests/btrfs/007 b/tests/btrfs/007
> index 5df9ccb..5430613 100755
> --- a/tests/btrfs/007
> +++ b/tests/btrfs/007
> @@ -31,7 +31,6 @@ seq=`basename $0`
> seqres=$RESULT_DIR/$seq
> echo "QA output created by $seq"
>
> -here=`pwd`
> tmp=`mktemp -d`
> status=1
Yeah, redefining $here is a bad thing to do :/
And I'd missed that this was being done in all the new btrfs tests,
otherwise I would have pulled it up earlier.
It also points out that the btrfs tests are using a non-standard
$tmp directory - one that is in the xfstests source directory.
That's a bad thing, too - tests should be using:
tmp=/tmp/$$
to store small temporary files.
If /tmp is too small for what a test needs, then the test should be
using $TEST_DIR as the store for the temporary files to exercise
the filesystem under test as much as possible. e.g. send image
files build form snapshots of SCRATCH_DEV should be stored on TEST_DIR,
not in $tmp; filesystem image files that are mounted by loopback
should be stored on TEST_DIR or SCRATCH_MNT, not $tmp. And so on.
i.e. the idea is that you direct as much of the IO to the test_DIR
and SCRATCH_MNT as possible, not to the filesystem that is hosting
$tmp or the xfstests source directory
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstests: add test btrfs/042 for btrfs incremental send
On Fri, Feb 21, 2014 at 12:02:56AM +, Filipe David Borba Manana wrote: > Regression test for a btrfs incremental send issue where invalid paths for > utimes, chown and chmod operations were sent to the send stream, causing > btrfs receive to fail. > > If a directory had a move/rename operation delayed, and none of its parent > directories, except for the immediate one, had delayed move/rename operations, > after processing the directory's references, the incremental send code would > issue invalid paths for utimes, chown and chmod operations. > > This issue is fixed by the following linux kernel btrfs patch: > >Btrfs: fix send issuing outdated paths for utimes, chown and chmod > > Signed-off-by: Filipe David Borba Manana > +FSSUM_PROG=$here/src/fssum > +[ -x $FSSUM_PROG ] || _notrun "fssum not built" Helper... . > +_scratch_unmount > +_check_btrfs_filesystem $SCRATCH_DEV And _check_scratch_fs > +_scratch_mkfs >/dev/null 2>&1 > +_scratch_mount > + > +_run_btrfs_util_prog receive $SCRATCH_MNT -f $tmp/1.snap > +run_check $FSSUM_PROG -r $tmp/1.fssum $SCRATCH_MNT/mysnap1 2>> $seqres.full > + > +_run_btrfs_util_prog receive $SCRATCH_MNT -f $tmp/2.snap > +run_check $FSSUM_PROG -r $tmp/2.fssum $SCRATCH_MNT/mysnap2 2>> $seqres.full And the redirection I'm happy to see you write your tests consistently, Filipe :) Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstests: add test for btrfs send issuing premature rmdir operations
On Wed, Feb 19, 2014 at 02:32:32PM +, Filipe David Borba Manana wrote:
> Regression test for btrfs incremental send issue where a rmdir instruction
> is sent against an orphan directory inode which is not empty yet, causing
> btrfs receive to fail when it attempts to remove the directory.
>
> This issue is fixed by the following linux kernel btrfs patch:
>
>Btrfs: fix send attempting to rmdir non-empty directories
>
> Signed-off-by: Filipe David Borba Manana
> ---
> tests/btrfs/041 | 153
> +++
> tests/btrfs/041.out |1 +
> tests/btrfs/group |1 +
> 3 files changed, 155 insertions(+)
> create mode 100755 tests/btrfs/041
> create mode 100644 tests/btrfs/041.out
>
> diff --git a/tests/btrfs/041 b/tests/btrfs/041
> new file mode 100755
> index 000..9de9326
> --- /dev/null
> +++ b/tests/btrfs/041
> @@ -0,0 +1,153 @@
> +#! /bin/bash
> +# FS QA Test No. btrfs/041
> +#
> +# Regression test for btrfs incremental send issue where a rmdir instruction
> +# is sent against an orphan directory inode which is not empty yet, causing
> +# btrfs receive to fail when it attempts to remove the directory.
> +#
> +# This issue is fixed by the following linux kernel btrfs patch:
> +#
> +# Btrfs: fix send attempting to rmdir non-empty directories
> +#
> +#---
> +# Copyright (c) 2014 Filipe Manana. 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
> +#---
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=`mktemp -d`
> +status=1 # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +rm -fr $tmp
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs btrfs
> +_supported_os Linux
> +_require_scratch
> +_need_to_be_root
> +
> +FSSUM_PROG=$here/src/fssum
> +[ -x $FSSUM_PROG ] || _notrun "fssum not built"
This is duplicated across several tests now. Perhaps this should be
factored now into a _requires_fssum helper (separate patch is
fine)?
> +
> +_scratch_unmount
> +_check_btrfs_filesystem $SCRATCH_DEV
you should be able to use _check_scratch_fs() here. I note that the
btrfs path does not unmount the scratch device, so you should update
it to do so (like _check_xfs_filesytem does) and then
_check_scratch_fs() will just Do The Right Thing.
> +_scratch_mkfs >/dev/null 2>&1
> +_scratch_mount
> +
> +_run_btrfs_util_prog receive $SCRATCH_MNT -f $tmp/1.snap
> +run_check $FSSUM_PROG -r $tmp/1.fssum $SCRATCH_MNT/mysnap1 2>> $seqres.full
> +
> +_run_btrfs_util_prog receive $SCRATCH_MNT -f $tmp/2.snap
> +run_check $FSSUM_PROG -r $tmp/2.fssum $SCRATCH_MNT/mysnap2 2>> $seqres.full
Hasn't run_check already redirected everything to $seqres.full?
> +_scratch_unmount
> +_check_btrfs_filesystem $SCRATCH_DEV
_check_scratch_fs() here too.
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Hard drive hangs after excessive I/O
On Fri, Feb 21, 2014 at 12:59 PM, Chris Murphy wrote: > Five concurrent VMs for one spinning disk. It sounds like it's seeking to > death. That's my reproducible test, not typical use. "Seeking to death" would indicate drive activity. When the problem occurs, drive activity stops for minutes at a time. I suppose I shouldn't have even mentioned VMs because that seems to be muddying the discussion. The problem persists after all the VMs are shut down. For instance, right now I'm not running any VMs but I'm experiencing the problem (though I was performing the above test). I can observe it by doing a local git clone of the linux kernel: git clone /usr/src/linux If I run this from a fresh boot, it causes the hard drive to chatter but doesn't cause IO to hang. After running the above 5-VM test for a day, even after shutting down the VMs, my machine is still in this state that causes IO to periodically hang. -- -=[dave]=- Entropy isn't what it used to be. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Hard drive hangs after excessive I/O
On Fri, Feb 21, 2014 at 11:09 AM, Duncan <1i5t5.dun...@cox.net> wrote: > One other question: Are you, or perhaps your distro via likely automated > script, doing any btrfs snapshotting of the VM-image containing btrfs > (sub)volume? I haven't done any snapshotting on this machine. This problem is pretty easy to reproduce, I spawn about five new virtual machines and begin installing Windows XP on them. The host machine obviously slows down quite a bit but the hard drive chatters throughout the process. The next day I wipe those five machines, spawn five new ones, and begin the concurrent installations again. This time I see the IO spurts that I described in my first email; that is, drive thrashes for a while, then hangs for a couple minutes... repeat until host reboot. -- -=[dave]=- Entropy isn't what it used to be. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hard drive hangs after excessive I/O
I'm currently running kernel 3.13.3 with Chris's for-linus merged in (up to commit 93de4ba86480a9e0d1062cb1d535fa97fb81af48). After about a day of heavy IO I'll start to see the following in my kernel log: [3.454967] INFO: task qemu-system-x86:27673 blocked for more than 120 seconds. [3.454970] Not tainted 3.13.3-2-dzk #1 [3.454972] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [3.454974] qemu-system-x86 D 8804039d7800 0 27673 1 0x [3.454978] 8800d1d6bda0 0082 8800096aec00 8800d1d6bfd8 [3.454983] 00014440 00014440 8800096aec00 a04aafa0 [3.454988] 018b 8802e887 3000 1000 [3.454993] Call Trace: [3.455015] [] ? btrfs_set_token_64+0x60/0xf0 [btrfs] [3.455036] [] ? release_extent_buffer+0x2b/0xd0 [btrfs] [3.455041] [] schedule+0x29/0x70 [3.455061] [] wait_log_commit.isra.18+0x102/0x130 [btrfs] [3.455066] [] ? __wake_up_sync+0x20/0x20 [3.455085] [] btrfs_sync_log+0x3e5/0x6b0 [btrfs] [3.455091] [] ? dput+0x20/0x100 [3.455112] [] btrfs_sync_file+0x2c1/0x2f0 [btrfs] [3.455117] [] do_fsync+0x51/0x80 [3.455122] [] SyS_fdatasync+0x13/0x20 [3.455127] [] system_call_fastpath+0x1a/0x1f During this time, all IO totally freezes. Atop reports no disk activity, while my desktop is totally hung. After a few minutes everything comes back to life, only to freeze again after a few more minutes. A reboot clears everything up for a day or so but the problem invariably comes back. Can anybody shed some light on this? PS, the above qemu process is accessing image files on a btrfs volume. These files were created as such: touch winxp.img chattr +C winxp.img fallocate -l20G winxp.img I'm actually quite pleased with VM performance on top of btrfs (until the above problem starts occurring). -- -=[dave]=- Entropy isn't what it used to be. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] xfstests/btrfs: add qgroup rescan stress test
On Thu, Feb 13, 2014 at 11:18:57AM +0800, Wang Shilong wrote:
> Test flow is to run fsstress after triggering quota rescan.
> the ruler is simple, we just remove all files and directories,
> sync filesystem and see if qgroup's ref and excl are nodesize.
>
> Signed-off-by: Wang Shilong
> ---
> tests/btrfs/038 | 75
> +
> tests/btrfs/038.out | 3 +++
> tests/btrfs/group | 1 +
> 3 files changed, 79 insertions(+)
> create mode 100644 tests/btrfs/038
> create mode 100644 tests/btrfs/038.out
>
> diff --git a/tests/btrfs/038 b/tests/btrfs/038
> new file mode 100644
> index 000..f6bd872
> --- /dev/null
> +++ b/tests/btrfs/038
> @@ -0,0 +1,75 @@
> +#! /bin/bash
> +# FSQA Test No. btrfs/038
> +#
> +# Quota rescan stress test, we run fsstress and quota rescan concurrently
> +#
> +#---
> +# Copyright (C) 2014 Fujitsu. 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
> +#
> +#---
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1
> +
> +_cleanup()
> +{
> + rm -f $tmp.*
> +}
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_need_to_be_root
> +_supported_fs btrfs
> +_supported_os Linux
> +_require_scratch
> +
> +rm -f $seqres.full
> +
> +run_check _scratch_mkfs "-b 2g --nodesize 4096"
> +run_check _scratch_mount
> +
> +# -w ensures that the only ops are ones which cause write I/O
> +run_check $FSSTRESS_PROG -d $SCRATCH_MNT -w -p 5 -n 1000 \
> + $FSSTRESS_AVOID >&/dev/null
> +
> +run_check $BTRFS_UTIL_PROG subvolume snapshot $SCRATCH_MNT \
> + $SCRATCH_MNT/snap1 >>$seqres.full 2>&1
> +
> +run_check $FSSTRESS_PROG -d $SCRATCH_MNT/snap1 -w -p 5 -n 1000 \
> + $FSSTRESS_AVOID >&/dev/null
> +
> +run_check $BTRFS_UTIL_PROG quota enable $SCRATCH_MNT
> +run_check $BTRFS_UTIL_PROG quota rescan -w $SCRATCH_MNT
"run_check considered harmful."
http://oss.sgi.com/archives/xfs/2014-02/msg00482.html
Once I've committed Filipe's run_btrfs_util_prog, can you update
this series to remove all the unnecessary run_check calls and
repost? Thanks!
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] xfstests: test for atime-related mount options
On Mon, Feb 17, 2014 at 02:40:45PM -0600, Eric Sandeen wrote: > On 2/17/14, 2:25 PM, Koen De Wit wrote: > > Tests the noatime, relatime, strictatime and nodiratime mount options. > > > > There is an extra check for Btrfs to ensure that the access time is > > never updated on read-only subvolumes. (Regression test for bug fixed > > with commit 93fd63c2f001ca6797c6b15b696a484b165b4800) > > I think this looks ok. Only little nit is that _require_relatime > now implicitly requires scratch, but *shrug* it was Dave's idea. ;) There's a _require_scratch call in there, so it's all good. ;) Thanks Koen! > Thanks! > > Reviewed-by: Eric Sandeen And thanks for the reivew, Eric ;) Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] xfstests: Btrfs: add test for large metadata blocks
On Tue, Feb 11, 2014 at 08:01:08PM +0100, Koen De Wit wrote:
> On 02/10/2014 11:04 PM, Dave Chinner wrote:
> >On Mon, Feb 10, 2014 at 10:39:22PM +0100, Koen De Wit wrote:
> >>+
> >>+_test_illegal_leafsize() {
> >>+_scratch_mkfs -l $1 >>$seqres.full 2>&1
> >>+[ $? -ne 0 ] || _fail "'$1' is an illegal value for the" \
> >>+"leafsize option, mkfs should have failed."
> >>+}
> >You just re-implemented run_check
>
> I believe I didn't :
>
>run_check()
>{
> echo "# $@" >> $seqres.full 2>&1
> "$@" >> $seqres.full 2>&1 || _fail "failed: '$@'"
>}
>
> run_check() takes an arbitrary command and executes it,
> _test_illegal_leafsize() takes a leafsize as parameter and tries
> mkfs.btrfs with that leafsize. run_check() makes the test fail if
> the return code is not zero, _test_illegal_leafsize() does the
> opposite.
Which points out the madness of trying to determine pass/fail by
return codes. See how easy it is to get wrong when reading the
test code?
So, this tends to point towards writing a proper mkfs filter for
btrfs that anonymizes the output so that the golden output can do
the checking without having to care about return values. That's what
we do for XFS (see _filter_mkfs), and i'd suggest that this is the
correct thing to do for btrfs as well. Then you can stop having to
check the return value of mkfs.btrfs
i.e. factor _filter_mkfs into _filter_mkfs_xfs() and
_filter_mkfs_btrfs() and implement the necessary filtering in
_filter_mkfs_btrfs.
Yes, it's a bit of a pain to do in the first place, but once the
filter is in place it will capture any future unintended
modifications to mkfs output and make people modifying mkfs.btrfs
think about what they are actually doing when they break the filter.
Indeed, we've been modifying the output of mkfs.xfs for years (every
new feature changes the mkfs output) and we've been able to do in a
way that doesn't break the filtering.
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstests: add regression test for btrfs incremental send
On Mon, Feb 17, 2014 at 01:36:02AM +, Filipe David Manana wrote:
> On Monday, February 17, 2014, Dave Chinner wrote:
>
> > On Mon, Feb 17, 2014 at 12:20:38AM +, Filipe David Borba Manana wrote:
> > > Test for a btrfs incremental send issue where we end up sending a
> > > wrong section of data from a file extent if the corresponding file
> > > extent is compressed and the respective file extent item has a non
> > > zero data offset.
> > >
> > > Fixed by the following linux kernel btrfs patch:
> > >
> > >Btrfs: use right clone root offset for compressed extents
> > >
> > > Signed-off-by: Filipe David Borba Manana
> > >
> > > ---
> > >
> > > V2: Made the test more reliable. Now it doesn't depend anymore of btrfs'
> > > hole punch implementation leaving hole file extent items when we
> > punch
> > > beyond the file's current size.
> > > V3: Filter xfs_io output and make less use of the run_check function, as
> > > suggested by Dave Chinner.
> >
> > Awesome. Thanks for the quick turn around.
> >
> > > common/rc |5 +++
> > > tests/btrfs/040 | 119
> > +++
> > > tests/btrfs/040.out |9
> > > tests/btrfs/group |1 +
> > > 4 files changed, 134 insertions(+)
> > > create mode 100755 tests/btrfs/040
> > > create mode 100644 tests/btrfs/040.out
> > >
> > > diff --git a/common/rc b/common/rc
> > > index e91568b..27be009 100644
> > > --- a/common/rc
> > > +++ b/common/rc
> > > @@ -2207,6 +2207,11 @@ run_check()
> > > "$@" >> $seqres.full 2>&1 || _fail "failed: '$@'"
> > > }
> > >
> > > +_run_btrfs_util_prog()
> > > +{
> > > + run_check $BTRFS_UTIL_PROG $*
> > > +}
> >
> > Can you do a cleanup of all the other btrfs tests that can use this?
>
>
> Ok. I just did that for all the test cases not yet merged, as you probably
> noticed already.
Yes, I did ;)
> For the ones already in the repository, I'll see if I can do it soon this
> coming week.
Great! Thanks for doing this.
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] xfstests: add regression test for btrfs incremental send
On Mon, Feb 17, 2014 at 12:20:38AM +, Filipe David Borba Manana wrote:
> Test for a btrfs incremental send issue where we end up sending a
> wrong section of data from a file extent if the corresponding file
> extent is compressed and the respective file extent item has a non
> zero data offset.
>
> Fixed by the following linux kernel btrfs patch:
>
>Btrfs: use right clone root offset for compressed extents
>
> Signed-off-by: Filipe David Borba Manana
> ---
>
> V2: Made the test more reliable. Now it doesn't depend anymore of btrfs'
> hole punch implementation leaving hole file extent items when we punch
> beyond the file's current size.
> V3: Filter xfs_io output and make less use of the run_check function, as
> suggested by Dave Chinner.
Awesome. Thanks for the quick turn around.
> common/rc |5 +++
> tests/btrfs/040 | 119
> +++
> tests/btrfs/040.out |9
> tests/btrfs/group |1 +
> 4 files changed, 134 insertions(+)
> create mode 100755 tests/btrfs/040
> create mode 100644 tests/btrfs/040.out
>
> diff --git a/common/rc b/common/rc
> index e91568b..27be009 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2207,6 +2207,11 @@ run_check()
> "$@" >> $seqres.full 2>&1 || _fail "failed: '$@'"
> }
>
> +_run_btrfs_util_prog()
> +{
> + run_check $BTRFS_UTIL_PROG $*
> +}
Can you do a cleanup of all the other btrfs tests that can use this?
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] xfstests: add regression test for btrfs incremental send
On Sun, Feb 16, 2014 at 11:43:17PM +, Filipe David Manana wrote:
> On Sun, Feb 16, 2014 at 11:08 PM, Dave Chinner wrote:
> > On Sat, Feb 15, 2014 at 03:36:13PM +, Filipe David Borba Manana wrote:
> >> Test for a btrfs incremental send issue where we end up sending a
> >> wrong section of data from a file extent if the corresponding file
> >> extent is compressed and the respective file extent item has a non
> >> zero data offset.
> >>
> >> Fixed by the following linux kernel btrfs patch:
> >>
> >>Btrfs: use right clone root offset for compressed extents
> >>
> >> Signed-off-by: Filipe David Borba Manana
> >> ---
> >>
> >> V2: Made the test more reliable. Now it doesn't depend anymore of btrfs'
> >> hole punch implementation leaving hole file extent items when we punch
> >> beyond the file's current size.
> >>
> >> tests/btrfs/040 | 115
> >> +++
> >> tests/btrfs/040.out |1 +
> >> tests/btrfs/group |1 +
> >> 3 files changed, 117 insertions(+)
> >> create mode 100755 tests/btrfs/040
> >> create mode 100644 tests/btrfs/040.out
> >>
> >> diff --git a/tests/btrfs/040 b/tests/btrfs/040
> >> new file mode 100755
> >> index 000..d6b37bf
> >> --- /dev/null
> >> +++ b/tests/btrfs/040
> >> @@ -0,0 +1,115 @@
> >> +#! /bin/bash
> >> +# FS QA Test No. btrfs/040
> >> +#
> >> +# Test for a btrfs incremental send issue where we end up sending a
> >> +# wrong section of data from a file extent if the corresponding file
> >> +# extent is compressed and the respective file extent item has a non
> >> +# zero data offset.
> >> +#
> >> +# Fixed by the following linux kernel btrfs patch:
> >> +#
> >> +# Btrfs: use right clone root offset for compressed extents
> >> +#
> >> +#---
> >> +# Copyright (c) 2014 Filipe Manana. 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
> >> +#---
> >> +#
> >> +
> >> +seq=`basename $0`
> >> +seqres=$RESULT_DIR/$seq
> >> +echo "QA output created by $seq"
> >> +
> >> +here=`pwd`
> >> +tmp=`mktemp -d`
> >> +status=1 # failure is the default!
> >> +trap "_cleanup; exit \$status" 0 1 2 3 15
> >> +
> >> +_cleanup()
> >> +{
> >> +rm -fr $tmp
> >> +}
> >> +
> >> +# get standard environment, filters and checks
> >> +. ./common/rc
> >> +. ./common/filter
> >> +
> >> +# real QA test starts here
> >> +_supported_fs btrfs
> >> +_supported_os Linux
> >> +_require_scratch
> >> +_need_to_be_root
> >> +
> >> +FSSUM_PROG=$here/src/fssum
> >> +[ -x $FSSUM_PROG ] || _notrun "fssum not built"
> >> +
> >> +rm -f $seqres.full
> >> +
> >> +_scratch_mkfs >/dev/null 2>&1
> >> +_scratch_mount "-o compress-force=lzo"
> >> +
> >> +run_check $XFS_IO_PROG -f -c "truncate 118811" $SCRATCH_MNT/foo
> >> +run_check $XFS_IO_PROG -c "pwrite -S 0x0d -b 39987 92267 39987" \
> >> + $SCRATCH_MNT/foo
> >
> > Ugh. filter the output, don't use run_check.
> >
> > $XFS_IO_PROG -f -c "truncate 118811" $SCRATCH_MNT/foo
> > $XFS_IO_PROG -c "pwrite -S 0x0d -b 39987 92267 39987" \
> > $SCRATCH_MNT/foo | _filter_xfs_io
> >
> > If something fails, we still want the test to continue running, even
> > if all it does is exercise
Re: [PATCH v2] xfstests: add regression test for btrfs incremental send
he btrfs
progs commands have inconsistent output and so are difficult to
match. However, given that this is leading to bad habits like using
run_check for everything.
I'd suggest that we need a set of $BTRFS_UTIL_PROG specific handlers
to deal with these differences rather than continuing to pollute the
tests with run_check. e.g.
_run_btrfs_util_prog()
{
run_check $BTRFS_UTIL_PROG $*
}
would be a good start because it gets that run_check pattern out of
the main test scripts and hence out of the heads of test writers.
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstests: test for atime-related mount options
On Fri, Feb 14, 2014 at 09:02:08PM -0600, Eric Sandeen wrote:
> On 2/14/14, 7:39 PM, Dave Chinner wrote:
> > On Fri, Feb 14, 2014 at 05:48:59PM -0600, Eric Sandeen wrote:
> >> On 2/14/14, 4:24 PM, Dave Chinner wrote:
> >>> On Fri, Feb 14, 2014 at 10:41:16AM -0600, Eric Sandeen wrote:
> >>>> On 2/14/14, 10:39 AM, David Sterba wrote:
> >>>>> On Thu, Feb 13, 2014 at 10:42:55AM -0600, Eric Sandeen wrote:
> >>>>>>> +cat /proc/mounts | grep "$SCRATCH_MNT" | grep relatime >>
> >>>>>>> $seqres.full
> >>>>>>> +[ $? -ne 0 ] && echo "The relatime mount option should be the
> >>>>>>> default."
> >>>>>>
> >>>>>> Ok, I guess "relatime" in /proc/mounts is from core vfs code and
> >>>>>> should be there for the foreseeable future, so seems ok.
> >>>>>>
> >>>>>> But - relatime was added in v2.6.20, and made default in 2.6.30. So
> >>>>>> testing older kernels may not go as expected; it'd probably be best to
> >>>>>> catch situations where relatime isn't available (< 2.6.20) or not
> >>>>>> default (< 2.6.30), by explicitly mounting with relatime, and skipping
> >>>>>> relatime/strictatime tests if that fails?
> >>>>>
> >>>>> Is there some consensus what's the lowest kernel version to be supported
> >>>>> by xfstests? 2.6.32 is the lowest base for kernels in use today, so
> >>>>> worrying about anything older does not seem necessary.
> >>>>>
> >>>>
> >>>> I don't know that it's been discussed - selfishly, I know our QE uses
> >>>> xfstests on RHEL5, which is 2.6.18-based.
> >>>
> >>> Sure, but they can just add the test to a "rhel5-expunged" file and
> >>> they don't have to care about tests that won't work on RHEL 5 or
> >>> other older kernels. Or to send patches to add "_requires_relatime"
> >>> so that it automatically does the right thing for older kernels.
> >>
> >> sure but some of this test is still valid on a kernel w/o relatime.
> >> And since it's the default, "relatime" might disappear from /proc/mounts
> >> some day anyway, so explicitly mounting with the option & failing
> >> if that fails might be good future-proofind in any case.
> >>
> >> *shrug*
> >>
> >> It was just a request, not a demand. :) Koen, you can do with
> >> it whatever you like. Reviews aren't ultimatums. :)
> >>
> >> If xfstests upstream is only targeted at the current kernel, that's
> >> fine, but maye we should make that a little more explicit.
> >
> > That's not what I meant. ;)
> >
> > Really, all I'm saying is that we can't expect people who are
> > writing tests that work on current kernels to know what is necessary
> > to make tests work on 7 year old distros that don't support a
> > feature that has been in mainline for 5 years. Hence that shouldn't
> > be a barrier to having a test committed as we have mechanisms for
> > distro QE to handle these sorts of issues...
>
> Sure, that's perfectly fair.
>
> I wasn't really thinking of RHEL5 when I made my first comment,
> just general portability across kernels. dsterba suggested that
> 2.6.32 is the oldest kernel used, and I pointed out that we do
> still use 2.6.18. :)
>
> Anyway, for general portability across releases, perhaps rather than:
>
> +cat /proc/mounts | grep "$SCRATCH_MNT" | grep relatime >> $seqres.full
> +[ $? -ne 0 ] && echo "The relatime mount option should be the default."
>
> which would fail the test, it should just [notrun] if relatime
> isn't there, for any reason, on any kernel, if relatime is not
> default as expected for the test framework. i.e.
>
> +[ $? -ne 0 ] && _notrun "The relatime mount option is not the default."
I disagree - the test doesn't need to care what the default mount
option is - it's a relatively silly thing to test because it doesn't
determine whether the behaviour of the option is correct or not.
Especially as the test is checking the behaviour of specific atime
mount options so it should just specify each one it is testing and
ignoring what the default is. IOWs, the default atime behaviour just
doesn't matter for the purpose of the test
What the test actually cares about is this:
_require_relatime()
{
_scratch_mkfs > /dev/null 2>&1
_mount -t $FSTYP -o relatime $SCRATCH_DEV $SCRATCH_MNT || \
_notrun "relatime not supported by the current kernel"
}
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstests: test for atime-related mount options
On Fri, Feb 14, 2014 at 05:48:59PM -0600, Eric Sandeen wrote: > On 2/14/14, 4:24 PM, Dave Chinner wrote: > > On Fri, Feb 14, 2014 at 10:41:16AM -0600, Eric Sandeen wrote: > >> On 2/14/14, 10:39 AM, David Sterba wrote: > >>> On Thu, Feb 13, 2014 at 10:42:55AM -0600, Eric Sandeen wrote: > >>>>> +cat /proc/mounts | grep "$SCRATCH_MNT" | grep relatime >> $seqres.full > >>>>> +[ $? -ne 0 ] && echo "The relatime mount option should be the default." > >>>> > >>>> Ok, I guess "relatime" in /proc/mounts is from core vfs code and > >>>> should be there for the foreseeable future, so seems ok. > >>>> > >>>> But - relatime was added in v2.6.20, and made default in 2.6.30. So > >>>> testing older kernels may not go as expected; it'd probably be best to > >>>> catch situations where relatime isn't available (< 2.6.20) or not > >>>> default (< 2.6.30), by explicitly mounting with relatime, and skipping > >>>> relatime/strictatime tests if that fails? > >>> > >>> Is there some consensus what's the lowest kernel version to be supported > >>> by xfstests? 2.6.32 is the lowest base for kernels in use today, so > >>> worrying about anything older does not seem necessary. > >>> > >> > >> I don't know that it's been discussed - selfishly, I know our QE uses > >> xfstests on RHEL5, which is 2.6.18-based. > > > > Sure, but they can just add the test to a "rhel5-expunged" file and > > they don't have to care about tests that won't work on RHEL 5 or > > other older kernels. Or to send patches to add "_requires_relatime" > > so that it automatically does the right thing for older kernels. > > sure but some of this test is still valid on a kernel w/o relatime. > And since it's the default, "relatime" might disappear from /proc/mounts > some day anyway, so explicitly mounting with the option & failing > if that fails might be good future-proofind in any case. > > *shrug* > > It was just a request, not a demand. :) Koen, you can do with > it whatever you like. Reviews aren't ultimatums. :) > > If xfstests upstream is only targeted at the current kernel, that's > fine, but maye we should make that a little more explicit. That's not what I meant. ;) Really, all I'm saying is that we can't expect people who are writing tests that work on current kernels to know what is necessary to make tests work on 7 year old distros that don't support a feature that has been in mainline for 5 years. Hence that shouldn't be a barrier to having a test committed as we have mechanisms for distro QE to handle these sorts of issues... Indeed, I'm quite happy to host distro specific test expunge files in the upstream repo so anyone can see what tests are expected to pass/run on various distros Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstests: test for atime-related mount options
On Fri, Feb 14, 2014 at 10:41:16AM -0600, Eric Sandeen wrote: > On 2/14/14, 10:39 AM, David Sterba wrote: > > On Thu, Feb 13, 2014 at 10:42:55AM -0600, Eric Sandeen wrote: > >>> +cat /proc/mounts | grep "$SCRATCH_MNT" | grep relatime >> $seqres.full > >>> +[ $? -ne 0 ] && echo "The relatime mount option should be the default." > >> > >> Ok, I guess "relatime" in /proc/mounts is from core vfs code and > >> should be there for the foreseeable future, so seems ok. > >> > >> But - relatime was added in v2.6.20, and made default in 2.6.30. So > >> testing older kernels may not go as expected; it'd probably be best to > >> catch situations where relatime isn't available (< 2.6.20) or not > >> default (< 2.6.30), by explicitly mounting with relatime, and skipping > >> relatime/strictatime tests if that fails? > > > > Is there some consensus what's the lowest kernel version to be supported > > by xfstests? 2.6.32 is the lowest base for kernels in use today, so > > worrying about anything older does not seem necessary. > > > > I don't know that it's been discussed - selfishly, I know our QE uses > xfstests on RHEL5, which is 2.6.18-based. Sure, but they can just add the test to a "rhel5-expunged" file and they don't have to care about tests that won't work on RHEL 5 or other older kernels. Or to send patches to add "_requires_relatime" so that it automatically does the right thing for older kernels. Ultimately, upstream developers can't do all the work necessary to support distros - that's why the distros have their own engineers and QE to make sure the upstream code works correctly when they backport it. xfstests is no different. ;) IOWs, if someone wants to run a modern test suite on a 7 year old distro, then they need to make sure that the test suite does the right thing for their distro. We'll take the patches that make it work, but we can't expect upstream developers to know what old distros require, let alone test and make stuff work on them... Just my 2c worth. Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [3.14-rc1] BUG: soft lockup - CPU#1 stuck for 22s with 255 GiB BTRFS with only 6 GiB free
On Tue, Feb 11, 2014 at 10:36 AM, Martin Steigerwald wrote: > Today I started getting those on 3.14-rc. One core as displayed as 100% > system CPU. I rebooted cause the system didn´t respond consistently to > user input anymore. Does 3.14-rc1 have Joseph's delayed refs throttling code? I had two separate machines that exhibited similar symptoms. Chris's for-linus branch has a fix for this which solved my problems: https://git.kernel.org/cgit/linux/kernel/git/mason/linux-btrfs.git/commit/?h=for-linus&id=27a377db745ed4d11b3b9b340756857cb8dde07f -- -=[dave]=- Entropy isn't what it used to be. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] xfstests: Btrfs: add test for large metadata blocks
On Mon, Feb 10, 2014 at 10:39:22PM +0100, Koen De Wit wrote:
> Tests Btrfs filesystems with all possible metadata block sizes, by
> setting large extended attributes on files.
>
> Signed-off-by: Koen De Wit
> +
> +_test_illegal_leafsize() {
> +_scratch_mkfs -l $1 >>$seqres.full 2>&1
> +[ $? -ne 0 ] || _fail "'$1' is an illegal value for the" \
> +"leafsize option, mkfs should have failed."
> +}
You just re-implemented run_check
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstests: btrfs/004: fix to make test really work
On Mon, Feb 10, 2014 at 08:10:56PM +0800, Wang Shilong wrote:
> From: Wang Shilong
>
> So i was wandering why test 004 could pass my previous wrong
> kernel patch while it defenitely should not.
>
> By some debugging, i found here perl script is wrong, we did not
> filter out anything and this unit test did not work acutally.so
> it came out we will never fail this test.
>
> Signed-off-by: Wang Shilong
> ---
> tests/btrfs/004 | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
> mode change 100755 => 100644 tests/btrfs/004
>
> diff --git a/tests/btrfs/004 b/tests/btrfs/004
> old mode 100755
> new mode 100644
> index 14da9f1..17a6e34
> --- a/tests/btrfs/004
> +++ b/tests/btrfs/004
> @@ -57,10 +57,9 @@ _require_command "/usr/sbin/filefrag"
>
> rm -f $seqres.full
>
> -FILEFRAG_FILTER='if (/, blocksize (\d+)/) {$blocksize = $1; next} ($ext, '\
> -'$logical, $physical, $expected, $length, $flags) = (/^\s*(\d+)\s+(\d+)'\
> -'\s+(\d+)\s+(?:(\d+)\s+)?(\d+)\s+(.*)/) or next; $flags =~ '\
> -'/(?:^|,)inline(?:,|$)/ and next; print $physical * $blocksize, "#", '\
> +FILEFRAG_FILTER='if (/blocks of (\d+) bytes/) {$blocksize = $1; next} ($ext,
> '\
> +'$logical, $physical, $length) = (/^\s*(\d+):\s+(\d+)..\s+\d+:'\
> +'\s+(\d+)..\s+\d+:\s+(\d+):/) or next; print $physical * $blocksize, "#", '\
> '$length * $blocksize, "#", $logical * $blocksize, " "'
Oh, boy, who allowed that mess to pass review? Please format this in
a readable manner while you are changing it.
FILEFRAG_FILTER='
if (/blocks of (\d+) bytes/) { \
$blocksize = $1;\
next; \
}
.
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstests: Btrfs: add test for large metadata blocks
On Sat, Feb 08, 2014 at 09:30:51AM +0100, Koen De Wit wrote:
> On 02/07/2014 11:49 PM, Dave Chinner wrote:
> >On Fri, Feb 07, 2014 at 06:14:45PM +0100, Koen De Wit wrote:
> > echo -n "$xattr_value" | md5sum
> > ${ATTR_PROG} -Lq -s attr_$char -V $xattr_value $file
> > ${ATTR_PROG} -Lq -g attr_$char $file | md5sum
> > ${ATTR_PROG} -Lq -g attr_$char $lnkfile | md5sum
> >
> >is all that neds to be done here.
>
> The problem with this is that the length of the output will depend on the
> page size. The code above runs for every valid leafsize, which can be any
> multiple of the page size up to 64KB, as defined in the loop initialization:
> for leafsize in `seq $pagesize_kb $pagesize_kb 64`; do
That's only a limit on the mkfs leafsize parameter, yes? An the
limiation is that the leaf size can't be smaller than page size?
So really, the attribute sizes that are being tested are independent
of the mkfs parameters being tested. i.e:
for attrsize in `seq 4 4 64`; do
if [ $attrsize -lt $pagesize ]; then
leafsize=$pagesize
else
leafsize=$attrsize
fi
$BTRFS_MKFS_PROG -l $leafsize $SCRATCH_DEV
And now the test executes a fixed loop, testing the same attribute
sizes on all the filesystems under test. i.e. the attribute sizes
being tested are *independent* of the mkfs parameters being tested.
Always test the same attribute sizes, the mkfs parameters simply
vary by page size.
> >>+_scratch_unmount + +# Some illegal leafsizes + +_scratch_mkfs
> >>-l 0 2>> $seqres.full +echo $?
> >Same again - you are dumping the error output into a different
> >file, then detecting the error manually. pass the output of
> >_scratch_mkfs through a filter, and let errors cause golden
> >output mismatches.
>
> I did this to make the golden output not depend on the output of
> mkfs.btrfs, inspired by
> http://oss.sgi.com/cgi-bin/gitweb.cgi?p=xfs/cmds/xfstests.git;a=commit;h=fd7a8e885732475c17488e28b569ac1530c8eb59
> and
> http://oss.sgi.com/cgi-bin/gitweb.cgi?p=xfs/cmds/xfstests.git;a=commit;h=78d86b996c9c431542fdbac11fa08764b16ceb7d
> However, in my opinion the test should simply be updated if the
> output of mkfs.btrfs changes, so I agree with you and I fixed this
> in v2.
While I agree with the sentiment, I'm questioning the
implementation. i.e. you've done this differently to every other
test that needs to check for failures. run_check woul dbe just
fine, as would be simply filtering the output of mkfs.
FWIW, the method for detecting the cp error in the second commit
is for a very specific case. It could have also been done with a
filter, as we have done in the past with such error messages. So
what's good for one case is not necessarily the right way to handle
the output for another.
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstests: Btrfs: add test for large metadata blocks
On Fri, Feb 07, 2014 at 06:14:45PM +0100, Koen De Wit wrote:
> Tests Btrfs filesystems with all possible metadata block sizes, by
> setting large extended attributes on files.
>
> Signed-off-by: Koen De Wit
There's a few things here that need fixing.
> +pagesize=`$here/src/feature -s`
> +pagesize_kb=`expr $pagesize / 1024`
> +
> +# Test all valid leafsizes
> +for leafsize in `seq $pagesize_kb $pagesize_kb 64`; do
> +_scratch_unmount >/dev/null 2>&1
Indentation are tabs, and tabs are 8 spaces in size, please.
> +_scratch_mkfs -l ${leafsize}K >/dev/null
> +_scratch_mount
No need to use _scratch_unmount here - you should be doing a
_check_scratch_fs at the end of the loop.
> +# Calculate the xattr size, but leave 512 bytes for other metadata.
> +xattr_size=`expr $leafsize \* 1024 - 512`
> +
> +touch $SCRATCH_MNT/emptyfile
> +# smallfile will be inlined, bigfile not.
> +$XFS_IO_PROG -f -c "pwrite 0 100" $SCRATCH_MNT/smallfile >/dev/null
> +$XFS_IO_PROG -f -c "pwrite 0 9000" $SCRATCH_MNT/bigfile >/dev/null
> +ln -s $SCRATCH_MNT/bigfile $SCRATCH_MNT/bigfile_softlink
> +
> +files=(emptyfile smallfile bigfile bigfile_softlink)
> +chars=(a b c d)
> +for i in `seq 0 1 3`; do
> +char=${chars[$i]}
> +file=$SCRATCH_MNT/${files[$i]}
> +lnkfile=${file}_hardlink
> +ln $file $lnkfile
> +xattr_value=`head -c $xattr_size < /dev/zero | tr '\0' $char`
> +
> +set_md5=`echo -n "$xattr_value" | md5sum`
Just dump the md5sum to the output file.
> +${ATTR_PROG} -Lq -s attr_$char -V $xattr_value $file
> +get_md5=`${ATTR_PROG} -Lq -g attr_$char $file | md5sum`
> +get_ln_md5=`${ATTR_PROG} -Lq -g attr_$char $lnkfile | md5sum`
And dump these to the output file, too. Then the golden image
matching when the test is finish will tell you if it passed or not.
i.e:
echo -n "$xattr_value" | md5sum
${ATTR_PROG} -Lq -s attr_$char -V $xattr_value $file
${ATTR_PROG} -Lq -g attr_$char $file | md5sum
${ATTR_PROG} -Lq -g attr_$char $lnkfile | md5sum
is all that neds to be done here.
> +# Test attributes with a size larger than the leafsize.
> +# Should result in an error.
> +if [ "$leafsize" -lt "64" ]; then
> +# Bash command lines cannot be larger than 64K characters, so we
> +# do not test attribute values with a size >64KB.
> +xattr_size=`expr $leafsize \* 1024 + 512`
> +xattr_value=`head -c $xattr_size < /dev/zero | tr '\0' x`
> +${ATTR_PROG} -q -s attr_toobig -V $xattr_value \
> +$SCRATCH_MNT/emptyfile >> $seqres.full 2>&1
> +if [ "$?" -eq "0" ]; then
> +echo "Expected error, xattr_size is bigger than ${leafsize}K"
> +fi
What you are doing is redirecting the error to $seqres.full
so that it doesn't end up in the output file, then detecting the
absence of an error and dumping a message to the output file to make
the test fail.
IOWs, the ATTR_PROG failure message should be in the golden output
file and you don't have to do anything else to detect a pass/fail
condition.
> +_scratch_unmount
> +
> +# Some illegal leafsizes
> +
> +_scratch_mkfs -l 0 2>> $seqres.full
> +echo $?
Same again - you are dumping the error output into a different file,
then detecting the error manually. pass the output of _scratch_mkfs
through a filter, and let errors cause golden output mismatches.
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] Btrfs: add regression test for running snapshot and send concurrently
On Fri, Feb 07, 2014 at 12:18:31PM +0800, Wang Shilong wrote: > > Hi Dave, > > > On Fri, Feb 07, 2014 at 12:10:08AM +0800, Wang Shilong wrote: > >> +$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \ > >> + $SCRATCH_MNT/snap_1 >> $seqres.full 2>&1 > >> + > >> +do_snapshots & > >> +snapshots_pid=$! > >> + > >> +$BTRFS_UTIL_PROG send $SCRATCH_MNT/snap_1 > /dev/null 2>&1 || echo "btrfs > >> send failed" > > > > Let's stop this anti-pattern before it takes hold. > > > > If there's output from the send command it should be > > filtered and captured in the golden image. Hence any deviation > > caused by errors is automatically flagged as an error. > > > > That's the whole point of using golden images for capturing errors - > > you don't need to capture return values from binaries and it > > guarantees that users are informed about failures through error > > messages. IOWs: > > > > $BTRFS_UTIL_PROG send $SCRATCH_MNT/snap_1 | _btrfs_send_filter > > > > is what you should be doing here. > > I knew what you mean here, in fact, i did this on purpose. Ok, then you need to explain why you did it on purpose with a comment. It's just as important to explain the reason for doing something in test code as it is in the kernel code. i.e. so when we are looking at the test in 5 years time we know the reason for it being that way. > for this test failure, btrfs-prog did not output failure > information from the beginning. I have nothing good to say about that state of affairs, but... > So to make older progs can also > detect the test failure, i dropped into this way. .. it's going to have to stay like it. Please insert an appropriately sarcastic comment about the usefulness of a silent send command here, because if I write it I'm going to offend lots of people. :/ Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Btrfs: add regression test for running snapshot and send concurrently
On Thu, Feb 06, 2014 at 09:12:51PM +0800, Wang Shilong wrote: > Hi Dave, > > > On Mon, Feb 03, 2014 at 11:22:36PM +0800, Wang Shilong wrote: > >> From: Wang Shilong > >> > >> Btrfs would fail to send if snapshot run concurrently, this test is to make > >> sure we have fixed the bug. > >> > > Couple of comments below. > > > >> +_scratch_mkfs > /dev/null 2>&1 > >> +_scratch_mount > >> + > >> + > >> +touch $SCRATCH_MNT/foo > >> + > >> +# get file with fragments by using backwards writes. > >> +for i in `seq 10240 -1 1`; do > >> + $XFS_IO_PROG -f -d -c "pwrite $(($i * 4096)) 4096" \ > >> + $SCRATCH_MNT/foo > /dev/null | _filter_xfs_io > > > > Indentation. > > > >> +done > >> + > >> +$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \ > >> + $SCRATCH_MNT/snap_1 >> $seqres.full 2>&1 > >> + > >> +$BTRFS_UTIL_PROG send -f $SCRATCH_MNT/send_file \ > >> + $SCRATCH_MNT/snap_1 >> $seqres.full 2>&1 & > >> + > >> +pid=$! > >> + > >> +$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT/snap_1 \ > >> + $SCRATCH_MNT/snap_2 >> $seqres.full 2>&1 > >> + > >> +wait $pid || echo "Failed to send, see dmesg" > > > > This seems kind of racy. It assumes that the send command > > doesn't complete before the wait $pid call is made. If $pid doesn't > > exist at this time because it has completed, wait will return 127 > > and the test will fail…. > > Sorry for delay reply! > > Maybe a better idea for this will be: > > Opposite to previous way, we do snapshots background while at the > same time we do sending. > > And btrfs-progs should output meaningful information on send failure, i will > make a tiny patch to address this issue. but to make this test more friendly, > i > think we can still do something like: > > btrfs send <..> || echo "Failed to send" If send is emitting error messages on failures, then the "|| echo..." is redundant and not necessary to cause a golden image mismatch. If send is not emitting error messages on failure, then it needs fixing because users are going to hate you for silently failing to send the data they asked to be sent. Either way, the echo command on error in the test is not needed. Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] btrfs/035: add new clone overwrite regression test
On Thu, Feb 06, 2014 at 02:59:14PM +0100, David Disseldorp wrote: > This test uses the newly added cloner binary to dispatch full file and > range specific clone (reflink) requests. > + > +echo -n "$src_str" > $SCRATCH_MNT/src || echo "failed to create src" Not exactly what I intended. If echo fails, it will output some kind of error message, and that will cause the golden image mismatch. Otherwise the test looks good. Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] Btrfs: add regression test for running snapshot and send concurrently
On Fri, Feb 07, 2014 at 12:10:08AM +0800, Wang Shilong wrote: > +$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \ > + $SCRATCH_MNT/snap_1 >> $seqres.full 2>&1 > + > +do_snapshots & > +snapshots_pid=$! > + > +$BTRFS_UTIL_PROG send $SCRATCH_MNT/snap_1 > /dev/null 2>&1 || echo "btrfs > send failed" Let's stop this anti-pattern before it takes hold. If there's output from the send command it should be filtered and captured in the golden image. Hence any deviation caused by errors is automatically flagged as an error. That's the whole point of using golden images for capturing errors - you don't need to capture return values from binaries and it guarantees that users are informed about failures through error messages. IOWs: $BTRFS_UTIL_PROG send $SCRATCH_MNT/snap_1 | _btrfs_send_filter is what you should be doing here. Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] btrfs/035: add new clone overwrite regression test
On Wed, Feb 05, 2014 at 12:16:49PM +0100, David Disseldorp wrote:
> This test uses the newly added cloner binary to dispatch full file and
> range specific clone (reflink) requests.
A couple of small nits:
> +CLONER_PROG=$here/src/cloner
Need to test that the binary was build and is present.
> +
> +src_str="aa"
> +
> +echo -n "$src_str" > $SCRATCH_MNT/src || _fail "failed to create src"
No need for the "|| _fail ..." in any part of this test. Failures
will be caught in the output and hence cause golden output
mismatches.
Letting the test run even after a failure exercises the filesystem
in interesting ways, so it's worthwhile ignoring failures in the
test and letting the harness pick up the failures through error
messages.
> +$CLONER_PROG $SCRATCH_MNT/src $SCRATCH_MNT/src.clone1
> +
> +src_str="bbcc"
> +
> +echo -n "$src_str" > $SCRATCH_MNT/src || _fail "failed to create src"
> +
> +$CLONER_PROG $SCRATCH_MNT/src $SCRATCH_MNT/src.clone2
> +
> +snap_src_sz=`ls -lah $SCRATCH_MNT/src.clone1 | awk '{print $5}'`
> +echo "attempting ioctl (src.clone1 src)"
> +$CLONER_PROG -s 0 -d 0 -l ${snap_src_sz} \
> + $SCRATCH_MNT/src.clone1 $SCRATCH_MNT/src || _fail "ioctl failed"
And to do that here, you probably need to add perror() output to
the cloner program when it detects an error. i.e. let it give you
the exact error that was detected, rather than lumping them all into
a catchall here...
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] btrfs: add small program for clone testing
On Wed, Feb 05, 2014 at 12:16:48PM +0100, David Disseldorp wrote:
> The cloner program is capable of cloning files using the BTRFS_IOC_CLONE
> and BTRFS_IOC_CLONE_RANGE ioctls.
>
> Signed-off-by: David Disseldorp
Hi Dave - long time since I've seen your head pop up around here ;)
A few comments below.
> +struct btrfs_ioctl_clone_range_args {
> + int64_t src_fd;
> + uint64_t src_offset;
> + uint64_t src_length;
> + uint64_t dest_offset;
> +};
> +
> +#define BTRFS_IOCTL_MAGIC 0x94
> +#define BTRFS_IOC_CLONE _IOW(BTRFS_IOCTL_MAGIC, 9, int)
> +#define BTRFS_IOC_CLONE_RANGE _IOW(BTRFS_IOCTL_MAGIC, 13, \
> +struct btrfs_ioctl_clone_range_args)
Is there some published header file that these belong to? i.e.
somewhere in the include/linux/uapi/ kernel directory? Normally the
way to handle this sort of thing is by autoconf - if the header file
exists, then we include it, otherwise we use the manual definitions.
This just means that if the public api ever changes, we'll pick it
up automatically in future...
> +int
> +main(int argc, char **argv)
> +{
> + bool full_file = true;
> + uint64_t src_off = 0;
> + uint64_t dst_off = 0;
> + uint64_t len = 0;
> + char *src_file;
> + int src_fd;
> + char *dst_file;
> + int dst_fd;
> + int ret;
> + int opt;
> +
> + while ((opt = getopt(argc, argv, "s:d:l:")) != -1) {
> + switch (opt) {
> + case 's':
> + src_off = atoi(optarg);
atoi() only returns 32 bit numbers. You probably should use
strtoull() as the offset parameters are 64 bit.
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Btrfs: add regression test for running snapshot and send concurrently
On Mon, Feb 03, 2014 at 11:22:36PM +0800, Wang Shilong wrote: > From: Wang Shilong > > Btrfs would fail to send if snapshot run concurrently, this test is to make > sure we have fixed the bug. > Couple of comments below. > +_scratch_mkfs > /dev/null 2>&1 > +_scratch_mount > + > + > +touch $SCRATCH_MNT/foo > + > +# get file with fragments by using backwards writes. > +for i in `seq 10240 -1 1`; do > + $XFS_IO_PROG -f -d -c "pwrite $(($i * 4096)) 4096" \ > + $SCRATCH_MNT/foo > /dev/null | _filter_xfs_io Indentation. > +done > + > +$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \ > + $SCRATCH_MNT/snap_1 >> $seqres.full 2>&1 > + > +$BTRFS_UTIL_PROG send -f $SCRATCH_MNT/send_file \ > + $SCRATCH_MNT/snap_1 >> $seqres.full 2>&1 & > + > +pid=$! > + > +$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT/snap_1 \ > + $SCRATCH_MNT/snap_2 >> $seqres.full 2>&1 > + > +wait $pid || echo "Failed to send, see dmesg" This seems kind of racy. It assumes that the send command doesn't complete before the wait $pid call is made. If $pid doesn't exist at this time because it has completed, wait will return 127 and the test will fail Also, why would a failure to send result in meaingful information in dmesg? Shouldn't the userspace command output information to tell you why there was a failure into $seqres.full? Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUCE] xfstests: updated to ad969ca
Hi folks, The xfstests repository at git://oss.sgi.com/xfs/cmds/xfstests has just been updated. Patches often get missed, so please check if your outstanding patches were in this update. If they have not been in this update, please resubmit them to x...@oss.sgi.com so they can be picked up in the next update. The new head of the master branch is commit: ad969ca btrfs/030: add more test cases New Commits: Brian Foster (2): [20641b8] xfs: remove spurious line continuation from _require_xfs_crc [710281f] generic/313: initialise TEST_DIR before use Eric Whitney (1): [138ea5c] ext4/306: avoid failures caused by incompatible mount options Filipe David Borba Manana (2): [fd7a8e8] btrfs/025: make test more robust [ad969ca] btrfs/030: add more test cases Josef Bacik (2): [78d86b9] btrfs/029: filter mkfs and cp output [8ebabf7] generic/299: truncate can fail with ENOSPC Wang Shilong (2): [6717b24] Btrfs: add regression test for transaction abortion when remounting [cd86825] Btrfs: add regression test for iterating backrefs Code Diffstat: common/rc | 2 +- tests/btrfs/025 | 34 ++--- tests/btrfs/025.out | 12 tests/btrfs/029 | 6 ++-- tests/btrfs/029.out | 4 +-- tests/btrfs/030 | 86 - tests/btrfs/032 | 57 +++ tests/btrfs/032.out | 3 ++ tests/btrfs/033 | 71 +++ tests/btrfs/033.out | 2 ++ tests/btrfs/group | 2 ++ tests/ext4/306 | 21 + tests/generic/299 | 2 +- tests/generic/313 | 3 +- 14 files changed, 233 insertions(+), 72 deletions(-) create mode 100755 tests/btrfs/032 create mode 100644 tests/btrfs/032.out create mode 100755 tests/btrfs/033 create mode 100644 tests/btrfs/033.out -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstests: more tests for test case btrfs/030
On Sun, Feb 02, 2014 at 10:08:06PM +, Filipe David Manana wrote: > On Sun, Feb 2, 2014 at 9:57 PM, Dave Chinner wrote: > > On Sat, Feb 01, 2014 at 02:05:32AM +, Filipe David Borba Manana wrote: > >> This change adds some new tests for btrfs' incremental send feature. > >> These are all related with inverting the parent-child relationship > >> of directories, and cover the cases: > >> > >> * when the new parent didn't get renamed (just moved) > >> * when a child file of the former parent gets renamed too > >> > >> These new cases are fixed by the following btrfs linux kernel patches: > >> > >> * "Btrfs: more send support for parent/child dir relationship inversion" > >> * "Btrfs: fix send dealing with file renames and directory moves" > >> > >> Signed-off-by: Filipe David Borba Manana > > > > Rather than modifying 030 which will cause it to fail on kernels > > where it previously passed, can you factor out the common code and > > create a new test with the additional coverage? > > > > i.e. the rule of thumb is that once a test is "done" we don't go > > back and modify it in significant ways - we write a new unit test > > that covers the new/extended functionality. Redundancy in unit tests > > is not a bad thing... > > Right. The only reason I did this, instead of a new test file, is that > because the former fix which btrfs/030 relates to is not yet in any > kernel release. Given this fact, what do you think? Ok, so if it already fails for everyone, then I think we'll be fine to modify it like this. "done" is a flexible concept when it comes to unit tests ;) Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstests: more tests for test case btrfs/030
On Sat, Feb 01, 2014 at 02:05:32AM +, Filipe David Borba Manana wrote: > This change adds some new tests for btrfs' incremental send feature. > These are all related with inverting the parent-child relationship > of directories, and cover the cases: > > * when the new parent didn't get renamed (just moved) > * when a child file of the former parent gets renamed too > > These new cases are fixed by the following btrfs linux kernel patches: > > * "Btrfs: more send support for parent/child dir relationship inversion" > * "Btrfs: fix send dealing with file renames and directory moves" > > Signed-off-by: Filipe David Borba Manana Rather than modifying 030 which will cause it to fail on kernels where it previously passed, can you factor out the common code and create a new test with the additional coverage? i.e. the rule of thumb is that once a test is "done" we don't go back and modify it in significant ways - we write a new unit test that covers the new/extended functionality. Redundancy in unit tests is not a bad thing... Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] xfstests updated to 197f773
Hi all, The xfstests repository at git://oss.sgi.com/xfs/cmds/xfstests has just been updated. Patches often get missed, so please check if your outstanding patches were in this update. If they have not been in this update, please resubmit them to x...@oss.sgi.com so they can be picked up in the next update. The new head of the master branch is commit: 197f773 xfstests: define $seqres in btrfs/026-029 New Commits: Dave Chinner (7): [ea0b6eb] xfs: test scratch device mkfs features [b9b5d74] xfs: New _require_* tests for CRC enabled filesystems [86e91cc] xfs: add fsstress/recovery test [5b524ee] xfs/073, 208: remove .full output before starting the test [b04b0fd] xfs: support xfs_metadump with external logs [3ed573e] xfs/104: use fixed log size [7657a10] generic/204: use fixed log size for XFS David Sterba (3): [80622a6] xfstests: don't suggest to run full diff when DIFF_LENGTH is 0 [586a06c] xfstests: use value of FSTYP if defined externally [1ed9046] xfstests: clean command names in btrfs tests Eric Sandeen (1): [a93b1dd] ext4: regression test for ext4 resize with non-extent files Filipe David Borba Manana (1): [8c427eb] xfstests: add test for btrfs incremental send infinite loop issue Koen De Wit (2): [0c58766] xfstests: btrfs: cross-subvolume sparse copy [197f773] xfstests: define $seqres in btrfs/026-029 Code Diffstat: README | 3 ++ check | 13 ++--- common/attr | 6 +++ common/rc | 122 +++--- tests/btrfs/003 | 2 +- tests/btrfs/004 | 2 +- tests/btrfs/007 | 4 +- tests/btrfs/008 | 12 ++--- tests/btrfs/009 | 6 +-- tests/btrfs/013 | 2 +- tests/btrfs/014 | 6 +-- tests/btrfs/015 | 2 +- tests/btrfs/016 | 14 ++--- tests/btrfs/017 | 2 +- tests/btrfs/019 | 14 ++--- tests/btrfs/022 | 6 +-- tests/btrfs/024 | 2 +- tests/btrfs/025 | 4 +- tests/btrfs/026 | 1 + tests/btrfs/027 | 1 + tests/btrfs/028 | 1 + tests/btrfs/029 | 1 + tests/btrfs/030 | 149 tests/btrfs/030.out | 1 + tests/btrfs/031 | 138 tests/btrfs/031.out | 48 + tests/btrfs/group | 2 + tests/ext4/306 | 82 + tests/ext4/306.out | 13 + tests/ext4/group| 1 + tests/generic/204 | 4 ++ tests/generic/208 | 2 + tests/shared/298| 2 +- tests/xfs/073 | 2 + tests/xfs/104 | 2 +- tests/xfs/186 | 1 + tests/xfs/187 | 32 ++- tests/xfs/244 | 1 + tests/xfs/253 | 3 +- tests/xfs/278 | 1 + tests/xfs/287 | 3 +- tests/xfs/291 | 4 +- tests/xfs/306 | 105 tests/xfs/306.out | 2 + tests/xfs/group | 1 + 45 files changed, 716 insertions(+), 109 deletions(-) -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] xfstests updated to 3099791
Hi all, The xfstests repository at git://oss.sgi.com/xfs/cmds/xfstests has just been updated. Patches often get missed, so please check if your outstanding patches were in this update. If they have not been in this update, please resubmit them to x...@oss.sgi.com so they can be picked up in the next update. Note that I didn't pull in the tmpfs support patch series this time around as I'm not sure the discussions ever came to a conclusion before xmas/new year/LCA intervened. That will need to be picked up again... The new head of the master branch is commit: 3099791 btrfs: sparse copy between different filesystems/mountpoints New Commits: Anand Jain (2): [83adc23] btrfs/006: fails with mixed-mode/small disks [7cb5299] btrfs/001: filter subvol delete output David Sterba (3): [9ced657] Makefile: fix minor build warning [d7e5b7f] check: accept tests/ prefix for test name on commandline [a050cb0] lsqa.pl: update for new tests layout Eric Sandeen (2): [94d3f77] xfs/049, 073: use MKFS_XFS_PROG where appropriate [6571cea] xfstests: allow override of XFS_IOC_DIOINFO Filipe David Borba Manana (1): [74643fc] btrfs: test send issue with non-aligned clone operations Jeff Mahoney (1): [99b3a9a] aio-stress: use calloc for thread_info array Jie Liu (4): [2086933] common: introduce two pre-checkup routines for xfs crc specified testing [cc92a95] xfs: refactor xfs/299 for crc feature pre-checkup [b42851e] xfs: verify turn group/project quotas off while user quotas is on [9645d9a] xfs: disable group/project quotas along with fsstress Koen De Wit (4): [c8d9f19] btrfs: simple sparse copy testcase for btrfs [890bc11] btrfs: sparse copy of a directory tree on btrfs [9b48c97] btrfs: moving and deleting sparse copies on btrfs [3099791] btrfs: sparse copy between different filesystems/mountpoints Lukas Czerner (3): [a056ab7] common: Filter out lost+found directory from _ls_l() output [1a98c8b] generic/322: use _filter_scratch() [3128e9c] generic/321, 322: do not remove lost+found Miao Xie (1): [5aafebc] common: Enhance the scratch dev pool and deletable device check Wang Shilong (1): [d1d43f6] btrfs/022: fix failed case with qgroup limit test Code Diffstat: Makefile | 3 +- check | 3 +- common/filter | 6 +++ common/filter.btrfs | 11 + common/rc | 63 - lsqa.pl | 32 +++ ltp/aio-stress.c | 4 +- ltp/doio.c| 12 ++ ltp/fsstress.c| 12 ++ ltp/iogen.c | 10 + src/unwritten_sync.c | 5 +++ tests/btrfs/001 | 3 +- tests/btrfs/003 | 2 +- tests/btrfs/011 | 2 +- tests/btrfs/022 | 20 -- tests/btrfs/025 | 98 + tests/btrfs/025.out | 18 + tests/btrfs/026 | 90 + tests/btrfs/026.out | 16 tests/btrfs/027 | 107 + tests/btrfs/027.out | 25 tests/btrfs/028 | 81 + tests/btrfs/028.out | 7 tests/btrfs/029 | 108 ++ tests/btrfs/029.out | 15 +++ tests/btrfs/group | 5 +++ tests/generic/321 | 2 +- tests/generic/322 | 10 ++--- tests/generic/322.out | 8 ++-- tests/xfs/049 | 2 +- tests/xfs/073 | 2 +- tests/xfs/299 | 14 ++- tests/xfs/299.out | 1 - tests/xfs/304 | 86 tests/xfs/304.out | 7 tests/xfs/305 | 89 + tests/xfs/305.out | 7 tests/xfs/group | 2 + 38 files changed, 937 insertions(+), 51 deletions(-) -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstests: Add pairing mount options test
On Mon, Jan 13, 2014 at 12:00:26PM +0800, Qu Wenruo wrote: > On Mon, 13 Jan 2014 14:26:50 +1100, Dave Chinner wrote: > >On Mon, Jan 13, 2014 at 10:26:05AM +0800, Qu Wenruo wrote: > >>On mon, 13 Jan 2014 12:52:39 +1100, Dave Chinner wrote: > >>>On Sun, Jan 12, 2014 at 07:35:44PM -0600, Eric Sandeen wrote: > >>>>I won't say no to this, but it seems to be of somewhat limited use. > >>>What happens to the test when mount options are deprecated/removed? > >>>How are we going to handle the matrix of testable/untestable mount > >>>options across kernels with different mount option support? > >>In my opinion,there may be two ways to deal it: > >>1) Introduce up_limit_kver and down_limit_kver to *every* mount option. > >>If needed also add deprecated flags. > >Both of which are messy, and kernel version number checks don't work > >with vendor kernels that have stuff back ported to them. > > > >>This method will introduce more effort tomaintain the test case, but > >>due to the small codes and > >>relativly less changes in mount options, I consider it as an > >>acceptable method. > >What you are saying is that such a test will require constant > >maintenance from upstream developers to keep working across all the > >kernels that btrfs supports. > > > >When combined with Eric's comments that it doesn't test the > >functionality and so has relatively little benefit in terms of > >improving code coverage, it doesn't paint a pretty picture. So from > >that point of view, I'd say no to such a test. > > > >>It would be quite nice if any one can provide any better idea. > >Write a test for each individual feature that exercises and > >validates that feature in some way. Part of a functional test would > >be to test that the mount options for that function do what they are > >intended to do. Eric suggested the same thing (though in a different > >way). > > > >Cheers, > > > >Dave. > That's right, individual test case is the best way. > > But most of the options are just instructive options, > and only affects performance, it's very hard to test. (like > space_cace and nospace_cache) Bad example. nospace_cache/space_cache change what is stored on disk, and affect *mount* processing depending on whether there is a cache or not. It also affects how tools like fsck.btrfs check the filesystem and so on. There is far more to test than just remount behaviour when testing the functionality of such a mount option, and that's the reason why they should be done in targeted feature tests. > Now I'm interested in how other filesystems like xfs makes sure that > every pairing > mount options are tested. XFS has extremely limited remount option support, and we already have xfs/189 for that. And it does all sorts of interesting things to support the different configurations and behaviours of /etc/mtab that different distros use because that changes mount behaviour Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstests: Add pairing mount options test
On Mon, Jan 13, 2014 at 10:26:05AM +0800, Qu Wenruo wrote: > On mon, 13 Jan 2014 12:52:39 +1100, Dave Chinner wrote: > >On Sun, Jan 12, 2014 at 07:35:44PM -0600, Eric Sandeen wrote: > >>I won't say no to this, but it seems to be of somewhat limited use. > >What happens to the test when mount options are deprecated/removed? > >How are we going to handle the matrix of testable/untestable mount > >options across kernels with different mount option support? > In my opinion,there may be two ways to deal it: > 1) Introduce up_limit_kver and down_limit_kver to *every* mount option. > If needed also add deprecated flags. Both of which are messy, and kernel version number checks don't work with vendor kernels that have stuff back ported to them. > This method will introduce more effort tomaintain the test case, but > due to the small codes and > relativly less changes in mount options, I consider it as an > acceptable method. What you are saying is that such a test will require constant maintenance from upstream developers to keep working across all the kernels that btrfs supports. When combined with Eric's comments that it doesn't test the functionality and so has relatively little benefit in terms of improving code coverage, it doesn't paint a pretty picture. So from that point of view, I'd say no to such a test. > It would be quite nice if any one can provide any better idea. Write a test for each individual feature that exercises and validates that feature in some way. Part of a functional test would be to test that the mount options for that function do what they are intended to do. Eric suggested the same thing (though in a different way). Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstests: kill lib/random.c
On Tue, Jan 07, 2014 at 09:20:12PM +, Chris Mason wrote: > On Tue, 2014-01-07 at 16:17 -0500, Josef Bacik wrote: > > On 01/07/2014 03:40 PM, Ben Myers wrote: > > > On Tue, Jan 07, 2014 at 02:10:15PM -0600, Eric Sandeen wrote: > > >> On 1/7/14, 2:01 PM, Ben Myers wrote: > > >>> Hey Gents, > > >>> > > >>> On Mon, Jan 06, 2014 at 03:46:58PM -0600, Eric Sandeen wrote: > > >>>> On 1/6/14, 3:42 PM, Josef Bacik wrote: > > >>>>> On 01/06/2014 04:32 PM, Eric Sandeen wrote: > > >>>>>> On 1/6/14, 1:58 PM, Josef Bacik wrote: > > >>>>>>> I was trying to reproduce something with fsx and I noticed that no > > >>>>>>> matter what > > >>>>>>> seed I set I was getting the same file. Come to find out we are > > >>>>>>> overloading > > >>>>>>> random() with our own custom horribleness for some unknown reason. > > >>>>>>> So nuke the > > >>>>>>> damn thing from orbit and rely on glibc's random(). With this fix > > >>>>>>> the -S option > > >>>>>>> actually does something with fsx. Thanks, > For now we can just use srandom? Seems to me like it will solve the problem. Josef? Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstests: Add pairing mount options test
On Sun, Jan 12, 2014 at 07:35:44PM -0600, Eric Sandeen wrote: > On 1/12/14, 7:21 PM, Qu Wenruo wrote: > > On fri, 10 Jan 2014 10:15:37 -0600, Eric Sandeen wrote: > >> On 1/8/14, 12:30 AM, Qu Wenruo wrote: > >>> Test remount btrfs with different pairing options like barrier and no > >>> barrier. > >> It seems that while this tests that the remount succeeds, and that > >> the option string is present in /proc/mounts, it does not test that > >> the mount option is actually in effect. > > > > Yes, this is what the new test case is intended to do. > > This case was just a test case tests the mount options themselves > > to ensure all the pairing mount options works during remounting, > > since most pairing options are missing before. > >> > >> I suppose for many of these options that would be hard to test; for > >> i.e. acl though it should be trivial. > >> > >> What do you think, is this enough to ensure that remount handling > >> is working as expected for all of these options? > > In my opinion, this test should just focuses on the remount handling and > > the pairing options. > > For the detailed function should be examineed in other test cases. > > Except those won't test that a remount with those options actually *worked*; > in fact they don't do remount at all. > > In other words, all this does is test that an option flag was set or unset in > the superblock, but it doesn't really test whether the option has been > properly set up (or torn down) as a result. > > I won't say no to this, but it seems to be of somewhat limited use. What happens to the test when mount options are deprecated/removed? How are we going to handle the matrix of testable/untestable mount options across kernels with different mount option support? Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: btrfs raid1 and btrfs raid10 arrays NOT REDUNDANT
On Fri, Jan 3, 2014 at 9:59 PM, Jim Salter wrote: > You're suggesting the wrong alternatives here (mdraid, LVM, etc) - they > don't provide the features that I need or are accustomed to (true snapshots, > copy on write, self-correcting redundant arrays, and on down the line). If > you're going to shoo me off, the correct way to do it is to wave me in the > direction of ZFS, in which case I can tell you I've been a happy user of ZFS > for 5+ years now on hundreds of systems. ZFS and btrfs are literally the > *only* options available that do what I want to do, and have been doing for > years now. (At least aside from six-figure-and-up proprietary systems, which > I have neither the budget nor the inclination for.) Jim, there's nothing stopping you from creating a Btrfs filesystem on top of an mdraid array. I'm currently running three WD Red 3TB drives in a raid5 configuration under a Btrfs filesystem. This configuration works pretty well and fills the feature gap you're describing. I will say, though, that the whole tone of your email chain leaves a bad taste in my mouth; kind of like a poorly adjusted relative who shows up once a year for Thanksgiving and makes everyone feel uncomfortable. I find myself annoyed by the constant disclaimers I read on this list, about the experimental status of Btrfs, but it's apparent that this hasn't sunk in for everyone. Your poor budget doesn't a production filesytem make. I and many others on this list who have been using Btrfs, will tell you with no hesitation, that due to the maturity of the code, Btrfs should be making NO assumptions in the event of a failure, and everything should come to a screeching halt. I've seen it all: the infamous 120 second process hangs, csum errors, multiple separate catastrophic failures (search me on this list). Things are MOSTLY stable but you simply have to glance at a few weeks of history on this list to see the experimental status is fully justified. I use Btrfs because of its intoxicating feature set. As an IT director though, I'd never subject my company to these rigors. If Btrfs on mdraid isn't an acceptable solution for you, then ZFS is the only responsible alternative. -- -=[dave]=- Entropy isn't what it used to be. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Status of raid5/6 in 2014?
Back in Feb 2013 there was quite a bit of press about the preliminary raid5/6 implementation in Btrfs. At the time it wasn't useful for anything other then testing and it's my understanding that this is still the case. I've seen a few git commits and some chatter on this list but it would appear the developers are largely silent. Parity based raid would be a powerful addition the the Btrfs feature stack and it's the feature I most anxiously await. Are there any milestones planned for 2014? Keep up the good work... -- -=[dave]=- Entropy isn't what it used to be. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] Btrfs: introduce lock_ref/unlock_ref
On Wed, Dec 18, 2013 at 04:07:27PM -0500, Josef Bacik wrote:
> qgroups need to have a consistent view of the references for a particular
> extent
> record. Currently they do this through sequence numbers on delayed refs, but
> this is no longer acceptable. So instead introduce lock_ref/unlock_ref. This
> will provide the qgroup code with a consistent view of the reference while it
> does its accounting calculations without interfering with the delayed ref
> code.
> Thanks,
>
> Signed-off-by: Josef Bacik
> ---
> fs/btrfs/ctree.h | 11 ++
> fs/btrfs/delayed-ref.c | 2 +
> fs/btrfs/delayed-ref.h | 1 +
> fs/btrfs/extent-tree.c | 102
> +++--
> 4 files changed, 113 insertions(+), 3 deletions(-)
>
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index a924274..8b3fd61 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -1273,6 +1273,9 @@ struct btrfs_block_group_cache {
>
> /* For delayed block group creation */
> struct list_head new_bg_list;
> +
> + /* For locking reference modifications */
> + struct extent_io_tree ref_lock;
> };
>
> /* delayed seq elem */
> @@ -3319,6 +3322,14 @@ int btrfs_init_space_info(struct btrfs_fs_info
> *fs_info);
> int btrfs_delayed_refs_qgroup_accounting(struct btrfs_trans_handle *trans,
>struct btrfs_fs_info *fs_info);
> int __get_raid_index(u64 flags);
> +int lock_ref(struct btrfs_fs_info *fs_info, u64 root_objectid, u64 bytenr,
> + u64 num_bytes, int for_cow,
> + struct btrfs_block_group_cache **block_group,
> + struct extent_state **cached_state);
> +int unlock_ref(struct btrfs_fs_info *fs_info, u64 root_objectid, u64 bytenr,
> +u64 num_bytes, int for_cow,
> +struct btrfs_block_group_cache *block_group,
> +struct extent_state **cached_state);
Please namespace these - they are far too similar to the generic
struct lockref name and manipulation functions
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4 v3] fiemap: add EXTENT_DATA_COMPRESSED flag
On Thu, Dec 12, 2013 at 05:02:57PM -0700, Andreas Dilger wrote:
> On Dec 12, 2013, at 4:24 PM, Dave Chinner wrote:
> > On Thu, Dec 12, 2013 at 04:25:59PM +0100, David Sterba wrote:
> >> This flag was not accepted when fiemap was proposed [2] due to lack of
> >> in-kernel users. Btrfs has compression for a long time and we'd like to
> >> see that an extent is compressed in the output of 'filefrag' utility
> >> once it's taught about it.
> >>
> >> For that purpose, a reserved field from fiemap_extent is used to let the
> >> filesystem store along the physcial extent length when the flag is set.
> >> This keeps compatibility with applications that use FIEMAP.
> >
> > I'd prefer to just see the new physical length field always filled
> > out, regardless of whether it is a compressed extent or not. In
> > terms of backwards compatibility to userspace, it makes no
> > difference because the value of reserved/unused fields is undefined
> > by the API. Yes, the implementation zeros them, but there's nothing
> > in the documentation that says "reserved fields must be zero".
> > Hence I think we should just set it for every extent.
>
> I'd actually thought the same thing while reading the patch, but I figured
> people would object because it implies that old kernels will return a
> physical length of 0 bytes (which might be valid) and badly-written tools
> will not work correctly on older kernels.
Well, that's a problem regardless of whether new kernels return a
physical length by default or not. I think I'd prefer a flag that
says specifically whether the fe_phys_len field is valid or not. Old
kernels will never set the flag, new kernels can always set the
flag...
> That said, applications _should_
> be checking the FIEMAP_EXTENT_DATA_COMPRESSED flag, and I suspect in the
> future fewer developers will be confused if fe_phys_length == fe_length
> going forward.
I think an explicit flag is better than relying on a flag that
defines the encoding to imply the physical length field is valid.
> If the initial tools get it right (in particular filefrag),
I'd think xfs_io is the first target - because we'll need xfstests
coverage of this before there's a filefrag release that supports
it...
> then hopefully others will get it correct also.
Agreed.
> > From the point of view of the kernel API (fiemap_fill_next_extent),
> > passing the physical extent size in the "len" parameter for normal
> > extents, then passing 0 for the "physical length" makes absolutely
> > no sense.
> >
> > IOWs, what you have created is a distinction between the extent's
> > "logical length" and it's "physical length". For uncompressed
> > extents, they are both equal and they should both be passed to
> > fiemap_fill_next_extent as the same value. Extents where they are
> > different (i.e. encoded extents) is when they can be different.
> > Perhaps fiemap_fill_next_extent() should check and warn about
> > mismatches when they differ and the relevant flags are not set...
>
> Seems reasonable to have a WARN_ONCE() in that case. That would catch bugs
> in the filesystem, code as well:
>
> WARN_ONCE(phys_len != lgcl_len &&
> !(flags & FIEMAP_EXTENT_DATA_COMPRESSED),
> "physical len %llu != logical length %llu without
> DATA_COMPRESSED\n",
> phys_len, logical_len, phys_len, logical_len);
Yup, pretty much what I was thinking.
> >> --- a/include/uapi/linux/fiemap.h
> >> +++ b/include/uapi/linux/fiemap.h
> >> @@ -19,7 +19,9 @@ struct fiemap_extent {
> >>__u64 fe_physical; /* physical offset in bytes for the start
> >>* of the extent from the beginning of the disk */
> >>__u64 fe_length; /* length in bytes for this extent */
> >> - __u64 fe_reserved64[2];
> >> + __u64 fe_phys_length; /* physical length in bytes, undefined if
> >> + * DATA_COMPRESSED not set */
> >> + __u64 fe_reserved64;
> >>__u32 fe_flags;/* FIEMAP_EXTENT_* flags for this extent */
> >>__u32 fe_reserved[3];
> >> };
> >
> > The comment for fe_length needs to change, too, because it needs to
> > indicate that it is the logical extent length and that it may be
> > different to the fe_phys_length depending on the flags that are set
> > on the extent.
>
> Would it make sense to rename fe_length to fe_logi_length (or something,
> I'm open to suggestions), and have a compat macro:
>
Re: [PATCH 2/4 v3] fiemap: add EXTENT_DATA_COMPRESSED flag
On Thu, Dec 12, 2013 at 04:25:59PM +0100, David Sterba wrote:
> This flag was not accepted when fiemap was proposed [2] due to lack of
> in-kernel users. Btrfs has compression for a long time and we'd like to
> see that an extent is compressed in the output of 'filefrag' utility
> once it's taught about it.
>
> For that purpose, a reserved field from fiemap_extent is used to let the
> filesystem store along the physcial extent length when the flag is set.
> This keeps compatibility with applications that use FIEMAP.
I'd prefer to just see the new physical length field always filled
out, regardless of whether it is a compressed extent or not. In
terms of backwards compatibility to userspace, it makes no
difference because the value of reserved/unused fields is undefined
by the API. Yes, the implementation zeros them, but there's nothing
in the documentation that says "reserved fields must be zero".
Hence I think we should just set it for every extent.
>From the point of view of the kernel API (fiemap_fill_next_extent),
passing the physical extent size in the "len" parameter for normal
extents, then passing 0 for the "physical length" makes absolutely
no sense.
IOWs, what you have created is a distinction between the extent's
"logical length" and it's "physical length". For uncompressed
extents, they are both equal and they should both be passed to
fiemap_fill_next_extent as the same value. Extents where they are
different (i.e. encoded extents) is when they can be different.
Perhaps fiemap_fill_next_extent() should check and warn about
mismatches when they differ and the relevant flags are not set...
> diff --git a/include/uapi/linux/fiemap.h b/include/uapi/linux/fiemap.h
> index 93abfcd..0e32cae 100644
> --- a/include/uapi/linux/fiemap.h
> +++ b/include/uapi/linux/fiemap.h
> @@ -19,7 +19,9 @@ struct fiemap_extent {
> __u64 fe_physical; /* physical offset in bytes for the start
> * of the extent from the beginning of the disk */
> __u64 fe_length; /* length in bytes for this extent */
> - __u64 fe_reserved64[2];
> + __u64 fe_phys_length; /* physical length in bytes, undefined if
> +* DATA_COMPRESSED not set */
> + __u64 fe_reserved64;
> __u32 fe_flags;/* FIEMAP_EXTENT_* flags for this extent */
> __u32 fe_reserved[3];
> };
The comment for fe_length needs to change, too, because it needs to
indicate that it is the logical extent length and that it may be
different to the fe_phys_length depending on the flags that are set
on the extent.
And, FWIW, I wouldn't mention specific flags in the comment here,
but do it at the definition of the flags that indicate there is
a difference between physical and logical extent lengths
> @@ -50,6 +52,8 @@ struct fiemap {
> * Sets EXTENT_UNKNOWN. */
> #define FIEMAP_EXTENT_ENCODED0x0008 /* Data can not be
> read
> * while fs is unmounted */
> +#define FIEMAP_EXTENT_DATA_COMPRESSED0x0040 /* Data is
> compressed by fs.
> + * Sets EXTENT_ENCODED */
i.e. here.
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] xfstests: tree updated to 0a7f216b
Hi folks, I've just committed some outstanding patches to the xfstests repository. The commits are listed below for your information, with the current head being: 0a7f216b7962fd15e0fd749110776ca69b718932 generic: add a rename fsync test Some book-keeping stuff follows. Firstly, I'm going to look into adding commit hooks into the xfstests repository to get it send automated commit message emails when the repo is updated like is done for the main xfs kernel repository. That will save having to walk through every patch replying to say "applied". I'd like to start by getting those emails sent to the XFS, btrfs and ext4 mailing lists, but I'm open to having them sent elsewhere, too. I'm open to ideas about whether this is too many lists or whether there are other lists we should send such email to. Secondly, you might note that I rewrote the subject lines of the commits below. The main reason for doing this is to make the git log readable and to give an idea of what was changed in the commit. So, some new conventions I'd like to see people try to use for their xfstests patch descriptions. 1. the first word describes the tests/ subdirectory the change belongs to. e.g. xfs, btrfs, generic, shared, etc 2. if it's an infrastructure change or touches multiple different sets of tests, then use "xfstests" as the keyword 3. Don't use test numbers in commit messages or subject lines for new tests. We typically have to renumber tests as part of the commit process, so if we forget to modify the commit message then it looks rather strange 4. when fixing a specific test, always refer to it as "/", such as generic/273 or btrfs/022. This is needed because we can have duplicate test names in different sub directories. Hence a typical set of subjects might be: xfs: fix fubaroo in xfs/299 generic: use correct frobnozzle on generic/230 xfstests: prevent bozo errors when compiling ltp/iogen.c This makes life easier when browsing the commit history. I'll be modifying subject lines as I process them to follow these conventions, so if people adopt them it makes it better for everyone. Lastly, I'm going to try to keep the commit latency of reviewed xfstests patches to around 24-48 hours after the patch has been reviewed. I don't want reviewed patches to sit around for weeks before they are committed, so please ping the patch if it's been reviewed and not committed after a couple of days. Thanks all, Dave. Anand Jain (1): * [ed14876] btrfs: test if raids are actually created Brian Foster (1): * [0746f7b] generic: use correct size value in generic/273 Christoph Hellwig (1): * [c041421] xfstests: stop special casing nfs and udf Jie Liu (1): * [5bcbff9] xfs: verify xfs_quota commands against invalid mount path Josef Bacik (3): * [cb5dd61] btrfs: add basic qgroup testing * [640d1e1] generic: add new test for fsync() on directories * [0a7f216] generic: add a rename fsync test Miao Xie (1): * [bd50b75] btrfs: add wrong compression type regression test -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 14/18] xfs: use generic posix ACL infrastructure
On Sun, Dec 01, 2013 at 03:59:17AM -0800, Christoph Hellwig wrote: > Also create inodes with the proper mode instead of fixing it up later. > > Signed-off-by: Christoph Hellwig Nice cleanup work, Christoph. Reviewed-by: Dave Chinner -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Jfs-discussion] [PATCH 15/18] jfs: use generic posix ACL infrastructure
On 12/01/2013 05:59 AM, Christoph Hellwig wrote: > Copy the scheme I introduced to btrfs many years ago to only use the > xattr handler for ACLs, but pass plain attrs straight through. Looks good. > > Signed-off-by: Christoph Hellwig Reviewed-by: Dave Kleikamp > --- > fs/jfs/acl.c | 105 -- > fs/jfs/file.c |4 +- > fs/jfs/jfs_acl.h |7 +--- > fs/jfs/jfs_xattr.h |2 + > fs/jfs/namei.c |1 + > fs/jfs/super.c |2 + > fs/jfs/xattr.c | 108 > ++-- > 7 files changed, 89 insertions(+), 140 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: missing /sbin/fsck.btrfs
On Tue, Nov 26, 2013 at 08:06:36PM -0700, Chris Murphy wrote: > > On Nov 26, 2013, at 5:51 PM, Dave Chinner > wrote: > > > On Mon, Nov 25, 2013 at 11:40:49PM -0700, Chris Murphy wrote: > >> Hi, > >> > >> Is there supposed to be an /sbin/fsck.btrfs? I'm seeing a > >> handful of threads indicating some idea of having it just do a > >> no-op like fsck.xfs does, but then also the idea that > >> /etc/fstab should correctly set fs_passno to 0 instead of such > >> trickery. > > > > You're missing a key thing that fsck.xfs does that fstab expects > > to work - it fails with an error if the device is missing. If > > the device is present, then fsck.xfs returns success. > > The description of fs_passno taken literally doesn't account for > this explanation. It just says if fs_passno is not present or > zero, a value of zero is returned and fsck will assume that the > filesystem does not need to be checked. I'm not commenting on what fstab does or does not do - I commented on the incorrect assertion that was made about fsck.xfs being a no-op. > So the fstab expects (or is it systemd or an fsck instance spawned > by systemd?) this device present/missing flag to occur is a > convention? Or by design? Seems goofy. fstab expects that if it is asked for the filesystem to be checked and the device is missing, then fsck. will return an error because the device is missing and it could not be checked > > We did this because people were having problems when devices > > took a long time to instantiate (e.g. SAN, iscsi and other > > remote devices) and the 'device exists' check prevents > > /etc/fstab trying to mount the filesystems before they are > > present and then throwing a hissy fit…. > > OK so you're saying you'd want rootfs on XFS to have its fstab > entry retain an fs_passno of 1? No, I didn't say that. I just explained that things can go wrong if you don't detect certain types of errors in fsck. when it is called from fstab processing. What I am implying here is that we cannot prevent users from setting passno to 1 or 2 in /etc/fstab. We have no control over that and so asserting that "we don't need a fsck.btrfs because we can set passno to 0" is invalid. IOWs, fsck.btrfs needs to be present and it needs to behave correctly in these cases Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] xfstests,btrfs: add wrong compress type regression test
On Tue, Nov 26, 2013 at 03:00:23PM +0800, Miao Xie wrote: > Btrfs would crash when the users wrote some data into a file with compress > flag but the compression of the fs was disabled. This test case is to check > this bug still happen or not. > > Signed-off-by: Miao Xie Looks good now. Reviewed-by: Dave Chinner -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: missing /sbin/fsck.btrfs
On Mon, Nov 25, 2013 at 11:40:49PM -0700, Chris Murphy wrote: > Hi, > > Is there supposed to be an /sbin/fsck.btrfs? I'm seeing a handful > of threads indicating some idea of having it just do a no-op like > fsck.xfs does, but then also the idea that /etc/fstab should > correctly set fs_passno to 0 instead of such trickery. You're missing a key thing that fsck.xfs does that fstab expects to work - it fails with an error if the device is missing. If the device is present, then fsck.xfs returns success. We did this because people were having problems when devices took a long time to instantiate (e.g. SAN, iscsi and other remote devices) and the 'device exists' check prevents /etc/fstab trying to mount the filesystems before they are present and then throwing a hissy fit Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] xfstests,btrfs: add wrong compress type regression test
On Tue, Nov 26, 2013 at 02:01:17PM +0800, Miao Xie wrote: > Btrfs would crash when the users wrote some data into a file with compress > flag but the compression of the fs was disabled. This test case is to check > this bug still happen or not. > > Signed-off-by: Miao Xie > --- > Changlog v1 -> v2: > - address the commit from Dave Chinner. Testing every change before posting them for review is a good habit to develop. This: > + $XFS_IO_PROG -f -c "pwrite 0 1M" -c sync $work_file | _filter_xfs_io will dump this: wrote 1048576/1048576 bytes at offset 0 XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) command "sync" not found into the output file and hence the test will fail as the golden image file in the patch was not updated to match the new test output. It also points out that "-c sync" command is invalid - the command to fsync a file is "-c fsync". Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xfstests,btrfs: add wrong compress type regression test
On Mon, Nov 25, 2013 at 06:28:44PM +0800, Miao Xie wrote:
> A test to check if the oops will happen when the users write some
> data into the files whose compress flag is set but the compression
> of the fs is disabled.
>
> Signed-off-by: Miao Xie
> ---
> tests/btrfs/022 | 82
> +
> tests/btrfs/022.out | 5
> tests/btrfs/group | 1 +
> 3 files changed, 88 insertions(+)
> create mode 100755 tests/btrfs/022
> create mode 100644 tests/btrfs/022.out
>
> diff --git a/tests/btrfs/022 b/tests/btrfs/022
> new file mode 100755
> index 000..0755fb5
> --- /dev/null
> +++ b/tests/btrfs/022
> @@ -0,0 +1,82 @@
> +#! /bin/bash
> +# FS QA Test No. btrfs/008
> +#
> +# btrfs wrong compression type regression test, from a user report on
> +# linux-btrfs.
That doesn't tell me what is being tested. The commit message above
is what should be here.
> +#
> +#---
> +# Copyright (c) 2013 Fujitsu. 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
> +#
> +#---
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +
> +status=1 # failure is the default!
> +
> +_cleanup()
> +{
> + rm -f $tmp.*
> +}
> +
> +trap "_cleanup ; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs btrfs
> +_supported_os Linux
> +_require_scratch
> +
> +__workout()
> +{
> + work_file="$SCRATCH_MNT/tmpfile"
> +
> + touch $work_file || _fail "failed create test file"
> + chattr =c $work_file || _fail "failed change attribute of test file"
> + dd if=/dev/zero of=$work_file bs=1M count=1 oflag=sync &> /dev/null
> + if [ $? -ne 0 ]; then
> + _fail "failed write data into the test file"
> + fi
There is no need to run _fail on all of these commands. If touch
fails, it will emit an error that will end up in the output file,
and the golden output match will fail and the diff will tell you
exactly what the error was.
Also, xfs_io is preferable to using dd, and filtering the output
into the output file will give error detection and reporting, just
like for touch and chattr
__workout()
{
work_file="$SCRATCH_MNT/tmpfile"
touch $work_file
chattr =c $work_file
$XFS_IO_PROG -f -c "pwrite 0 1M" -c sync $work_file | _filter_xfs_io
}
> +echo "*** test nodatacow"
> +_scratch_mkfs > /dev/null 2>&1
> +_scratch_mount "-o nodatacow"
> +__workout
> +_scratch_unmount
_check_scratch_fs?
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] xfstests: add regression test for running btrfs balance and defrag concurrently
On Wed, Oct 30, 2013 at 10:12:52PM +0800, Liu Bo wrote:
> The test aims to trigger snapshot-aware defrag path in write endio by
> running balance, which is not expected and leads to a crash.
>
> Signed-off-by: Liu Bo
> ---
> v2: remove useless words as required.
>
> tests/btrfs/021 | 90
> +++
> tests/btrfs/021.out |2 +
> tests/btrfs/group |1 +
> 3 files changed, 93 insertions(+), 0 deletions(-)
> create mode 100755 tests/btrfs/021
> create mode 100644 tests/btrfs/021.out
>
> diff --git a/tests/btrfs/021 b/tests/btrfs/021
> new file mode 100755
> index 000..23eff47
> --- /dev/null
> +++ b/tests/btrfs/021
> @@ -0,0 +1,90 @@
> +#! /bin/bash
> +# FS QA Test No. 021
> +#
> +# A regression test of running btrfs balance and defrag concurrently.
> +#
> +# The test aims to trigger snapshot-aware defrag path in endio by
> +# running balance, which is not expected and leads to a crash.
> +#
> +#---
> +# Copyright (c) 2013 Oracle. 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
> +#---
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1 # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +cd /
> +rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +run_test()
> +{
> + $BTRFS_UTIL_PROG balance start $SCRATCH_MNT >> $seqres.full &
> +
> + sleep 0.5
> +
> + find $SCRATCH_MNT -type f -print0 | xargs -0 $BTRFS_UTIL_PROG
> filesystem defrag -f
Lines longer than 80 columns.
> +
> + sync
> + wait
> +}
> +
> +# Modify as appropriate.
Comment not needed.
> +_supported_fs btrfs
> +_supported_os Linux
> +_require_scratch
> +
> +rm -f $seqres.full
> +
> +_scratch_mkfs > /dev/null 2>&1
> +_scratch_mount
> +
> +
> +for ((j=0; j<100; j++))
> +do
> + touch $SCRATCH_MNT/padding-$j
> +done
> +
> +for ((j=0; j<50; j++))
> +do
> + for i in `seq 20 -1 1`
Please us a single syntax for for loops.
for j in `seq 0 50`; do
for i in `seq 20 -1 1`; do
...
> + do
> + dd if=/dev/zero of=$SCRATCH_MNT/foo-$j bs=4k count=1 seek=$i
> oflag=direct conv=notrunc 2>/dev/null;
> + done
Using xfs_io is preferable to dd.
$XFS_IO_PROG -f -d -c "pwrite $(($i * 4096)) 4096" \
$SCRATCH_MNT/foo-$j | _filter_xfs_io
> +done
> +
> +sync
Why do direct IO if you then run sync? If you are trying to fragment
the file, then please add a comment that you are doing a backwards
write to fragment it and how it interacts with the "touch loop"
above it. That way, if in future backwards writes are fixed to no
longer fragment files, we know that this test needs a new method of
fragmenting files...
> +# success, all done
> +echo "Silence is golden"
And with the use of the _filter_xfs_io, you get detection of write
failures due to xfs_io failures because the golden image match will
fail
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
