| Message ID | 54CE9E9E.2040700@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 > >> > -- 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
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 >
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
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;