From: Dave Chinner <david@fromorbit.com>
To: "Lukáš Czerner" <lczerner@redhat.com>
Cc: linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org,
xfs@oss.sgi.com
Subject: Re: [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range
Date: Thu, 27 Feb 2014 09:17:57 +1100 [thread overview]
Message-ID: <20140226221757.GD29907@dastard> (raw)
In-Reply-To: <alpine.LFD.2.00.1402261537040.2243@localhost.localdomain>
On Wed, Feb 26, 2014 at 03:58:36PM +0100, Lukáš Czerner wrote:
> On Wed, 26 Feb 2014, Dave Chinner wrote:
> > > Currently xfs/242 fails on xfs for me
> >
> > Really? Where's the bug report? I haven't seen a failure on xfs/242
> > on any of my test machines for at least a year, even on 1k block
> > size filesystems...
> >
> > $ sudo ./check xfs/242
> > 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
> >
> > xfs/242 1s ... 0s
> > Ran: xfs/242
> > Passed all 1 tests
> > $
>
> Ok so once again. Yesterday was rally too late and I've
> misinterpreted the diff. It's not that xfs behaves differently, but
> rather ext4 behaves differently because we in fact have a code that
> will zero out entire unwritten extent if it's small enough rather
> than split it into unwritten and written.
Yes, we've come across this before, and there are several solutions.
this one is used in tests/generic/285:
# Disable extent zeroing for ext4 as that change where holes ar created
if [ "$FSTYP" = "ext4" ]; then
DEV=`basename $TEST_DEV`
echo 0 >/sys/fs/ext4/$DEV/extent_max_zeroout_kb
fi
> > > and it does behave differently than ext4.
> >
> > In what way? Does FALLOC_FL_ZERO_RANGE on XFS behave identically to
> > XFS_IOC_ZERO_RANGE, or is that different too? Or you haven't tested
> > it because you wrote this test as an ext4 specific test and so
> > haven't run this specific test exercising the FALLOC_FL_ZERO_RANGE
> > path in XFS?
>
> It does behave differently, but not because of zero_range code, but
> rather when writing into uninitialized extent which is small enough.
> The extent will not be split but rather converted to initialized and
> respective parts will be zeroed out.
>
> Btw that's actually the reason why we use
>
> _filter_hole_fiemap
>
> filtes instead of
>
> _filter_fiemap
>
> I've used in ext4/242.
Sure, but even so I think we might do better just to use the above
zeroout tune and be explicit in what we expect to happen w.r.t. data
vs holes...
>
> That said I think that both tests fs specific and fs independent
> have it's value so I'll create generic/242 as well by using
> _filter_hole_fiemap.
Just use the first unused generic test number - trying to keep test
numbers the same across different subdirs is just going to cause
confusion....
> > hole punching - the only difference between a hole punch and a zero
> > range on filesystems that use unwritten extents should be that the
> > range being operated on has unwritten extents rather a hole.....
> >
> > > Btw this kind of optimization is actually something I've been
> > > thinking of as well for ext4. Rather than going though the hassle of
> > > changing extents around it might be worth in some situation to zero
> > > out. But that's an optimization I have not implemented yet.
> >
> > Exactly my point - until such optimisations are implemented, all the
> > filesystems should be behaving the same way using unwritten extents,
> > just like for hole punching. Hence the tests should be checking that
> > the behaviour is the same across filesystems, just like we do for
> > hole punching.
>
> Using _filter_hole_fiemap filter in such test we would not make a
> difference between unwritten and written extent. However in the case
> of zero_range this somewhat make the test much less effective so
> it'll be worth having fs specific test as well as generic test I
> said above.
>
> Or we could actually directly inspect the data as we do in xfs/290, or
> generic/290 respectively.
The md5sum does the data inspection for us. The whole point of
hole punch and zero range and so one is that they are extent
manipulation operations. If we don't check that extents have been
manipulated correctly, then we aren't testing that the key behaviour
the filesystems are supposed to display for those operations....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2014-02-26 22:18 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-25 19:15 [PATCH 1/6] generic/290: Add test for fallocate zero range Lukas Czerner
2014-02-25 19:15 ` [PATCH 2/6] xfstests: Add fallocate zero range operation to fsstress Lukas Czerner
2014-02-25 20:15 ` Dave Chinner
2014-02-25 19:15 ` [PATCH 3/6] xfstests: fsstress punch should always have FALLOC_FL_KEEP_SIZE set Lukas Czerner
2014-02-25 20:18 ` Dave Chinner
2014-02-26 13:04 ` Lukáš Czerner
2014-02-25 19:15 ` [PATCH 4/6] xfstests: Add fallocate zero range operation to fsx Lukas Czerner
2014-02-25 20:31 ` Dave Chinner
2014-02-25 19:15 ` [PATCH 5/6] xfstests: Define fallocate flags locally in fsx Lukas Czerner
2014-02-25 20:39 ` Dave Chinner
2014-02-25 21:56 ` Christoph Hellwig
2014-02-26 6:07 ` Dave Chinner
2014-02-25 19:15 ` [PATCH 6/6] ext4/242: Add ext4 specific test for fallocate zero range Lukas Czerner
2014-02-25 20:53 ` Dave Chinner
2014-02-25 21:01 ` Lukáš Czerner
2014-02-25 21:27 ` Lukáš Czerner
2014-02-25 21:50 ` Dave Chinner
2014-02-26 14:24 ` Lukáš Czerner
2014-02-26 22:01 ` Dave Chinner
2014-02-27 12:03 ` Lukáš Czerner
2014-02-27 19:35 ` Dave Chinner
2014-02-28 12:38 ` Lukáš Czerner
2014-02-26 14:58 ` Lukáš Czerner
2014-02-26 22:17 ` Dave Chinner [this message]
2014-02-27 11:48 ` Lukáš Czerner
2014-02-25 20:01 ` [PATCH 1/6] generic/290: Add " Dave Chinner
2014-02-25 20:55 ` Dave Chinner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140226221757.GD29907@dastard \
--to=david@fromorbit.com \
--cc=lczerner@redhat.com \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=xfs@oss.sgi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).