All of lore.kernel.org
 help / color / mirror / Atom feed
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) &&



             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.