libext2fs: use proper functions to set/clear block group flags

PATCH Superseded Eric Sandeen 3 participants 5 messages Sep 2, 2009 – Sep 6, 2009

lore

Share & tools

lore

Eric Sandeen

author

2009-09-02 20:36 UTC #0 lore raw

PATCH 1/1 libext2fs: use proper functions to set/clear block group flags

As Justin & Ric reported, something like this on a 22T (sparse) bigfile:

e2fsprogs/misc/mke2fs -E lazy_itable_init=1 \
-O uninit_bg -b 4096 bigfile
mount -o loop bigfile mnt/
for I in seq 1 5; do mkdir mnt/$I; done umount mnt/
e2fsprogs/e2fsck/e2fsck -f bigfile

would give us corrupted block group checksums:

<other problems related to uninit snipped>
One or more block group descriptor checksums are invalid. Fix<y>? yes

Group descriptor 6301 checksum is invalid. FIXED. Group descriptor 7799 checksum is invalid. FIXED.

There wer=re a few places which accessed bg_flags directly rather than using the helper functions; fixing these seems to resolve the problem.

Reported-by Justin Maggard <[email protected]>

Reported-by Ric Wheeler <[email protected]>

Signed-off-by Eric Sandeen <[email protected]>

---

Applies to the pu branch

e2fsck/pass2.c +3 -3

Index: e2fsprogs/e2fsck/pass2.c
===================================================================
@@ -987,12 +987,12 @@ out_htree:
 		 * we could call a function in pass1.c that checks the
 		 * newly visible inodes.
 		 */
-		if (fs->group_desc[group].bg_flags & EXT2_BG_INODE_UNINIT) {
+		if (ext2fs_bg_flag_test(fs, group, EXT2_BG_INODE_UNINIT)) {
 			pctx.num = dirent->inode;
 			if (fix_problem(ctx, PR_2_INOREF_BG_INO_UNINIT,
 					&cd->pctx)){
-				fs->group_desc[group].bg_flags &=
-					~EXT2_BG_INODE_UNINIT;
+				ext2fs_bg_flags_clear(fs, group,
+						      EXT2_BG_INODE_UNINIT);
 				ext2fs_mark_super_dirty(fs);
 				ctx->flags |= E2F_FLAG_RESTART_LATER;
 			} else {

lib/ext2fs/openfs.c +6 -5

Index: e2fsprogs/lib/ext2fs/openfs.c
===================================================================
@@ -350,11 +350,12 @@ errcode_t ext2fs_open2(const char *name,
 	if (superblock > 1 && EXT2_HAS_RO_COMPAT_FEATURE(fs->super,
 					EXT4_FEATURE_RO_COMPAT_GDT_CSUM)) {
 		struct ext2_group_desc *gd;
-		for (i = 0, gd = fs->group_desc; i < fs->group_desc_count;
-		     i++, gd++) {
-			gd->bg_flags &= ~EXT2_BG_BLOCK_UNINIT;
-			gd->bg_flags &= ~EXT2_BG_INODE_UNINIT;
-			gd->bg_itable_unused = 0;
+		dgrp_t group;
+
+		for (group = 0; group < fs->group_desc_count; group++) {
+			ext2fs_bg_flags_clear(fs, group, EXT2_BG_BLOCK_UNINIT);
+			ext2fs_bg_flags_clear(fs, group, EXT2_BG_INODE_UNINIT);
+			fs->group_desc[group].bg_itable_unused = 0;
 		}
 		ext2fs_mark_super_dirty(fs);
 	}

Eric Sandeen

author

2009-09-02 21:15 UTC #1 lore raw

Quoted 15 lines · Eric Sandeen

As Justin & Ric reported, something like this on a 22T (sparse) bigfile:
e2fsprogs/misc/mke2fs -E lazy_itable_init=1 \
-O uninit_bg -b 4096 bigfile
mount -o loop bigfile mnt/
for I in seq 1 5; do mkdir mnt/$I; done umount mnt/
e2fsprogs/e2fsck/e2fsck -f bigfile
would give us corrupted block group checksums:
<other problems related to uninit snipped>
One or more block group descriptor checksums are invalid. Fix<y>? yes

Quoted 6 lines

Group descriptor 6301 checksum is invalid. FIXED. Group descriptor 7799 checksum is invalid. FIXED.
There wer=re a few places which accessed bg_flags directly rather than using the helper functions; fixing these seems to resolve the problem.

Well, it resolves that problem, but further testing reveals another lurking behind it, so expect more patches ;)

-Eric

Justin Maggard

2009-09-02 21:33 UTC #2 lore raw

Quoted 2 lines · Eric Sandeen

Well, it resolves that problem, but further testing reveals another lurking behind it, so expect more patches ;)

Thanks for the update. :) My test just finished with, unfortunately, the same results.

-Justin

Eric Sandeen

author

2009-09-02 21:36 UTC #3 lore raw

PATCH v2 1/1 libext2fs: use proper functions to set/clear block group flags

In reply to Eric Sandeen

As Justin & Ric reported, something like this on a 22T (sparse) bigfile:

e2fsprogs/misc/mke2fs -E lazy_itable_init=1 \
-O uninit_bg -b 4096 bigfile
mount -o loop bigfile mnt/
for I in seq 1 5; do mkdir mnt/$I; done umount mnt/
e2fsprogs/e2fsck/e2fsck -f bigfile

would give us corrupted block group checksums:

<other problems related to uninit snipped>
One or more block group descriptor checksums are invalid. Fix<y>? yes

Group descriptor 6301 checksum is invalid. FIXED. Group descriptor 7799 checksum is invalid. FIXED.

There wer=re a few places which accessed bg_flags directly rather than using the helper functions; fixing these seems to resolve the problem.

V2: use _flag_clear not _flags_clear, which clears all flags ...

Even w/ this patch, I'm still getting bitmap mismatches, off by a 33rd bit.

Reported-by Justin Maggard <[email protected]>

Reported-by Ric Wheeler <[email protected]>

Signed-off-by Eric Sandeen <[email protected]>

---

Applies to the pu branch

e2fsck/pass2.c +3 -3

Index: e2fsprogs/e2fsck/pass2.c
===================================================================
@@ -987,12 +987,12 @@ out_htree:
 		 * we could call a function in pass1.c that checks the
 		 * newly visible inodes.
 		 */
-		if (fs->group_desc[group].bg_flags & EXT2_BG_INODE_UNINIT) {
+		if (ext2fs_bg_flag_test(fs, group, EXT2_BG_INODE_UNINIT)) {
 			pctx.num = dirent->inode;
 			if (fix_problem(ctx, PR_2_INOREF_BG_INO_UNINIT,
 					&cd->pctx)){
-				fs->group_desc[group].bg_flags &=
-					~EXT2_BG_INODE_UNINIT;
+				ext2fs_bg_flag_clear(fs, group,
+						      EXT2_BG_INODE_UNINIT);
 				ext2fs_mark_super_dirty(fs);
 				ctx->flags |= E2F_FLAG_RESTART_LATER;
 			} else {

lib/ext2fs/openfs.c +6 -5

Index: e2fsprogs/lib/ext2fs/openfs.c
===================================================================
@@ -350,11 +350,12 @@ errcode_t ext2fs_open2(const char *name,
 	if (superblock > 1 && EXT2_HAS_RO_COMPAT_FEATURE(fs->super,
 					EXT4_FEATURE_RO_COMPAT_GDT_CSUM)) {
 		struct ext2_group_desc *gd;
-		for (i = 0, gd = fs->group_desc; i < fs->group_desc_count;
-		     i++, gd++) {
-			gd->bg_flags &= ~EXT2_BG_BLOCK_UNINIT;
-			gd->bg_flags &= ~EXT2_BG_INODE_UNINIT;
-			gd->bg_itable_unused = 0;
+		dgrp_t group;
+
+		for (group = 0; group < fs->group_desc_count; group++) {
+			ext2fs_bg_flag_clear(fs, group, EXT2_BG_BLOCK_UNINIT);
+			ext2fs_bg_flag_clear(fs, group, EXT2_BG_INODE_UNINIT);
+			fs->group_desc[group].bg_itable_unused = 0;
 		}
 		ext2fs_mark_super_dirty(fs);
 	}

Theodore Tso

2009-09-06 16:29 UTC #4 lore raw

Quoted 15 lines · Eric Sandeen

As Justin & Ric reported, something like this on a 22T (sparse) bigfile:
e2fsprogs/misc/mke2fs -E lazy_itable_init=1 \
-O uninit_bg -b 4096 bigfile
mount -o loop bigfile mnt/
for I in seq 1 5; do mkdir mnt/$I; done umount mnt/
e2fsprogs/e2fsck/e2fsck -f bigfile
would give us corrupted block group checksums:
<other problems related to uninit snipped>
One or more block group descriptor checksums are invalid. Fix<y>? yes

Quoted 8 lines

Group descriptor 6301 checksum is invalid. FIXED. Group descriptor 7799 checksum is invalid. FIXED.
There wer=re a few places which accessed bg_flags directly rather than using the helper functions; fixing these seems to resolve the problem.
V2: use _flag_clear not _flags_clear, which clears all flags ...

Added to the e2fsprogs 64-bits patch set (and thus the pu branch).

- Ted