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: Fri, 28 Feb 2014 06:35:33 +1100 [thread overview]
Message-ID: <20140227193533.GB30131@dastard> (raw)
In-Reply-To: <alpine.LFD.2.00.1402271257590.2247@localhost.localdomain>
On Thu, Feb 27, 2014 at 01:03:09PM +0100, Lukáš Czerner wrote:
> On Thu, 27 Feb 2014, Dave Chinner wrote:
> > > xfs/242 fails on ppc64 with latest linus tree
> >
> > OK, that's a different kettle of fish. It's using 64k pages, right?
>
> 64k pages, yes.
....
> > Yeah, ok, the data in all the files is correct - the md5sums all
> > match. What's different? Just about all unwritten extents are now
> > written (i.e. data) or contain some portion of written extents.
> >
> > So, ZERO_RANGE has the following size threshold for converting
> > blocks to unwritten extents vs just zeroing them:
> >
> > granularity = max_t(uint, 1 << mp->m_sb.sb_blocklog, PAGE_CACHE_SIZE);
> >
> > So if this is a 64k page size machine, it's going to have different
> > extent layout compared to a 4k page size machine. The file data will
> > still be the same, the difference will be zeroed blocks instead of
> > unwritten blocks, and that's exactly what we see.
> >
> > IOWs, the result in terms of data the application sees is correct,
> > just the extent layout representing that zeroed data is different.
>
> Ok, so that's yet another difference between xfs and ext4 code which
> makes having generic test even more complicated.
It actually makes the need for a generic test more important. A
generic test should handle the differences between block/page size
behaviours - the issue is that xfs/242 was written and tested on 4k
page machines, not 64k page machines.
We've already got the "multiple" code in _generic_test_punch to
increase the size of the regions being worked on, so the simple fix
here is to increase the sizes so that 64k page and 4k page behaviour
is the same and results in the same extent layout. This is the
normal way tests evolve as people run them on different hardware and
configurations....
> So as I said before
> I'll make the generic test (using _filter_hole_fiemap) and then ext4
> specific test as well to really make sure that the extent
> manipulation is right.
Which ignores the fact that if you turn off zeroout on ext4 then a
generic test can also determine that the extent manipulation is
correct. I really don't see the need for an ext4 specific test
here...
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-27 19:35 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 [this message]
2014-02-28 12:38 ` Lukáš Czerner
2014-02-26 14:58 ` Lukáš Czerner
2014-02-26 22:17 ` Dave Chinner
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=20140227193533.GB30131@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).