From: Qu Wenruo <quwenruo@cn.fujitsu.com> To: Eric Sandeen <sandeen@sandeen.net>, "Darrick J. Wong" <darrick.wong@oracle.com>, Eryu Guan <eguan@redhat.com> Cc: "Theodore Ts'o" <tytso@mit.edu>, <linux-btrfs@vger.kernel.org>, <fstests@vger.kernel.org>, <linux-ext4@vger.kernel.org> Subject: Re: [PATCH] fstests: generic: Check if cycle mount and sleep can affect fiemap result Date: Mon, 10 Apr 2017 10:07:54 +0800 [thread overview] Message-ID: <e1d76075-1244-f671-b5e7-1e8851d4f0c1@cn.fujitsu.com> (raw) In-Reply-To: <d6e81d34-fda4-d242-01bd-270436177771@sandeen.net> At 04/07/2017 11:48 PM, Eric Sandeen wrote: > On 4/7/17 10:42 AM, Darrick J. Wong wrote: >> On Fri, Apr 07, 2017 at 01:02:58PM +0800, Eryu Guan wrote: >>> On Thu, Apr 06, 2017 at 11:28:01AM -0500, Eric Sandeen wrote: >>>> On 4/6/17 11:26 AM, Theodore Ts'o wrote: >>>>> On Wed, Apr 05, 2017 at 10:35:26AM +0800, Eryu Guan wrote: >>>>>> >>>>>> Test fails with ext3/2 when driving with ext4 driver, fiemap changed >>>>>> after umount/mount cycle, then changed back to original result after >>>>>> sleeping some time. An ext4 bug? (cc'ed linux-ext4 list.) >>>>> >>>>> I haven't had time to look at this, but I'm not sure this test is a >>>>> reasonable one on the face of it. >>>>> >>>>> A file system may choose to optimize a file's extent tree for whatever >>>>> reason it wants, whenever it wants, including on an unmount --- and >>>>> that would not be an invalid thing to do. So to have an xfstests that >>>>> causes a test failure if a file system were to, say, do some cleanup >>>>> at mount or unmount time, or when the file is next opened, to merge >>>>> adjacent extents together (and hence change what is returned by >>>>> FIEMAP) might be strange, or even weird --- but is this any of user >>>>> space's business? Or anything we want to enforce as wrong wrong wrong >>>>> by xfstests? >>> >>> So I was asking for a review from ext4 side instead of queuing it for >>> next xfstests update :) >> >> In general FIEMAP can return pretty much whatever it wants, which >> usually means that it won't report extents larger than the underlying >> block mapping extents, though as we've seen it can split a single >> on-disk extent into multiple FIEMAP records for the purpose of reporting >> sharedness. >> >> For ext3 I'm wondering if it's the case that the first time we FIEMAP an >> indirect map file we see a possibly-merged version of whatever's in the >> particular leaf node we land in; then that information gets cached & >> merged with other records in the extent status tree, such that >> subsequent FIEMAP calls see longer extents than the first time around. >> >>>> I had the same question. If the exact behavior isn't defined anywhere, >>>> I don't know what we can be testing, TBH. >>> >>> Agreed, I was about to ask for the expected behavior today if there was >>> no new review comments on this patch. >> >> I think the expected behavior is that any behavior is expected. :( > > I suppose that if any particular filesystem wants to enforce stricter > rules for its own fiemap interface, that could be done in a > filesystem-specific test... Well, then some test cases need to be moved from generic to filesystem specified test, and some filsystems will have similar test cases. I think it's a hell for maintainer to keep all these test cases consistent. This is not the first time Btrfs failed to pass reflink and fiemap test designed for xfs though. Btrfs does skip some generic test case originally designed for xfs, like generic/372. But most of the time, xfs and btrfs can get consistent result. And for btrfs side, we already have such fix now: https://patchwork.kernel.org/patch/9668721/ It's OK that any fs can have either own fiemap behavior, but if it's a not big fix and the behavior itself is somewhat wired, why not joining the common behavior? Thanks, Qu > > -Eric > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > >
WARNING: multiple messages have this Message-ID (diff)
From: Qu Wenruo <quwenruo@cn.fujitsu.com> To: Eric Sandeen <sandeen@sandeen.net>, "Darrick J. Wong" <darrick.wong@oracle.com>, Eryu Guan <eguan@redhat.com> Cc: Theodore Ts'o <tytso@mit.edu>, linux-btrfs@vger.kernel.org, fstests@vger.kernel.org, linux-ext4@vger.kernel.org Subject: Re: [PATCH] fstests: generic: Check if cycle mount and sleep can affect fiemap result Date: Mon, 10 Apr 2017 10:07:54 +0800 [thread overview] Message-ID: <e1d76075-1244-f671-b5e7-1e8851d4f0c1@cn.fujitsu.com> (raw) In-Reply-To: <d6e81d34-fda4-d242-01bd-270436177771@sandeen.net> At 04/07/2017 11:48 PM, Eric Sandeen wrote: > On 4/7/17 10:42 AM, Darrick J. Wong wrote: >> On Fri, Apr 07, 2017 at 01:02:58PM +0800, Eryu Guan wrote: >>> On Thu, Apr 06, 2017 at 11:28:01AM -0500, Eric Sandeen wrote: >>>> On 4/6/17 11:26 AM, Theodore Ts'o wrote: >>>>> On Wed, Apr 05, 2017 at 10:35:26AM +0800, Eryu Guan wrote: >>>>>> >>>>>> Test fails with ext3/2 when driving with ext4 driver, fiemap changed >>>>>> after umount/mount cycle, then changed back to original result after >>>>>> sleeping some time. An ext4 bug? (cc'ed linux-ext4 list.) >>>>> >>>>> I haven't had time to look at this, but I'm not sure this test is a >>>>> reasonable one on the face of it. >>>>> >>>>> A file system may choose to optimize a file's extent tree for whatever >>>>> reason it wants, whenever it wants, including on an unmount --- and >>>>> that would not be an invalid thing to do. So to have an xfstests that >>>>> causes a test failure if a file system were to, say, do some cleanup >>>>> at mount or unmount time, or when the file is next opened, to merge >>>>> adjacent extents together (and hence change what is returned by >>>>> FIEMAP) might be strange, or even weird --- but is this any of user >>>>> space's business? Or anything we want to enforce as wrong wrong wrong >>>>> by xfstests? >>> >>> So I was asking for a review from ext4 side instead of queuing it for >>> next xfstests update :) >> >> In general FIEMAP can return pretty much whatever it wants, which >> usually means that it won't report extents larger than the underlying >> block mapping extents, though as we've seen it can split a single >> on-disk extent into multiple FIEMAP records for the purpose of reporting >> sharedness. >> >> For ext3 I'm wondering if it's the case that the first time we FIEMAP an >> indirect map file we see a possibly-merged version of whatever's in the >> particular leaf node we land in; then that information gets cached & >> merged with other records in the extent status tree, such that >> subsequent FIEMAP calls see longer extents than the first time around. >> >>>> I had the same question. If the exact behavior isn't defined anywhere, >>>> I don't know what we can be testing, TBH. >>> >>> Agreed, I was about to ask for the expected behavior today if there was >>> no new review comments on this patch. >> >> I think the expected behavior is that any behavior is expected. :( > > I suppose that if any particular filesystem wants to enforce stricter > rules for its own fiemap interface, that could be done in a > filesystem-specific test... Well, then some test cases need to be moved from generic to filesystem specified test, and some filsystems will have similar test cases. I think it's a hell for maintainer to keep all these test cases consistent. This is not the first time Btrfs failed to pass reflink and fiemap test designed for xfs though. Btrfs does skip some generic test case originally designed for xfs, like generic/372. But most of the time, xfs and btrfs can get consistent result. And for btrfs side, we already have such fix now: https://patchwork.kernel.org/patch/9668721/ It's OK that any fs can have either own fiemap behavior, but if it's a not big fix and the behavior itself is somewhat wired, why not joining the common behavior? Thanks, Qu > > -Eric > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" 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:[~2017-04-10 2:08 UTC|newest] Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-04-03 7:09 [PATCH] fstests: generic: Check if cycle mount and sleep can affect fiemap result Qu Wenruo 2017-04-03 7:09 ` Qu Wenruo 2017-04-03 16:31 ` Darrick J. Wong 2017-04-05 0:51 ` Qu Wenruo 2017-04-05 0:51 ` Qu Wenruo 2017-04-05 2:35 ` Eryu Guan 2017-04-05 2:42 ` Qu Wenruo 2017-04-05 2:42 ` Qu Wenruo 2017-04-06 16:26 ` Theodore Ts'o 2017-04-06 16:28 ` Eric Sandeen 2017-04-07 0:48 ` Qu Wenruo 2017-04-07 0:48 ` Qu Wenruo 2017-04-07 5:02 ` Eryu Guan 2017-04-07 15:42 ` Darrick J. Wong 2017-04-07 15:48 ` Eric Sandeen 2017-04-10 2:07 ` Qu Wenruo [this message] 2017-04-10 2:07 ` Qu Wenruo
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=e1d76075-1244-f671-b5e7-1e8851d4f0c1@cn.fujitsu.com \ --to=quwenruo@cn.fujitsu.com \ --cc=darrick.wong@oracle.com \ --cc=eguan@redhat.com \ --cc=fstests@vger.kernel.org \ --cc=linux-btrfs@vger.kernel.org \ --cc=linux-ext4@vger.kernel.org \ --cc=sandeen@sandeen.net \ --cc=tytso@mit.edu \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.