public inbox for linux-ext4@vger.kernel.org
 help / color / mirror / Atom feed
From: Qu Wenruo <quwenruo@cn.fujitsu.com>
To: Eric Sandeen <sandeen@sandeen.net>, Theodore Ts'o <tytso@mit.edu>,
	Eryu Guan <eguan@redhat.com>
Cc: <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: Fri, 7 Apr 2017 08:48:20 +0800	[thread overview]
Message-ID: <519beb0e-f82c-91dc-4eae-7c61e9ee69fa@cn.fujitsu.com> (raw)
In-Reply-To: <36663e81-9520-0c52-5e72-1d54ddbfb0b7@sandeen.net>



At 04/07/2017 12:28 AM, 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?
>
> I had the same question.  If the exact behavior isn't defined anywhere,
> I don't know what we can be testing, TBH.

For unmount cleanup, it's acceptable.

But if we're getting different result even we didn't modify the fs 
during the same mount duration, fiemap still changes, then it's at least 
anti-instinct.

And unfortunately, btrfs and ext3 shares the same problem here:

=== Fiemap after cycle mount ===
/mnt/scratch/file:
  EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
    0: [0..95]:         139264..139359      96 0x1000
    1: [96..127]:       139360..139391      32 0x1000
    2: [128..511]:      138112..138495     384 0x1000
    3: [512..1023]:     138752..139263     512 0x1000
    4: [1024..2047]:    143360..144383    1024 0x1000
    5: [2048..4095]:    145408..147455    2048 0x1001
=== Fiemap after cycle mount and sleep ===
/mnt/scratch/file:
  EXT: FILE-OFFSET      BLOCK-RANGE      TOTAL FLAGS
    0: [0..127]:        139264..139391     128 0x1000
    1: [128..511]:      138112..138495     384 0x1000
    2: [512..1023]:     138752..139263     512 0x1000
    3: [1024..2047]:    143360..144383    1024 0x1000
    4: [2048..4095]:    145408..147455    2048 0x1001

I was using the same excuse just as you guys, until I found btrfs is 
merging extent maps at read time, which causes the problem.
That's why the 2nd fiemap in btrfs returns merged result.

We fix btrfs by caching fiemap extent result before calling 
fiemap_fill_next_extent(), and try to merge with cached result.
Which fixes the problem quite easy.

Thanks,
Qu
>
> -Eric
>
>> 		       		   	   - Ted
>
>



  reply	other threads:[~2017-04-07  0:48 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20170403070923.18518-1-quwenruo@cn.fujitsu.com>
2017-04-05  2:35 ` [PATCH] fstests: generic: Check if cycle mount and sleep can affect fiemap result Eryu Guan
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 [this message]
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

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=519beb0e-f82c-91dc-4eae-7c61e9ee69fa@cn.fujitsu.com \
    --to=quwenruo@cn.fujitsu.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: 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