From: Eryu Guan <guan@eryu.me>
To: Boris Burkov <boris@bur.io>
Cc: "Darrick J. Wong" <djwong@kernel.org>,
linux-btrfs@vger.kernel.org, kernel-team@fb.com,
fstests@vger.kernel.org
Subject: Re: [PATCH v3] generic: test fiemap offsets and < 512 byte ranges
Date: Sun, 25 Apr 2021 13:14:07 +0800 [thread overview]
Message-ID: <YIT6n8Y2pmQb5Y5t@desktop> (raw)
In-Reply-To: <YIB5XbY2PX4J5dlN@zen>
On Wed, Apr 21, 2021 at 12:13:33PM -0700, Boris Burkov wrote:
> On Sun, Apr 11, 2021 at 10:03:59PM +0800, Eryu Guan wrote:
> > On Wed, Apr 07, 2021 at 01:13:26PM -0700, Boris Burkov wrote:
> > > btrfs trims fiemap extents to the inputted offset, which leads to
> > > inconsistent results for most inputs, and downright bizarre outputs like
> > > [7..6] when the trimmed extent is at the end of an extent and shorter
> > > than 512 bytes.
> > >
> > > The test writes out one extent of the file system's block size and tries
> > > fiemaps at various offsets. It expects that all the fiemaps return the
> > > full single extent.
> > >
> > > I ran it under the following fs, block size combinations:
> > > ext2: 1024, 2048, 4096
> > > ext3: 1024, 2048, 4096
> > > ext4: 1024, 2048, 4096
> > > xfs: 512, 1024, 2048, 4096
> > > f2fs: 4096
> > > btrfs: 4096
> > >
> > > This test is fixed for btrfs by:
> > > btrfs: return whole extents in fiemap
> > > (https://lore.kernel.org/linux-btrfs/274e5bcebdb05a8969fc300b4802f33da2fbf218.1617746680.git.boris@bur.io/)
> > >
> > > Signed-off-by: Boris Burkov <boris@bur.io>
> >
> > generic/473, which tests fiemap, has been marked as broken, as fiemap
> > behavior is not consistent across filesystems, and the specific behavior
> > tested by generic/473 is not defined and filesystems could have
> > different implementations.
> >
> > I'm not sure if this test fits into the undefined-behavior fiemap
> > categary. I think it's fine if it tests a well-defined & consistent
> > behavior.
> >
>
> Interesting, I didn't know about that test being marked as broken.
>
> I was worried about this problem to some extent and attempted to
> mitigate it by only requiring that all the output be the same, rather
> than matching some specific standard.
>
> Thinking about it further, I think this test is portable only so long as
> the step where it writes a file with one extent is portable.
>
> If "pwrite 0 block-size" ends up as a file with multiple extents, then
> it is possible one of the partial fiemaps will only intersect with a
> subset of the extents and rightly return those. In fact, that was broken
> in the original version of the test which explicitly used 4096 instead of
> being detecting the block size.
>
> I do think it is nice to have this as a regression test for btrfs, since
> we have pretty complicated logic for fiemap and it was so broken in this
> case. If you prefer, I can make this a btrfs specific test.
Yeah, a btrfs specific test seems safer, and we could move it to generic
later if the behavior is well defined.
Thanks,
Eryu
next prev parent reply other threads:[~2021-04-25 5:14 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-06 22:47 [PATCH] generic: test fiemap offsets and < 512 byte ranges Boris Burkov
2021-04-06 22:54 ` [PATCH v2] " Boris Burkov
2021-04-07 16:10 ` Darrick J. Wong
2021-04-07 20:13 ` [PATCH v3] " Boris Burkov
2021-04-11 14:03 ` Eryu Guan
2021-04-21 19:13 ` Boris Burkov
2021-04-25 5:14 ` Eryu Guan [this message]
2021-04-07 20:27 ` [PATCH v2] " Boris Burkov
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=YIT6n8Y2pmQb5Y5t@desktop \
--to=guan@eryu.me \
--cc=boris@bur.io \
--cc=djwong@kernel.org \
--cc=fstests@vger.kernel.org \
--cc=kernel-team@fb.com \
--cc=linux-btrfs@vger.kernel.org \
/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).