From: Damien Guibouret <damien.guibouret@partition-saving.com>
To: linux-ext4@vger.kernel.org
Subject: s_first_meta_bg treatment incompatibility between kernel and e2fsprogs
Date: Wed, 11 Nov 2009 13:09:00 +0100 [thread overview]
Message-ID: <4AFAA95C.5000304@partition-saving.com> (raw)
Hello,
I have taken a look at META_BG feature and think there is some
incoherency between kernel and e2fsprogs about s_first_meta_bg handling.
When considering initialisation of unitialised block bitmaps for groups
before first meta group one:
- kernel considers that descriptors blocks occupy
EXT4_SB(sb)->s_gdb_count blocks (see ext4_bg_num_gdb_nometa at
balloc.c:764 that is indirectly called from ext4_init_block_bitmap),
s_gdb_count being the number of blocks to store all descriptors
(computed from super.c).
- e2fsprogs considers that descriptors blocks occupy s_first_meta_bg
(see ext2fs_reserve_super_and_bgd at alloc_sb.c:55-68).
The difference of behaviour is wrong as e2fsck will certainly complain
there is a bitmap marked unused when it should be if the bitmap was
initialised by kernel and number of descriptors blocks is lower than
s_first_meta_bg or reverse if number of descriptors blocks is higher
than s_first_meta_bg.
So, the kernel behaviour seems to be wrong in the META_BG case when
s_first_meta_bg is not 0: ext4_bg_num_gdb returns either 1 (META_BG
feature present and group being one of the meta group) or s_gdb_count
(META_BG feature not present or group being one before first meta
group). As s_gdb_count is number of blocks for all groups, I think it
should returns either 1 (META_BG present and group being one of the meta
group) or s_first_meta_bg (META_BG present and group being one before
first meta group) or s_gdb_count (META_BG not set).
I see also that the resize2fs does not handle the s_first_meta_bg flag
in case a filesystem is shrunk such as number of descriptor blocks goes
below s_first_meta_bg. To what I looked, it does not seem to be a
problem (apart from the problem in kernel described above), but I did
not perform a complete check about that. At least there is some blocks
still allocated when there is no more need for that (but e2fsck does not
complain as it uses also the s_first_meta_bg value). I do not know if it
is desired behaviour. In case the s_first_meta_bg is lowered and blocks
freed, it will certainly be better to add a check into e2fsck to check
that s_first_meta_bg is coherent with number of descriptor blocks
(s_first_meta_bg <= fs->desc_blocks).
If you want to perform some tests on that, I modified tune2fs to allow
setting the META_BG flag on a filesystem that does not have it with
setting s_first_meta_bg to the current number of blocks for descriptors
(it is how I understand META_BG/s_first_meta_group should be used).
Regards,
Damien
*** misc/tune2fs.old 2009-11-11 12:20:33.698192912 +0100
--- misc/tune2fs.c 2009-11-11 11:38:20.265333248 +0100
***************
*** 121,126 ****
--- 121,127 ----
EXT2_FEATURE_COMPAT_DIR_INDEX,
/* Incompat */
EXT2_FEATURE_INCOMPAT_FILETYPE |
+ EXT2_FEATURE_INCOMPAT_META_BG |
EXT3_FEATURE_INCOMPAT_EXTENTS |
EXT4_FEATURE_INCOMPAT_FLEX_BG,
/* R/O compat */
***************
*** 418,423 ****
--- 419,440 ----
}
}
+ if (FEATURE_ON(E2P_FEATURE_INCOMPAT, EXT2_FEATURE_INCOMPAT_META_BG)) {
+ if (mount_flags & EXT2_MF_MOUNTED) {
+ fputs(_("The meta_bg feature may only be "
+ "set when the filesystem is\n"
+ "unmounted.\n"), stderr);
+ exit(1);
+ }
+ if (sb->s_feature_compat & EXT2_FEATURE_COMPAT_RESIZE_INODE) {
+ fputs(_("The meta_bg feature cannot be "
+ "set when the resize_inode is\n"
+ "set.\n"), stderr);
+ exit(1);
+ }
+ sb->s_first_meta_bg = fs->desc_blocks;
+ }
+
if (FEATURE_OFF(E2P_FEATURE_RO_INCOMPAT,
EXT4_FEATURE_RO_COMPAT_HUGE_FILE)) {
if ((mount_flags & EXT2_MF_MOUNTED) &&
next reply other threads:[~2009-11-11 12:07 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-11-11 12:09 Damien Guibouret [this message]
2009-11-15 4:20 ` s_first_meta_bg treatment incompatibility between kernel and e2fsprogs Theodore Tso
2009-11-15 10:28 ` Damien Guibouret
2009-11-15 19:23 ` Theodore Tso
2009-11-16 15:51 ` Damien Guibouret
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4AFAA95C.5000304@partition-saving.com \
--to=damien.guibouret@partition-saving.com \
--cc=linux-ext4@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.