* regression in ext4_ind_remove_space
@ 2015-01-30 18:56 Chris J Arges
2015-01-30 21:14 ` Chris J Arges
0 siblings, 1 reply; 6+ messages in thread
From: Chris J Arges @ 2015-01-30 18:56 UTC (permalink / raw)
To: Lukas Czerner, Theodore Ts'o, Jan Kara; +Cc: linux-fsdevel
Hi,
Users of non-extent ext4 filesystems (ext4 ^extents, or ext3 w/
CONFIG_EXT4_USE_FOR_EXT23=y) can encounter data corruption when using
fallocate with FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE flags. This
seems to be a regression in ext4_ind_remove_space introduced in
4f579ae7, whereas commit 77ea2a4b passes the following test case.
To reproduce this issue do the following:
1) Setup ext4 ^extents, or ext3 filesystem with CONFIG_EXT4_USE_FOR_EXT23=y
2) Create and install a VM using a qcow2 image and store the file on the
filesystem
3) Snapshot the image with qemu-img
4) Boot the image and do some disk operations (fio,etc)
5) Shutdown image and delete snapshot
6) Repeat 3-5 until VM no longer boots due to image corruption,
generally this takes a few iterations depending on disk operations.
In addition, I've tested this with a single vCPU and single host CPU and
the problem persists. Running the same test on ext4 w/ extents exhibits
no failures.
Any ideas for bug hunting here? Commit 4f579ae7 completely re-writes
ext4_ind_remove_space. A revert of that commit from master fixes the
issue, but I'm unsure if that un-fixes other things. I'm happy to
continue debugging, or run any tests as necessary.
Thanks,
--chris j arges
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: regression in ext4_ind_remove_space
2015-01-30 18:56 regression in ext4_ind_remove_space Chris J Arges
@ 2015-01-30 21:14 ` Chris J Arges
2015-02-01 21:46 ` Chris J Arges
0 siblings, 1 reply; 6+ messages in thread
From: Chris J Arges @ 2015-01-30 21:14 UTC (permalink / raw)
To: Lukas Czerner, Theodore Ts'o, Jan Kara; +Cc: linux-fsdevel
On 01/30/2015 12:56 PM, Chris J Arges wrote:
> Hi,
>
> Users of non-extent ext4 filesystems (ext4 ^extents, or ext3 w/
> CONFIG_EXT4_USE_FOR_EXT23=y) can encounter data corruption when using
> fallocate with FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE flags. This
> seems to be a regression in ext4_ind_remove_space introduced in
> 4f579ae7, whereas commit 77ea2a4b passes the following test case.
>
> To reproduce this issue do the following:
> 1) Setup ext4 ^extents, or ext3 filesystem with CONFIG_EXT4_USE_FOR_EXT23=y
> 2) Create and install a VM using a qcow2 image and store the file on the
> filesystem
> 3) Snapshot the image with qemu-img
> 4) Boot the image and do some disk operations (fio,etc)
> 5) Shutdown image and delete snapshot
> 6) Repeat 3-5 until VM no longer boots due to image corruption,
> generally this takes a few iterations depending on disk operations.
>
> In addition, I've tested this with a single vCPU and single host CPU and
> the problem persists. Running the same test on ext4 w/ extents exhibits
> no failures.
>
> Any ideas for bug hunting here? Commit 4f579ae7 completely re-writes
> ext4_ind_remove_space. A revert of that commit from master fixes the
> issue, but I'm unsure if that un-fixes other things.
Well, after a bit more extensive testing a revert of this patch still
has failures. This may just be a novel bug.
I'll run longer tests on 77ea2a4b as well.
--chris
I'm happy to
> continue debugging, or run any tests as necessary.
>
> Thanks,
> --chris j arges
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: regression in ext4_ind_remove_space
2015-01-30 21:14 ` Chris J Arges
@ 2015-02-01 21:46 ` Chris J Arges
2015-02-02 12:39 ` Lukáš Czerner
0 siblings, 1 reply; 6+ messages in thread
From: Chris J Arges @ 2015-02-01 21:46 UTC (permalink / raw)
To: Lukas Czerner, Theodore Ts'o, Jan Kara; +Cc: linux-fsdevel
On 01/30/2015 03:14 PM, Chris J Arges wrote:
>
>
> On 01/30/2015 12:56 PM, Chris J Arges wrote:
>> Hi,
>>
>> Users of non-extent ext4 filesystems (ext4 ^extents, or ext3 w/
>> CONFIG_EXT4_USE_FOR_EXT23=y) can encounter data corruption when using
>> fallocate with FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE flags. This
>> seems to be a regression in ext4_ind_remove_space introduced in
>> 4f579ae7, whereas commit 77ea2a4b passes the following test case.
>>
>> To reproduce this issue do the following:
>> 1) Setup ext4 ^extents, or ext3 filesystem with CONFIG_EXT4_USE_FOR_EXT23=y
>> 2) Create and install a VM using a qcow2 image and store the file on the
>> filesystem
>> 3) Snapshot the image with qemu-img
>> 4) Boot the image and do some disk operations (fio,etc)
>> 5) Shutdown image and delete snapshot
>> 6) Repeat 3-5 until VM no longer boots due to image corruption,
>> generally this takes a few iterations depending on disk operations.
>>
>> In addition, I've tested this with a single vCPU and single host CPU and
>> the problem persists. Running the same test on ext4 w/ extents exhibits
>> no failures.
>>
>> Any ideas for bug hunting here? Commit 4f579ae7 completely re-writes
>> ext4_ind_remove_space. A revert of that commit from master fixes the
>> issue, but I'm unsure if that un-fixes other things.
>
> Well, after a bit more extensive testing a revert of this patch still
> has failures. This may just be a novel bug.
>
> I'll run longer tests on 77ea2a4b as well.
> --chris
77ea2a4b eventually fails (~20 iterations) but seems to be a bit more
resilient. So this does not seem like a regression; but still
ext4_ind_remove_space is at the heart of the issue. If I use the
following code snippet below, I can run the test case for days (~400
iterations so far) without failure:
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 5653fa4..e14cdfe 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3367,6 +3367,10 @@ int ext4_punch_hole(struct inode *inode, loff_t
offset, loff_t length)
unsigned int credits;
int ret = 0;
+ /* EXTENTS required */
+ if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)))
+ return -EOPNOTSUPP;
+
if (!S_ISREG(inode->i_mode))
return -EOPNOTSUPP;
--chris
>
> I'm happy to
>> continue debugging, or run any tests as necessary.
>>
>> Thanks,
>> --chris j arges
>>
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: regression in ext4_ind_remove_space
2015-02-01 21:46 ` Chris J Arges
@ 2015-02-02 12:39 ` Lukáš Czerner
2015-02-02 13:54 ` Lukáš Czerner
0 siblings, 1 reply; 6+ messages in thread
From: Lukáš Czerner @ 2015-02-02 12:39 UTC (permalink / raw)
To: Chris J Arges; +Cc: Theodore Ts'o, Jan Kara, linux-fsdevel
On Sun, 1 Feb 2015, Chris J Arges wrote:
> Date: Sun, 01 Feb 2015 15:46:06 -0600
> From: Chris J Arges <chris.j.arges@canonical.com>
> To: Lukas Czerner <lczerner@redhat.com>, Theodore Ts'o <tytso@mit.edu>,
> Jan Kara <jack@suse.cz>
> Cc: linux-fsdevel@vger.kernel.org
> Subject: Re: regression in ext4_ind_remove_space
>
>
>
> On 01/30/2015 03:14 PM, Chris J Arges wrote:
> >
> >
> > On 01/30/2015 12:56 PM, Chris J Arges wrote:
> >> Hi,
> >>
> >> Users of non-extent ext4 filesystems (ext4 ^extents, or ext3 w/
> >> CONFIG_EXT4_USE_FOR_EXT23=y) can encounter data corruption when using
> >> fallocate with FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE flags. This
> >> seems to be a regression in ext4_ind_remove_space introduced in
> >> 4f579ae7, whereas commit 77ea2a4b passes the following test case.
> >>
> >> To reproduce this issue do the following:
> >> 1) Setup ext4 ^extents, or ext3 filesystem with CONFIG_EXT4_USE_FOR_EXT23=y
> >> 2) Create and install a VM using a qcow2 image and store the file on the
> >> filesystem
> >> 3) Snapshot the image with qemu-img
> >> 4) Boot the image and do some disk operations (fio,etc)
> >> 5) Shutdown image and delete snapshot
> >> 6) Repeat 3-5 until VM no longer boots due to image corruption,
> >> generally this takes a few iterations depending on disk operations.
> >>
> >> In addition, I've tested this with a single vCPU and single host CPU and
> >> the problem persists. Running the same test on ext4 w/ extents exhibits
> >> no failures.
> >>
> >> Any ideas for bug hunting here? Commit 4f579ae7 completely re-writes
> >> ext4_ind_remove_space. A revert of that commit from master fixes the
> >> issue, but I'm unsure if that un-fixes other things.
> >
> > Well, after a bit more extensive testing a revert of this patch still
> > has failures. This may just be a novel bug.
> >
> > I'll run longer tests on 77ea2a4b as well.
> > --chris
>
> 77ea2a4b eventually fails (~20 iterations) but seems to be a bit more
> resilient. So this does not seem like a regression; but still
> ext4_ind_remove_space is at the heart of the issue. If I use the
> following code snippet below, I can run the test case for days (~400
> iterations so far) without failure:
>
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 5653fa4..e14cdfe 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -3367,6 +3367,10 @@ int ext4_punch_hole(struct inode *inode, loff_t
> offset, loff_t length)
> unsigned int credits;
> int ret = 0;
>
> + /* EXTENTS required */
> + if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)))
> + return -EOPNOTSUPP;
> +
> if (!S_ISREG(inode->i_mode))
> return -EOPNOTSUPP;
Hi Chris,
unfortunately you're right. I've noticed this recently as well and I
am looking into this, however the indirect block handling code is
damn complicated (but maybe it's just me) and I was not able to find
the root cause of the problem yet.
Btw, this can also be very easily reproduced by running xfstest
generic/270 on 1k file system. That way I am able to reproduce the
problem in more than 90% runs.
Now since Al Viro was the one who wrote most of the indirect code and
since he is much smarter than me, maybe we can kindly ask him to
take a look at the ext4_ind_remove_space() to figure out where the
problem is ?
Al, could you take a quick look at this ?
Thanks!
-Lukas
>
>
> --chris
>
> >
> > I'm happy to
> >> continue debugging, or run any tests as necessary.
> >>
> >> Thanks,
> >> --chris j arges
> >>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: regression in ext4_ind_remove_space
2015-02-02 12:39 ` Lukáš Czerner
@ 2015-02-02 13:54 ` Lukáš Czerner
2015-02-02 16:21 ` Chris J Arges
0 siblings, 1 reply; 6+ messages in thread
From: Lukáš Czerner @ 2015-02-02 13:54 UTC (permalink / raw)
To: Chris J Arges; +Cc: Theodore Ts'o, Jan Kara, linux-fsdevel, Al Viro
[-- Attachment #1: Type: TEXT/PLAIN, Size: 4289 bytes --]
On Mon, 2 Feb 2015, Lukáš Czerner wrote:
> Date: Mon, 2 Feb 2015 13:39:07 +0100 (CET)
> From: Lukáš Czerner <lczerner@redhat.com>
> To: Chris J Arges <chris.j.arges@canonical.com>
> Cc: Theodore Ts'o <tytso@mit.edu>, Jan Kara <jack@suse.cz>,
> linux-fsdevel@vger.kernel.org
> Subject: Re: regression in ext4_ind_remove_space
>
> On Sun, 1 Feb 2015, Chris J Arges wrote:
>
> > Date: Sun, 01 Feb 2015 15:46:06 -0600
> > From: Chris J Arges <chris.j.arges@canonical.com>
> > To: Lukas Czerner <lczerner@redhat.com>, Theodore Ts'o <tytso@mit.edu>,
> > Jan Kara <jack@suse.cz>
> > Cc: linux-fsdevel@vger.kernel.org
> > Subject: Re: regression in ext4_ind_remove_space
> >
> >
> >
> > On 01/30/2015 03:14 PM, Chris J Arges wrote:
> > >
> > >
> > > On 01/30/2015 12:56 PM, Chris J Arges wrote:
> > >> Hi,
> > >>
> > >> Users of non-extent ext4 filesystems (ext4 ^extents, or ext3 w/
> > >> CONFIG_EXT4_USE_FOR_EXT23=y) can encounter data corruption when using
> > >> fallocate with FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE flags. This
> > >> seems to be a regression in ext4_ind_remove_space introduced in
> > >> 4f579ae7, whereas commit 77ea2a4b passes the following test case.
> > >>
> > >> To reproduce this issue do the following:
> > >> 1) Setup ext4 ^extents, or ext3 filesystem with CONFIG_EXT4_USE_FOR_EXT23=y
> > >> 2) Create and install a VM using a qcow2 image and store the file on the
> > >> filesystem
> > >> 3) Snapshot the image with qemu-img
> > >> 4) Boot the image and do some disk operations (fio,etc)
> > >> 5) Shutdown image and delete snapshot
> > >> 6) Repeat 3-5 until VM no longer boots due to image corruption,
> > >> generally this takes a few iterations depending on disk operations.
> > >>
> > >> In addition, I've tested this with a single vCPU and single host CPU and
> > >> the problem persists. Running the same test on ext4 w/ extents exhibits
> > >> no failures.
> > >>
> > >> Any ideas for bug hunting here? Commit 4f579ae7 completely re-writes
> > >> ext4_ind_remove_space. A revert of that commit from master fixes the
> > >> issue, but I'm unsure if that un-fixes other things.
> > >
> > > Well, after a bit more extensive testing a revert of this patch still
> > > has failures. This may just be a novel bug.
> > >
> > > I'll run longer tests on 77ea2a4b as well.
> > > --chris
> >
> > 77ea2a4b eventually fails (~20 iterations) but seems to be a bit more
> > resilient. So this does not seem like a regression; but still
> > ext4_ind_remove_space is at the heart of the issue. If I use the
> > following code snippet below, I can run the test case for days (~400
> > iterations so far) without failure:
> >
> > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> > index 5653fa4..e14cdfe 100644
> > --- a/fs/ext4/inode.c
> > +++ b/fs/ext4/inode.c
> > @@ -3367,6 +3367,10 @@ int ext4_punch_hole(struct inode *inode, loff_t
> > offset, loff_t length)
> > unsigned int credits;
> > int ret = 0;
> >
> > + /* EXTENTS required */
> > + if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)))
> > + return -EOPNOTSUPP;
> > +
> > if (!S_ISREG(inode->i_mode))
> > return -EOPNOTSUPP;
>
> Hi Chris,
>
> unfortunately you're right. I've noticed this recently as well and I
> am looking into this, however the indirect block handling code is
> damn complicated (but maybe it's just me) and I was not able to find
> the root cause of the problem yet.
>
> Btw, this can also be very easily reproduced by running xfstest
> generic/270 on 1k file system. That way I am able to reproduce the
> problem in more than 90% runs.
>
> Now since Al Viro was the one who wrote most of the indirect code and
> since he is much smarter than me, maybe we can kindly ask him to
> take a look at the ext4_ind_remove_space() to figure out where the
> problem is ?
>
> Al, could you take a quick look at this ?
Actually adding Al to CC.
-Lukas
>
> Thanks!
> -Lukas
>
> >
> >
> > --chris
> >
> > >
> > > I'm happy to
> > >> continue debugging, or run any tests as necessary.
> > >>
> > >> Thanks,
> > >> --chris j arges
> > >>
> >
> --
> 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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: regression in ext4_ind_remove_space
2015-02-02 13:54 ` Lukáš Czerner
@ 2015-02-02 16:21 ` Chris J Arges
0 siblings, 0 replies; 6+ messages in thread
From: Chris J Arges @ 2015-02-02 16:21 UTC (permalink / raw)
To: Lukáš Czerner
Cc: Theodore Ts'o, Jan Kara, linux-fsdevel, Al Viro
On 02/02/2015 07:54 AM, Lukáš Czerner wrote:
> On Mon, 2 Feb 2015, Lukáš Czerner wrote:
>
>> Date: Mon, 2 Feb 2015 13:39:07 +0100 (CET)
>> From: Lukáš Czerner <lczerner@redhat.com>
>> To: Chris J Arges <chris.j.arges@canonical.com>
>> Cc: Theodore Ts'o <tytso@mit.edu>, Jan Kara <jack@suse.cz>,
>> linux-fsdevel@vger.kernel.org
>> Subject: Re: regression in ext4_ind_remove_space
>>
>> On Sun, 1 Feb 2015, Chris J Arges wrote:
>>
>>> Date: Sun, 01 Feb 2015 15:46:06 -0600
>>> From: Chris J Arges <chris.j.arges@canonical.com>
>>> To: Lukas Czerner <lczerner@redhat.com>, Theodore Ts'o <tytso@mit.edu>,
>>> Jan Kara <jack@suse.cz>
>>> Cc: linux-fsdevel@vger.kernel.org
>>> Subject: Re: regression in ext4_ind_remove_space
>>>
>>>
>>>
>>> On 01/30/2015 03:14 PM, Chris J Arges wrote:
>>>>
>>>>
>>>> On 01/30/2015 12:56 PM, Chris J Arges wrote:
>>>>> Hi,
>>>>>
>>>>> Users of non-extent ext4 filesystems (ext4 ^extents, or ext3 w/
>>>>> CONFIG_EXT4_USE_FOR_EXT23=y) can encounter data corruption when using
>>>>> fallocate with FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE flags. This
>>>>> seems to be a regression in ext4_ind_remove_space introduced in
>>>>> 4f579ae7, whereas commit 77ea2a4b passes the following test case.
>>>>>
>>>>> To reproduce this issue do the following:
>>>>> 1) Setup ext4 ^extents, or ext3 filesystem with CONFIG_EXT4_USE_FOR_EXT23=y
>>>>> 2) Create and install a VM using a qcow2 image and store the file on the
>>>>> filesystem
>>>>> 3) Snapshot the image with qemu-img
>>>>> 4) Boot the image and do some disk operations (fio,etc)
>>>>> 5) Shutdown image and delete snapshot
>>>>> 6) Repeat 3-5 until VM no longer boots due to image corruption,
>>>>> generally this takes a few iterations depending on disk operations.
>>>>>
>>>>> In addition, I've tested this with a single vCPU and single host CPU and
>>>>> the problem persists. Running the same test on ext4 w/ extents exhibits
>>>>> no failures.
>>>>>
>>>>> Any ideas for bug hunting here? Commit 4f579ae7 completely re-writes
>>>>> ext4_ind_remove_space. A revert of that commit from master fixes the
>>>>> issue, but I'm unsure if that un-fixes other things.
>>>>
>>>> Well, after a bit more extensive testing a revert of this patch still
>>>> has failures. This may just be a novel bug.
>>>>
>>>> I'll run longer tests on 77ea2a4b as well.
>>>> --chris
>>>
>>> 77ea2a4b eventually fails (~20 iterations) but seems to be a bit more
>>> resilient. So this does not seem like a regression; but still
>>> ext4_ind_remove_space is at the heart of the issue. If I use the
>>> following code snippet below, I can run the test case for days (~400
>>> iterations so far) without failure:
>>>
>>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
>>> index 5653fa4..e14cdfe 100644
>>> --- a/fs/ext4/inode.c
>>> +++ b/fs/ext4/inode.c
>>> @@ -3367,6 +3367,10 @@ int ext4_punch_hole(struct inode *inode, loff_t
>>> offset, loff_t length)
>>> unsigned int credits;
>>> int ret = 0;
>>>
>>> + /* EXTENTS required */
>>> + if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)))
>>> + return -EOPNOTSUPP;
>>> +
>>> if (!S_ISREG(inode->i_mode))
>>> return -EOPNOTSUPP;
>>
>> Hi Chris,
>>
>> unfortunately you're right. I've noticed this recently as well and I
>> am looking into this, however the indirect block handling code is
>> damn complicated (but maybe it's just me) and I was not able to find
>> the root cause of the problem yet.
>>
>> Btw, this can also be very easily reproduced by running xfstest
>> generic/270 on 1k file system. That way I am able to reproduce the
>> problem in more than 90% runs.
Lukáš,
Test case 270 doesn't seem to fail when I run with ext3 and 1024 byte
blocks. Are there other steps I need to do to see this issue?
Thanks,
--chris
>>
>> Now since Al Viro was the one who wrote most of the indirect code and
>> since he is much smarter than me, maybe we can kindly ask him to
>> take a look at the ext4_ind_remove_space() to figure out where the
>> problem is ?
>>
>> Al, could you take a quick look at this ?
>
> Actually adding Al to CC.
>
> -Lukas
>
>>
>> Thanks!
>> -Lukas
>>
>>>
>>>
>>> --chris
>>>
>>>>
>>>> I'm happy to
>>>>> continue debugging, or run any tests as necessary.
>>>>>
>>>>> Thanks,
>>>>> --chris j arges
>>>>>
>>>
>> --
>> 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
--
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-02-02 16:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-30 18:56 regression in ext4_ind_remove_space Chris J Arges
2015-01-30 21:14 ` Chris J Arges
2015-02-01 21:46 ` Chris J Arges
2015-02-02 12:39 ` Lukáš Czerner
2015-02-02 13:54 ` Lukáš Czerner
2015-02-02 16:21 ` Chris J Arges
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).