2014-02-10 10:25:20

by Azat Khuzhin

[permalink] [raw]
Subject: [PATCH] ext4: initialize multi-block allocator before checking block descriptors

with EXT4FS_DEBUG ext4_count_free_clusters() will call
ext4_read_block_bitmap() without s_group_info initialized.

Here is bt:
(gdb) bt
#0 ext4_get_group_info (group=0, sb=0xffff880079a10000) at ext4.h:2430
#1 ext4_validate_block_bitmap (sb=sb@entry=0xffff880079a10000, desc=desc@entry=0xffff880056510000, block_group=block_group@entry=0,
bh=bh@entry=0xffff88007bf2b2d8) at balloc.c:358
#2 0xffffffff81232202 in ext4_wait_block_bitmap (sb=sb@entry=0xffff880079a10000, block_group=block_group@entry=0,
bh=bh@entry=0xffff88007bf2b2d8) at balloc.c:476
#3 0xffffffff81232eaf in ext4_read_block_bitmap (sb=sb@entry=0xffff880079a10000, block_group=block_group@entry=0) at balloc.c:489
#4 0xffffffff81232fc0 in ext4_count_free_clusters (sb=sb@entry=0xffff880079a10000) at balloc.c:665
#5 0xffffffff81259ffa in ext4_check_descriptors (first_not_zeroed=<synthetic pointer>, sb=0xffff880079a10000) at super.c:2143
#6 ext4_fill_super (sb=sb@entry=0xffff880079a10000, data=<optimized out>, data@entry=0x0 <irq_stack_union>, silent=silent@entry=0)
at super.c:3851
#7 0xffffffff811b8340 in mount_bdev (fs_type=<optimized out>, flags=0, dev_name=<optimized out>, data=0x0 <irq_stack_union>,
fill_super=fill_super@entry=0xffffffff812589c0 <ext4_fill_super>) at super.c:987
#8 0xffffffff8124ec35 in ext4_mount (fs_type=<optimized out>, flags=<optimized out>, dev_name=<optimized out>, data=<optimized out>)
at super.c:5365
#9 0xffffffff811b8cf9 in mount_fs (type=type@entry=0xffffffff81c71840 <ext4_fs_type>, flags=flags@entry=0,
name=name@entry=0xffff880077a80c70 "/dev/loop4", data=data@entry=0x0 <irq_stack_union>) at super.c:1090
#10 0xffffffff811d2ff3 in vfs_kern_mount (type=type@entry=0xffffffff81c71840 <ext4_fs_type>, flags=0,
name=name@entry=0xffff880077a80c70 "/dev/loop4", data=data@entry=0x0 <irq_stack_union>) at namespace.c:813
#11 0xffffffff811d55de in do_new_mount (data=0x0 <irq_stack_union>, name=0xffff880077a80c70 "/dev/loop4", mnt_flags=32,
flags=<optimized out>, fstype=0xffff880077a80ca0 "ext4-insane", path=0xffff88007a5b1ed0) at namespace.c:2068
#12 do_mount (dev_name=0xffff880077a80c70 "/dev/loop4", dir_name=<optimized out>, type_page=0xffff880077a80ca0 "ext4-insane",
flags=<optimized out>, flags@entry=3236757504, data_page=0x0 <irq_stack_union>) at namespace.c:2392
#13 0xffffffff811d6183 in SYSC_mount (data=0x0 <irq_stack_union>, flags=3236757504, type=<optimized out>, dir_name=<optimized out>,
dev_name=0x7ffad9649c20 "/dev/loop4") at namespace.c:2586
#14 SyS_mount (dev_name=140715365800992, dir_name=<optimized out>, type=<optimized out>, flags=3236757504, data=0) at namespace.c:2559

Signed-off-by: Azat Khuzhin <[email protected]>
---
fs/ext4/super.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 1f7784d..7e8c5e4 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -3838,6 +3838,14 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)

bgl_lock_init(sbi->s_blockgroup_lock);

+ ext4_ext_init(sb);
+ err = ext4_mb_init(sb);
+ if (err) {
+ ext4_msg(sb, KERN_ERR, "failed to initialize mballoc (%d)",
+ err);
+ goto failed_mount;
+ }
+
for (i = 0; i < db_count; i++) {
block = descriptor_loc(sb, logical_sb_block, i);
sbi->s_group_desc[i] = sb_bread(sb, block);
@@ -4094,14 +4102,6 @@ no_journal:
goto failed_mount4a;
}

- ext4_ext_init(sb);
- err = ext4_mb_init(sb);
- if (err) {
- ext4_msg(sb, KERN_ERR, "failed to initialize mballoc (%d)",
- err);
- goto failed_mount5;
- }
-
err = ext4_register_li_request(sb, first_not_zeroed);
if (err)
goto failed_mount6;
@@ -4175,9 +4175,6 @@ failed_mount8:
failed_mount7:
ext4_unregister_li_request(sb);
failed_mount6:
- ext4_mb_release(sb);
-failed_mount5:
- ext4_ext_release(sb);
ext4_release_system_zone(sb);
failed_mount4a:
dput(sb->s_root);
@@ -4207,7 +4204,10 @@ failed_mount2:
for (i = 0; i < db_count; i++)
brelse(sbi->s_group_desc[i]);
ext4_kvfree(sbi->s_group_desc);
+
+ ext4_mb_release(sb);
failed_mount:
+ ext4_ext_release(sb);
if (sbi->s_chksum_driver)
crypto_free_shash(sbi->s_chksum_driver);
if (sbi->s_proc) {
--
1.8.5.2



2014-03-15 21:40:34

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] ext4: initialize multi-block allocator before checking block descriptors

>With EXT4FS_DEBUG ext4_count_free_clusters() will call
>ext4_read_block_bitmap() without s_group_info initialized, so we need to
>initialize multi-block allocator before.
>
>And we can't initialize multi-block allocator without group descriptors,
>since it use them.
>Also we need to install s_op before initializing multi-block allocator,
>because in ext4_mb_init_backend() new inode is created.

I've had to drop this patch because it's causing a series of bigalloc
failures. I'll take a closer look at this later, but I wantd to give
you a heads up.

- Ted




2014-03-15 23:54:34

by Azat Khuzhin

[permalink] [raw]
Subject: Re: [PATCH] ext4: initialize multi-block allocator before checking block descriptors

On Sun, Mar 16, 2014 at 1:40 AM, <[email protected]> wrote:
>>With EXT4FS_DEBUG ext4_count_free_clusters() will call
>>ext4_read_block_bitmap() without s_group_info initialized, so we need to
>>initialize multi-block allocator before.
>>
>>And we can't initialize multi-block allocator without group descriptors,
>>since it use them.
>>Also we need to install s_op before initializing multi-block allocator,
>>because in ext4_mb_init_backend() new inode is created.
>
> I've had to drop this patch because it's causing a series of bigalloc
> failures. I'll take a closer look at this later, but I wantd to give
> you a heads up.

Thanks Ted, I'll take a look.
Are you talking about xfs-tests failures?

>
> - Ted
>
>
>



--
Respectfully
Azat Khuzhin

2014-03-16 02:38:46

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] ext4: initialize multi-block allocator before checking block descriptors

On Sun, Mar 16, 2014 at 03:54:34AM +0400, Azat Khuzhin wrote:
>
> Thanks Ted, I'll take a look.
> Are you talking about xfs-tests failures?

Yes, the kernel is complaining that the file system looks corrupt.
The xfstest generic/001 is the easist one to trigger it:

generic/001 18s ... [17:35:49][ 26.793766] EXT4-fs error (device vde): ext4_mb_generate_buddy:756: group 1, 32763 clusters in bitmap, 32764
in gd; block bitmap corrupt.
[17:36:09] 20s
[ 39.636943] EXT4-fs (vde): warning: mounting fs with errors, running e2fsck is recommended

I this was using a freshly generated file system using a 20 GiB test
partition, formatted using:

mke2fs -t ext4 -O bigalloc /dev/vde

Dropping the patch made the error go away.

Cheers,

- Ted

2014-03-16 14:17:47

by Azat Khuzhin

[permalink] [raw]
Subject: Re: [PATCH] ext4: initialize multi-block allocator before checking block descriptors

On Sun, Mar 16, 2014 at 6:38 AM, <[email protected]> wrote:
> On Sun, Mar 16, 2014 at 03:54:34AM +0400, Azat Khuzhin wrote:
>>
>> Thanks Ted, I'll take a look.
>> Are you talking about xfs-tests failures?
>
> Yes, the kernel is complaining that the file system looks corrupt.
> The xfstest generic/001 is the easist one to trigger it:
>
> generic/001 18s ... [17:35:49][ 26.793766] EXT4-fs error (device vde): ext4_mb_generate_buddy:756: group 1, 32763 clusters in bitmap, 32764
> in gd; block bitmap corrupt.
> [17:36:09] 20s
> [ 39.636943] EXT4-fs (vde): warning: mounting fs with errors, running e2fsck is recommended
>
> I this was using a freshly generated file system using a 20 GiB test
> partition, formatted using:
>
> mke2fs -t ext4 -O bigalloc /dev/vde
>
> Dropping the patch made the error go away.

Hi Ted,

I just checked with the last upstream commit d8ec26d7f8287 + patch,
and xfs tests haven't fallen
I even update xfstests, still nothing.

/src/xfstests # git describe
linux-v3.8-336-g3948694

/src/xfstests # ./run_generic.sh
mke2fs 1.42.8 (20-Jun-2013)
/dev/sdb1 is mounted; will not make a filesystem here!
FSTYP -- ext4
PLATFORM -- Linux/x86_64 debian-virtual-2-6-39 3.13.0+
MKFS_OPTIONS -- /dev/sdc1
MOUNT_OPTIONS -- -O bigalloc /dev/sdc1 /xfs-playground/scratch

generic/001 2s ... 2s
Ran: generic/001
Passed all 1 tests

# uname -r
3.13.0+

Maybe I need more recent kernel?

>
> Cheers,
>
> - Ted



--
Respectfully
Azat Khuzhin

2014-03-16 16:17:17

by Azat Khuzhin

[permalink] [raw]
Subject: Re: [PATCH] ext4: initialize multi-block allocator before checking block descriptors

On Sun, Mar 16, 2014 at 06:10:12PM +0400, Azat Khuzhin wrote:
> On Sun, Mar 16, 2014 at 6:38 AM, <[email protected]> wrote:
> > On Sun, Mar 16, 2014 at 03:54:34AM +0400, Azat Khuzhin wrote:
> >>
> >> Thanks Ted, I'll take a look.
> >> Are you talking about xfs-tests failures?
> >
> > Yes, the kernel is complaining that the file system looks corrupt.
> > The xfstest generic/001 is the easist one to trigger it:
> >
> > generic/001 18s ... [17:35:49][ 26.793766] EXT4-fs error (device vde): ext4_mb_generate_buddy:756: group 1, 32763 clusters in bitmap, 32764
> > in gd; block bitmap corrupt.
> > [17:36:09] 20s
> > [ 39.636943] EXT4-fs (vde): warning: mounting fs with errors, running e2fsck is recommended
> >
> > I this was using a freshly generated file system using a 20 GiB test
> > partition, formatted using:
> >
> > mke2fs -t ext4 -O bigalloc /dev/vde
> >
> > Dropping the patch made the error go away.
>
> Hi Ted,
>
> I just checked with the last upstream commit d8ec26d7f8287 + patch,
> and xfs tests haven't fallen
> I even update xfstests, still nothing.
>
> /src/xfstests # git describe
> linux-v3.8-336-g3948694
>
> /src/xfstests # ./run_generic.sh
> mke2fs 1.42.8 (20-Jun-2013)
> /dev/sdb1 is mounted; will not make a filesystem here!
> FSTYP -- ext4
> PLATFORM -- Linux/x86_64 debian-virtual-2-6-39 3.13.0+
> MKFS_OPTIONS -- /dev/sdc1
> MOUNT_OPTIONS -- -O bigalloc /dev/sdc1 /xfs-playground/scratch
>
> generic/001 2s ... 2s
> Ran: generic/001
> Passed all 1 tests
>
> # uname -r
> 3.13.0+
>
> Maybe I need more recent kernel?

I've also tested on 3.14.0-rc6+: a4ecdf8 ("Merge branch
'x86-urgent-for-linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip")

And no complaints from xfs-tests, and no errors in dmesg.

Could you give me more information about you playground?

>
> >
> > Cheers,
> >
> > - Ted
>
>
>
> --
> Respectfully
> Azat Khuzhin

--
Respectfully
Azat Khuzhin

2014-03-16 18:46:41

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] ext4: initialize multi-block allocator before checking block descriptors

On Sun, Mar 16, 2014 at 08:17:13PM +0400, [email protected] wrote:
> And no complaints from xfs-tests, and no errors in dmesg.
>
> Could you give me more information about you playground?

I just reproduced the problem this way. Starting with the ext4 dev
branch of the ext4 git tree[1], which should be commit eb3e7abb161ad5,
I applied your patch, as found in the ext4 patch queue's git tree[2],
with the name initialize-multi-block-allocator-before-checking-block-descriptors

I then ran a fairly recent version of xfstests (commit 3948694eb),
under KVM with the kernel built using the 32-bit x86 architecture,
with the KVM set up with 4 CPU's, and 2048 megs of memory. The 20 gig
partition I used was formatted using "mke2fs -t ext4 -O bigalloc", and
I used the mount options "-o block_validity" (this enables some
additional sanity checking).

The ext4 errors showed up when running xfstests ext4/305 this time
around. Please see the attached compressed log file.

Cheers,

- Ted

[1] git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git
[2] git://repo.or.cz/ext4-patch-queue.git


Attachments:
(No filename) (1.08 kB)
bigalloc-fail.log.gz (35.02 kB)
Download all attachments

2014-03-16 20:57:18

by Azat Khuzhin

[permalink] [raw]
Subject: Re: [PATCH] ext4: initialize multi-block allocator before checking block descriptors

On Sun, Mar 16, 2014 at 02:46:35PM -0400, [email protected] wrote:
> On Sun, Mar 16, 2014 at 08:17:13PM +0400, [email protected] wrote:
> > And no complaints from xfs-tests, and no errors in dmesg.
> >
> > Could you give me more information about you playground?
>
> I just reproduced the problem this way. Starting with the ext4 dev
> branch of the ext4 git tree[1], which should be commit eb3e7abb161ad5,
> I applied your patch, as found in the ext4 patch queue's git tree[2],
> with the name initialize-multi-block-allocator-before-checking-block-descriptors
>
> I then ran a fairly recent version of xfstests (commit 3948694eb),
> under KVM with the kernel built using the 32-bit x86 architecture,
> with the KVM set up with 4 CPU's, and 2048 megs of memory. The 20 gig
> partition I used was formatted using "mke2fs -t ext4 -O bigalloc", and
> I used the mount options "-o block_validity" (this enables some
> additional sanity checking).
>
> The ext4 errors showed up when running xfstests ext4/305 this time
> around. Please see the attached compressed log file.

Thanks for additional information.

After I tested ext4 dev branch (eb3e7abb161ad5), without any xfs-tests complaints,
I understand what goes wrong, you have not last version of
this patch, the latest is v3.
(Actually you have description from last patch, but not the latest changes.)

I could resend the whole patch if you want to.


$ diff
initialize-multi-block-allocator-before-checking-block-descriptors <(env
GIT_DIR=/src/oss/linux/.git git show
ext4-fix-checking-descriptors-patch)

100,105c102,103
< @@ -4084,21 +4094,13 @@ no_journal:
< if (err) {
< ext4_msg(sb, KERN_ERR, "failed to reserve %llu clusters for "
< "reserved pool", ext4_calculate_resv_clusters(sb));
< - goto failed_mount4a;
< + goto failed_mount5;
---
> @@ -4094,14 +4104,6 @@ no_journal:
> goto failed_mount4a;
108,114d105
< err = ext4_setup_system_zone(sb);
< if (err) {
< ext4_msg(sb, KERN_ERR, "failed to initialize system "
< "zone (%d)", err);
< - goto failed_mount4a;
< - }
< -
120,123c111,117
< goto failed_mount5;
< }
<
< @@ -4175,11 +4177,8 @@ failed_mount8:
---
> - goto failed_mount5;
> - }
> -
> err = ext4_register_li_request(sb, first_not_zeroed);
> if (err)
> goto failed_mount6;
> @@ -4175,9 +4177,6 @@ failed_mount8:
131,132c125
< -failed_mount4a:
< +failed_mount5:
---
> failed_mount4a:
134,135d126
< sb->s_root = NULL;
< failed_mount4:

Thanks.

>
> Cheers,
>
> - Ted
>
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git
> [2] git://repo.or.cz/ext4-patch-queue.git
>



--
Respectfully
Azat Khuzhin

2014-03-17 02:00:34

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] ext4: initialize multi-block allocator before checking block descriptors

On Mon, Mar 17, 2014 at 12:56:59AM +0400, [email protected] wrote:
> After I tested ext4 dev branch (eb3e7abb161ad5), without any xfs-tests complaints,
> I understand what goes wrong, you have not last version of
> this patch, the latest is v3.
> (Actually you have description from last patch, but not the latest changes.)

What I did was to take your v3 version of the patch, and then since
that patch removed the label failed_mount5, I changed instances of
failed_mount4a to failed_mount5, just for aesthetic reasons.

So there is no substantive difference between what is in the ext4
patch queue and your v3 patch. All I did was this:

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index d73f1d9..01c5088 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -4100,14 +4100,14 @@ no_journal:
if (err) {
ext4_msg(sb, KERN_ERR, "failed to reserve %llu clusters for "
"reserved pool", ext4_calculate_resv_clusters(sb));
- goto failed_mount4a;
+ goto failed_mount5;
}

err = ext4_setup_system_zone(sb);
if (err) {
ext4_msg(sb, KERN_ERR, "failed to initialize system "
"zone (%d)", err);
- goto failed_mount4a;
+ goto failed_mount5;
}

err = ext4_register_li_request(sb, first_not_zeroed);
@@ -4184,7 +4184,7 @@ failed_mount7:
ext4_unregister_li_request(sb);
failed_mount6:
ext4_release_system_zone(sb);
-failed_mount4a:
+failed_mount5:
dput(sb->s_root);
sb->s_root = NULL;
failed_mount4:


Did you actually test your v3 patch on top of the dev branch? Or did
you just note that the patch in the ext4 patch queue was different,
and assumed it was the v2 version of your patch?

Regards,

- Ted

2014-03-17 16:19:48

by Azat Khuzhin

[permalink] [raw]
Subject: Re: [PATCH] ext4: initialize multi-block allocator before checking block descriptors

On Mon, Mar 17, 2014 at 6:00 AM, <[email protected]> wrote:
> On Mon, Mar 17, 2014 at 12:56:59AM +0400, [email protected] wrote:
>> After I tested ext4 dev branch (eb3e7abb161ad5), without any xfs-tests complaints,
>> I understand what goes wrong, you have not last version of
>> this patch, the latest is v3.
>> (Actually you have description from last patch, but not the latest changes.)
>
> What I did was to take your v3 version of the patch, and then since
> that patch removed the label failed_mount5, I changed instances of
> failed_mount4a to failed_mount5, just for aesthetic reasons.
>
> So there is no substantive difference between what is in the ext4
> patch queue and your v3 patch. All I did was this:

Sorry I didn't look good at diff of two patches, my mistake.

>
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index d73f1d9..01c5088 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -4100,14 +4100,14 @@ no_journal:
> if (err) {
> ext4_msg(sb, KERN_ERR, "failed to reserve %llu clusters for "
> "reserved pool", ext4_calculate_resv_clusters(sb));
> - goto failed_mount4a;
> + goto failed_mount5;
> }
>
> err = ext4_setup_system_zone(sb);
> if (err) {
> ext4_msg(sb, KERN_ERR, "failed to initialize system "
> "zone (%d)", err);
> - goto failed_mount4a;
> + goto failed_mount5;
> }
>
> err = ext4_register_li_request(sb, first_not_zeroed);
> @@ -4184,7 +4184,7 @@ failed_mount7:
> ext4_unregister_li_request(sb);
> failed_mount6:
> ext4_release_system_zone(sb);
> -failed_mount4a:
> +failed_mount5:
> dput(sb->s_root);
> sb->s_root = NULL;
> failed_mount4:
>
>
> Did you actually test your v3 patch on top of the dev branch? Or did
> you just note that the patch in the ext4 patch queue was different,
> and assumed it was the v2 version of your patch?

Yes, I actually test v3 patch, but not on the top of dev branch,
instead the dev branch was on the top of v3 patch.
(but it changes nothing), with "-O bigalloc" and "-o block_validity"

Also today I recheck this using kvm/qemu, with v3 patch on the top of
ext4 dev branch, and still nothing, here is last commits:
62f5f55 ext4: initialize multi-block allocator before checking block descriptors
eb3e7ab ext4: fix partial cluster handling for bigalloc file systems
97d3979 ext4: delete path dealloc code in ext4_ext_handle_uninitialized_extents

The only difference between your tests and my, is that you have i386,
while i have x86_64. But I really doubt that this is significantly.

>
> Regards,
>
> - Ted



--
Respectfully
Azat Khuzhin

2014-03-17 17:24:02

by Darrick J. Wong

[permalink] [raw]
Subject: Re: [PATCH] ext4: initialize multi-block allocator before checking block descriptors

On Sun, Mar 16, 2014 at 10:00:30PM -0400, [email protected] wrote:
> On Mon, Mar 17, 2014 at 12:56:59AM +0400, [email protected] wrote:
> > After I tested ext4 dev branch (eb3e7abb161ad5), without any xfs-tests complaints,
> > I understand what goes wrong, you have not last version of
> > this patch, the latest is v3.
> > (Actually you have description from last patch, but not the latest changes.)
>
> What I did was to take your v3 version of the patch, and then since
> that patch removed the label failed_mount5, I changed instances of
> failed_mount4a to failed_mount5, just for aesthetic reasons.

/me wonders if it would be easier just to do something like this:

ext4: don't crash when validating block bitmap

If EXT4FS_DEBUG is defined, ext4_validate_block_bitmap is called via
ext4_count_free_clusters before sb->s_group_info is set up. When this
happens, the kernel crashes because group info hasn't been loaded.
Forego marking the group corrupt for now; not setting BH_Verified
means we'll revisit the bitmap if something went wrong.

Signed-off-by: Darrick J. Wong <[email protected]>
---
fs/ext4/balloc.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index 2f35689..b375768 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -355,7 +355,10 @@ void ext4_validate_block_bitmap(struct super_block *sb,
struct buffer_head *bh)
{
ext4_fsblk_t blk;
- struct ext4_group_info *grp = ext4_get_group_info(sb, block_group);
+ struct ext4_group_info *grp = NULL;
+
+ if (EXT4_SB(sb)->s_group_info)
+ grp = ext4_get_group_info(sb, block_group);

if (buffer_verified(bh))
return;
@@ -366,14 +369,18 @@ void ext4_validate_block_bitmap(struct super_block *sb,
ext4_unlock_group(sb, block_group);
ext4_error(sb, "bg %u: block %llu: invalid block bitmap",
block_group, blk);
- set_bit(EXT4_GROUP_INFO_BBITMAP_CORRUPT_BIT, &grp->bb_state);
+ if (grp)
+ set_bit(EXT4_GROUP_INFO_BBITMAP_CORRUPT_BIT,
+ &grp->bb_state);
return;
}
if (unlikely(!ext4_block_bitmap_csum_verify(sb, block_group,
desc, bh))) {
ext4_unlock_group(sb, block_group);
ext4_error(sb, "bg %u: bad block bitmap checksum", block_group);
- set_bit(EXT4_GROUP_INFO_BBITMAP_CORRUPT_BIT, &grp->bb_state);
+ if (grp)
+ set_bit(EXT4_GROUP_INFO_BBITMAP_CORRUPT_BIT,
+ &grp->bb_state);
return;
}
set_buffer_verified(bh);

2014-03-17 17:53:25

by Azat Khuzhin

[permalink] [raw]
Subject: Re: [PATCH] ext4: initialize multi-block allocator before checking block descriptors

On Mon, Mar 17, 2014 at 08:19:47PM +0400, Azat Khuzhin wrote:
> On Mon, Mar 17, 2014 at 6:00 AM, <[email protected]> wrote:
> > On Mon, Mar 17, 2014 at 12:56:59AM +0400, [email protected] wrote:
> >> After I tested ext4 dev branch (eb3e7abb161ad5), without any xfs-tests complaints,
> >> I understand what goes wrong, you have not last version of
> >> this patch, the latest is v3.
> >> (Actually you have description from last patch, but not the latest changes.)
> >
> > What I did was to take your v3 version of the patch, and then since
> > that patch removed the label failed_mount5, I changed instances of
> > failed_mount4a to failed_mount5, just for aesthetic reasons.
> >
> > So there is no substantive difference between what is in the ext4
> > patch queue and your v3 patch. All I did was this:
>
> Sorry I didn't look good at diff of two patches, my mistake.
>
> >
> > diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> > index d73f1d9..01c5088 100644
> > --- a/fs/ext4/super.c
> > +++ b/fs/ext4/super.c
> > @@ -4100,14 +4100,14 @@ no_journal:
> > if (err) {
> > ext4_msg(sb, KERN_ERR, "failed to reserve %llu clusters for "
> > "reserved pool", ext4_calculate_resv_clusters(sb));
> > - goto failed_mount4a;
> > + goto failed_mount5;
> > }
> >
> > err = ext4_setup_system_zone(sb);
> > if (err) {
> > ext4_msg(sb, KERN_ERR, "failed to initialize system "
> > "zone (%d)", err);
> > - goto failed_mount4a;
> > + goto failed_mount5;
> > }
> >
> > err = ext4_register_li_request(sb, first_not_zeroed);
> > @@ -4184,7 +4184,7 @@ failed_mount7:
> > ext4_unregister_li_request(sb);
> > failed_mount6:
> > ext4_release_system_zone(sb);
> > -failed_mount4a:
> > +failed_mount5:
> > dput(sb->s_root);
> > sb->s_root = NULL;
> > failed_mount4:
> >
> >
> > Did you actually test your v3 patch on top of the dev branch? Or did
> > you just note that the patch in the ext4 patch queue was different,
> > and assumed it was the v2 version of your patch?
>
> Yes, I actually test v3 patch, but not on the top of dev branch,
> instead the dev branch was on the top of v3 patch.
> (but it changes nothing), with "-O bigalloc" and "-o block_validity"
>
> Also today I recheck this using kvm/qemu, with v3 patch on the top of
> ext4 dev branch, and still nothing, here is last commits:
> 62f5f55 ext4: initialize multi-block allocator before checking block descriptors
> eb3e7ab ext4: fix partial cluster handling for bigalloc file systems
> 97d3979 ext4: delete path dealloc code in ext4_ext_handle_uninitialized_extents
>
> The only difference between your tests and my, is that you have i386,
> while i have x86_64. But I really doubt that this is significantly.

I was wrong, I've just tested on i386, and got the same errors as you.

>
> >
> > Regards,
> >
> > - Ted
>
>
>
> --
> Respectfully
> Azat Khuzhin

--
Respectfully
Azat Khuzhin

2014-03-25 00:17:09

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [PATCH] ext4: initialize multi-block allocator before checking block descriptors

On Mon, Mar 17, 2014 at 09:53:02PM +0400, [email protected] wrote:
> I was wrong, I've just tested on i386, and got the same errors as you.

I assume you will resubmit this patch once you figure out how to avoid
the error on 32-bit x86 systems?

- Ted

2014-03-25 06:24:15

by Azat Khuzhin

[permalink] [raw]
Subject: Re: [PATCH] ext4: initialize multi-block allocator before checking block descriptors

On Mon, Mar 24, 2014 at 08:17:00PM -0400, [email protected] wrote:
> On Mon, Mar 17, 2014 at 09:53:02PM +0400, [email protected] wrote:
> > I was wrong, I've just tested on i386, and got the same errors as you.
>
> I assume you will resubmit this patch once you figure out how to avoid
> the error on 32-bit x86 systems?

Yes, sure, thanks.
(I just don't have enough free time right now for this.)

BTW now I have this errors on x86_64 too.

>
> - Ted

--
Respectfully
Azat Khuzhin

2014-04-05 15:51:29

by Azat Khuzhin

[permalink] [raw]
Subject: Re: [PATCH] ext4: initialize multi-block allocator before checking block descriptors

On Tue, Mar 25, 2014 at 10:24 AM, <[email protected]> wrote:
> On Mon, Mar 24, 2014 at 08:17:00PM -0400, [email protected] wrote:
>> On Mon, Mar 17, 2014 at 09:53:02PM +0400, [email protected] wrote:
>> > I was wrong, I've just tested on i386, and got the same errors as you.
>>
>> I assume you will resubmit this patch once you figure out how to avoid
>> the error on 32-bit x86 systems?
>
> Yes, sure, thanks.
> (I just don't have enough free time right now for this.)
>
> BTW now I have this errors on x86_64 too.

Hi Ted,
I just sent v4 patch, I've tested it and tests passed without any fs
corruptions.
(It based on patch from ext4-patch-queue)

Thanks.

>
>>
>> - Ted
>
> --
> Respectfully
> Azat Khuzhin



--
Respectfully
Azat Khuzhin