From: Chris Mason <clm@fb.com>
To: <miaox@cn.fujitsu.com>, <dsterba@suse.cz>,
Fengguang Wu <fengguang.wu@intel.com>,
Dave Hansen <dave.hansen@intel.com>,
LKML <linux-kernel@vger.kernel.org>, <lkp@01.org>,
"linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>,
Abhay Sachan <lkp.abhay@gmail.com>
Subject: Re: [btrfs] 8d875f95: xfstests.generic.226.fail
Date: Wed, 20 Aug 2014 10:07:44 -0400 [thread overview]
Message-ID: <53F4ABB0.9000600@fb.com> (raw)
In-Reply-To: <53F47E07.3080807@cn.fujitsu.com>
On 08/20/2014 06:52 AM, Miao Xie wrote:
> On Tue, 19 Aug 2014 10:58:09 -0400, Chris Mason wrote:
>> On 08/19/2014 10:23 AM, David Sterba wrote:
>>> On Tue, Aug 19, 2014 at 07:58:20PM +0800, Fengguang Wu wrote:
>>>> We noticed an xfstests failure on commit
>>>>
>>>> 8d875f95da43c6a8f18f77869f2ef26e9594fecc ("btrfs: disable strict file flushes for renames and truncates")
>>>>
>>>> It's 100% reproducible in the 5 test runs.
>>>
>>> Same here, different mkfs configurations.
>>>
>>> generic/226 28s ... [16:11:52] [16:12:55] - output mismatch (see /root/xfstests/results//generic/226.out.bad)
>>> --- tests/generic/226.out 2013-05-29 17:16:03.000000000 +0200
>>> +++ /root/xfstests/results//generic/226.out.bad 2014-08-19 16:12:55.000000000 +0200
>>> @@ -1,6 +1,8 @@
>>> QA output created by 226
>>> --> mkfs 256m filesystem
>>> --> 16 buffered 64m writes in a loop
>>> -1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16
>>> +1 2 3 4 pwrite64: No space left on device
>>> +5 6 7 8 9 10 11 12 pwrite64: No space left on device
>>> +13 14 15 16
>>>
>>> enospc on a small filesystem (256M)
>>
>> I'm calling filemap flush more often, but otherwise everything else is
>> the same. I'll take a look.
>
> I found the problem is caused by the following function:
>
> int btrfs_release_file(struct inode *inode, struct file *filp)
> {
> ...
> filemap_flush(inode->i_mapping);
> return 0;
> }
>
> I don't think we need flush file at most situation. Ext4 flushes the file only
> after someone truncate the file to be zero-length, I don't know the real reason
> why ext4 flush the file only after the file is truncated, someone said it is to
> reduce the risk that the users find a zero-length file after a crash, which happens
> after truncate-write-close process.
>
> If we change btrfs_release_file by ext4's implementation, both the failure of
> xfstests's generic/226 and performance regression can be fixed.
>
You're completely right, my original had more checks here and I stripped
them out by accident. Fixing, thanks!
-chris
WARNING: multiple messages have this Message-ID (diff)
From: Chris Mason <clm@fb.com>
To: lkp@lists.01.org
Subject: Re: [btrfs] 8d875f95: xfstests.generic.226.fail
Date: Wed, 20 Aug 2014 10:07:44 -0400 [thread overview]
Message-ID: <53F4ABB0.9000600@fb.com> (raw)
In-Reply-To: <53F47E07.3080807@cn.fujitsu.com>
[-- Attachment #1: Type: text/plain, Size: 2135 bytes --]
On 08/20/2014 06:52 AM, Miao Xie wrote:
> On Tue, 19 Aug 2014 10:58:09 -0400, Chris Mason wrote:
>> On 08/19/2014 10:23 AM, David Sterba wrote:
>>> On Tue, Aug 19, 2014 at 07:58:20PM +0800, Fengguang Wu wrote:
>>>> We noticed an xfstests failure on commit
>>>>
>>>> 8d875f95da43c6a8f18f77869f2ef26e9594fecc ("btrfs: disable strict file flushes for renames and truncates")
>>>>
>>>> It's 100% reproducible in the 5 test runs.
>>>
>>> Same here, different mkfs configurations.
>>>
>>> generic/226 28s ... [16:11:52] [16:12:55] - output mismatch (see /root/xfstests/results//generic/226.out.bad)
>>> --- tests/generic/226.out 2013-05-29 17:16:03.000000000 +0200
>>> +++ /root/xfstests/results//generic/226.out.bad 2014-08-19 16:12:55.000000000 +0200
>>> @@ -1,6 +1,8 @@
>>> QA output created by 226
>>> --> mkfs 256m filesystem
>>> --> 16 buffered 64m writes in a loop
>>> -1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16
>>> +1 2 3 4 pwrite64: No space left on device
>>> +5 6 7 8 9 10 11 12 pwrite64: No space left on device
>>> +13 14 15 16
>>>
>>> enospc on a small filesystem (256M)
>>
>> I'm calling filemap flush more often, but otherwise everything else is
>> the same. I'll take a look.
>
> I found the problem is caused by the following function:
>
> int btrfs_release_file(struct inode *inode, struct file *filp)
> {
> ...
> filemap_flush(inode->i_mapping);
> return 0;
> }
>
> I don't think we need flush file at most situation. Ext4 flushes the file only
> after someone truncate the file to be zero-length, I don't know the real reason
> why ext4 flush the file only after the file is truncated, someone said it is to
> reduce the risk that the users find a zero-length file after a crash, which happens
> after truncate-write-close process.
>
> If we change btrfs_release_file by ext4's implementation, both the failure of
> xfstests's generic/226 and performance regression can be fixed.
>
You're completely right, my original had more checks here and I stripped
them out by accident. Fixing, thanks!
-chris
next prev parent reply other threads:[~2014-08-20 14:08 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-16 7:52 [btrfs] 4c468fd7485: +7.8% blogbench.write_score, -5.1% turbostat.Pkg_W Fengguang Wu
2014-08-16 7:52 ` Fengguang Wu
[not found] ` <CAKHchJwQcH326Z8otiaXtKoKs6fH9s+e9BqJDi=H1bWKiuybAw@mail.gmail.com>
2014-08-16 13:10 ` Fengguang Wu
2014-08-16 13:10 ` Fengguang Wu
2014-08-19 11:58 ` [btrfs] 8d875f95: xfstests.generic.226.fail Fengguang Wu
2014-08-19 11:58 ` Fengguang Wu
2014-08-19 14:23 ` David Sterba
2014-08-19 14:23 ` David Sterba
2014-08-19 14:58 ` Chris Mason
2014-08-19 14:58 ` Chris Mason
2014-08-20 10:52 ` Miao Xie
2014-08-20 10:52 ` Miao Xie
2014-08-20 14:07 ` Chris Mason [this message]
2014-08-20 14:07 ` Chris Mason
2014-08-20 14:48 ` [PATCH] Btrfs: fix filemap_flush call in btrfs_file_release Chris Mason
2014-08-20 14:48 ` Chris Mason
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=53F4ABB0.9000600@fb.com \
--to=clm@fb.com \
--cc=dave.hansen@intel.com \
--cc=dsterba@suse.cz \
--cc=fengguang.wu@intel.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lkp.abhay@gmail.com \
--cc=lkp@01.org \
--cc=miaox@cn.fujitsu.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 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.