Re: [PATCH v2] ext4: fix initialization of UNINIT bitmap blocks

Aneesh Kumar K.V
2008-09-15T13:37:39Z

On Mon, Sep 15, 2008 at 02:16:47PM +0200, Frédéric Bohé wrote:
> From: Frederic Bohe <frederic.bohe@bull.net>
>
> Do not rely on buffer head's uptodate flag to initialize
> uninitialized bitmap blocks.
>
> Signed-off-by: Frederic Bohe <frederic.bohe@bull.net>
> ---
> Sorry there was a copy/paste error in the previous mail !
>
> This patch makes sure to initialize uninited bitmap blocks.
> These are two test cases where bugs appear because of uninited blocks :
>
> 1- This test case lead to uninited block bitmap and an error message
> from the mballocator during the second dd.
>
> dd if=/dev/urandom of=/dev/md0 bs=1M count=300
> mkfs.ext4 -t ext4dev /dev/md0 1G
> mount -t ext4dev /dev/md0 /mnt/test
> resize2fs /dev/md0 2G
> dd if=/dev/zero of=/mnt/test/dummy bs=1M count=1500
>
> Note that the first dd is to make sure we have random garbage in the
> uninited blocks. If not, you could miss the issue depending what was in
> those blocks before running mkfs.
>
> 2- This test case lead to uninited inode bitmap blocks, making it
> impossible to use all the inodes of the fs.
>
> dd if=/dev/urandom of=/dev/md0 bs=1M count=20
> mkfs.ext4 -t ext4dev /dev/md0 10M
> mount -t ext4dev /dev/md0 /mnt/test
> resize2fs /dev/md0 20M
> for i in $(seq 1 3800); do touch /mnt/test/file${i} 2>&1; done
>
> balloc.c | 4 +++-
> ialloc.c | 4 +++-
> mballoc.c | 4 +++-
> 3 files changed, 9 insertions(+), 3 deletions(-)
>
> Index: linux-2.6.27-rc5+patch_queue/fs/ext4/balloc.c
> ===================================================================
> --- linux-2.6.27-rc5+patch_queue.orig/fs/ext4/balloc.c 2008-09-15 10:59:27.000000000 +0200
> +++ linux-2.6.27-rc5+patch_queue/fs/ext4/balloc.c 2008-09-15 14:03:04.000000000 +0200
> @@ -318,9 +318,11 @@ ext4_read_block_bitmap(struct super_bloc
> block_group, bitmap_blk);
> return NULL;
> }
> - if (bh_uptodate_or_lock(bh))
> + if (buffer_uptodate(bh) &&
> + !(desc->bg_flags & cpu_to_le16(EXT4_BG_BLOCK_UNINIT)))
> return bh;
>
> + lock_buffer(bh);
> spin_lock(sb_bgl_lock(EXT4_SB(sb), block_group));
> if (desc->bg_flags & cpu_to_le16(EXT4_BG_BLOCK_UNINIT)) {
> ext4_init_block_bitmap(sb, bh, block_group, desc);

Why ? I guess resize should mark those buffer_heads as not uptodate so
that we do a reinit of block bitmap again later. The above change will
result in calling ext4_init_block_bitmap everytime we do a
read_block_bitmap on an uninit group

> Index: linux-2.6.27-rc5+patch_queue/fs/ext4/ialloc.c
> ===================================================================
> --- linux-2.6.27-rc5+patch_queue.orig/fs/ext4/ialloc.c 2008-09-15 10:59:27.000000000 +0200
> +++ linux-2.6.27-rc5+patch_queue/fs/ext4/ialloc.c 2008-09-15 11:12:16.000000000 +0200
> @@ -115,9 +115,11 @@ ext4_read_inode_bitmap(struct super_bloc
> block_group, bitmap_blk);
> return NULL;
> }
> - if (bh_uptodate_or_lock(bh))
> + if (buffer_uptodate(bh) &&
> + !(desc->bg_flags & cpu_to_le16(EXT4_BG_INODE_UNINIT)))
> return bh;
>
> + lock_buffer(bh);
> spin_lock(sb_bgl_lock(EXT4_SB(sb), block_group));
> if (desc->bg_flags & cpu_to_le16(EXT4_BG_INODE_UNINIT)) {
> ext4_init_inode_bitmap(sb, bh, block_group, desc);
> Index: linux-2.6.27-rc5+patch_queue/fs/ext4/mballoc.c
> ===================================================================
> --- linux-2.6.27-rc5+patch_queue.orig/fs/ext4/mballoc.c 2008-09-15 10:59:27.000000000 +0200
> +++ linux-2.6.27-rc5+patch_queue/fs/ext4/mballoc.c 2008-09-15 14:02:44.000000000 +0200
> @@ -785,9 +785,11 @@ static int ext4_mb_init_cache(struct pag
> if (bh[i] == NULL)
> goto out;
>
> - if (bh_uptodate_or_lock(bh[i]))
> + if (buffer_uptodate(bh[i]) &&
> + !(desc->bg_flags & cpu_to_le16(EXT4_BG_BLOCK_UNINIT)))
> continue;
>
> + lock_buffer(bh[i]);
> spin_lock(sb_bgl_lock(EXT4_SB(sb), first_group + i));
> if (desc->bg_flags & cpu_to_le16(EXT4_BG_BLOCK_UNINIT)) {
> ext4_init_block_bitmap(sb, bh[i],
>

-aneesh