On 2/23/12 12:20 PM, Eric Sandeen wrote:
> Running a test like the m_uninit test runs:
>
> # mke2fs -F -o Linux -O uninit_bg testfs 131072
>
> results in groups which have blocks allocated for metadata, but
> which still have the BLOCK_UNINIT flag set:
>
> Group 2: (Blocks 16385-24576) [INODE_UNINIT, BLOCK_UNINIT, ITABLE_ZEROED]
> Checksum 0x17c2, unused inodes 2048
> Block bitmap at 16385 (+0), Inode bitmap at 16386 (+1)
> Inode table at 16387-16642 (+2)
>
> however, e2fsck doesn't find this error - which seems like another bug.
>
> Doing another test like this:
>
> mkfs.ext4 -I 256 -b 4096 -N 256 -G 1 -g 256 /dev/loop0 1024
> mount /dev/loop0 /mnt/test
> mkdir /mnt/test/dir
> umount /mnt/test
> fsck.ext4 -fn /dev/loop0
>
> does find the error on fsck, I think because BLOCK_UNINIT gets
> cleared, and the group isn't ignored at fsck time:
>
> Free blocks count wrong for group #2 (253, counted=249).
>
> In the first case, flex_bg is not set; in the 2nd it is set,
> but s_log_groups_per_flex == 0 (due to -G 1), so in neither
> case is flexbg_size set in ext2fs_allocate_group_table().
>
> This leads to skipping the code which clears BLOCK_UNINIT after
> the metadata is allocated.
>
> I think the fix is to unconditionally clear the BLOCK_UNINIT flag
> in whatever block group gets metadata allocated to it, as follows.
I guess that's not right after talking to adilger. I have to punt
on this for now, though, other deadlines approach. Testcases above
if anyone else wants to look.
-Eric
> the m_uninit test needs to be fixed up too, as does e2fsck it seems,
> to catch this error in the first place?
>
> Sorry for the long commit message, if the patch looks right I can
> try to do better ;)
>
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---
>
> diff --git a/lib/ext2fs/alloc_tables.c b/lib/ext2fs/alloc_tables.c
> index 9f3d4e0..3462e85 100644
> --- a/lib/ext2fs/alloc_tables.c
> +++ b/lib/ext2fs/alloc_tables.c
> @@ -88,6 +88,7 @@ errcode_t ext2fs_allocate_group_table(ext2_filsys fs, dgrp_t group,
> blk64_t group_blk, start_blk, last_blk, new_blk, blk;
> dgrp_t last_grp = 0;
> int rem_grps = 0, flexbg_size = 0;
> + dgrp_t gr;
>
> group_blk = ext2fs_group_first_block2(fs, group);
> last_blk = ext2fs_group_last_block2(fs, group);
> @@ -141,13 +142,13 @@ errcode_t ext2fs_allocate_group_table(ext2_filsys fs, dgrp_t group,
> return retval;
> ext2fs_mark_block_bitmap2(bmap, new_blk);
> ext2fs_block_bitmap_loc_set(fs, group, new_blk);
> + gr = ext2fs_group_of_blk2(fs, new_blk);
> + ext2fs_bg_flags_clear(fs, gr, EXT2_BG_BLOCK_UNINIT);
> if (flexbg_size) {
> - dgrp_t gr = ext2fs_group_of_blk2(fs, new_blk);
> ext2fs_bg_free_blocks_count_set(fs, gr, ext2fs_bg_free_blocks_count(fs, gr) - 1);
> ext2fs_free_blocks_count_add(fs->super, -1);
> - ext2fs_bg_flags_clear(fs, gr, EXT2_BG_BLOCK_UNINIT);
> - ext2fs_group_desc_csum_set(fs, gr);
> }
> + ext2fs_group_desc_csum_set(fs, gr);
> }
>
> if (flexbg_size) {
> @@ -172,13 +173,13 @@ errcode_t ext2fs_allocate_group_table(ext2_filsys fs, dgrp_t group,
> return retval;
> ext2fs_mark_block_bitmap2(bmap, new_blk);
> ext2fs_inode_bitmap_loc_set(fs, group, new_blk);
> + gr = ext2fs_group_of_blk2(fs, new_blk);
> + ext2fs_bg_flags_clear(fs, gr, EXT2_BG_BLOCK_UNINIT);
> if (flexbg_size) {
> - dgrp_t gr = ext2fs_group_of_blk2(fs, new_blk);
> ext2fs_bg_free_blocks_count_set(fs, gr, ext2fs_bg_free_blocks_count(fs, gr) - 1);
> ext2fs_free_blocks_count_add(fs->super, -1);
> - ext2fs_bg_flags_clear(fs, gr, EXT2_BG_BLOCK_UNINIT);
> - ext2fs_group_desc_csum_set(fs, gr);
> }
> + ext2fs_group_desc_csum_set(fs, gr);
> }
>
> /*
> @@ -209,14 +210,13 @@ errcode_t ext2fs_allocate_group_table(ext2_filsys fs, dgrp_t group,
> j < fs->inode_blocks_per_group;
> j++, blk++) {
> ext2fs_mark_block_bitmap2(bmap, blk);
> + gr = ext2fs_group_of_blk2(fs, blk);
> + ext2fs_bg_flags_clear(fs, gr, EXT2_BG_BLOCK_UNINIT);
> if (flexbg_size) {
> - dgrp_t gr = ext2fs_group_of_blk2(fs, blk);
> ext2fs_bg_free_blocks_count_set(fs, gr, ext2fs_bg_free_blocks_count(fs, gr) - 1);
> ext2fs_free_blocks_count_add(fs->super, -1);
> - ext2fs_bg_flags_clear(fs, gr,
> - EXT2_BG_BLOCK_UNINIT);
> - ext2fs_group_desc_csum_set(fs, gr);
> }
> + ext2fs_group_desc_csum_set(fs, gr);
> }
> ext2fs_inode_table_loc_set(fs, group, new_blk);
> }
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
