ext4: let ext4 get free blocks count of uninit groups from desc

ext4: let ext4 get free blocks count of uninit groups from desc

[Yongqiang Yang]

Hi all,

I do not know why ext4 does not get free blocks count of BLOCK_UNINIT groups
from group descriptors directly in the past.  But now, according to e2fsprogs
and code of kernel side we can get free blocks count from descriptors for
BLOCK_UNINIT groups, rather than compute it.  This can simplifies code a lot.

[PATCH 1/2] ext4: get free blocks count of BLOCK_UNINIT groups from group desc directly

[Yongqiang Yang]

This patch teaches ext4 to get free blocks count of BLOCK_UNINIT groups from
group desc directly.

Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
---
 fs/ext4/ialloc.c  |    2 --
 fs/ext4/mballoc.c |   14 ++------------
 2 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 21bb2f6..efdc605 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -954,9 +954,7 @@ got:
 		ext4_lock_group(sb, group);
 		/* recheck and clear flag under lock if we still need to */
 		if (gdp->bg_flags & cpu_to_le16(EXT4_BG_BLOCK_UNINIT)) {
-			free = ext4_free_blocks_after_init(sb, group, gdp);
 			gdp->bg_flags &= cpu_to_le16(~EXT4_BG_BLOCK_UNINIT);
-			ext4_free_blks_set(sb, gdp, free);
 			gdp->bg_checksum = ext4_group_desc_csum(sbi, group,
 								gdp);
 		}
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index b97a2d2..f92826b 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2248,13 +2248,7 @@ int ext4_mb_add_groupinfo(struct super_block *sb, ext4_group_t group,
 	 * initialize bb_free to be able to skip
 	 * empty groups without initialization
 	 */
-	if (desc->bg_flags & cpu_to_le16(EXT4_BG_BLOCK_UNINIT)) {
-		meta_group_info[i]->bb_free =
-			ext4_free_blocks_after_init(sb, group, desc);
-	} else {
-		meta_group_info[i]->bb_free =
-			ext4_free_blks_count(sb, desc);
-	}
+	meta_group_info[i]->bb_free = ext4_free_blks_count(sb, desc);
 
 	INIT_LIST_HEAD(&meta_group_info[i]->bb_prealloc_list);
 	init_rwsem(&meta_group_info[i]->alloc_sem);
@@ -2802,12 +2796,8 @@ ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac,
 	}
 #endif
 	mb_set_bits(bitmap_bh->b_data, ac->ac_b_ex.fe_start,ac->ac_b_ex.fe_len);
-	if (gdp->bg_flags & cpu_to_le16(EXT4_BG_BLOCK_UNINIT)) {
+	if (gdp->bg_flags & cpu_to_le16(EXT4_BG_BLOCK_UNINIT))
 		gdp->bg_flags &= cpu_to_le16(~EXT4_BG_BLOCK_UNINIT);
-		ext4_free_blks_set(sb, gdp,
-					ext4_free_blocks_after_init(sb,
-					ac->ac_b_ex.fe_group, gdp));
-	}
 	len = ext4_free_blks_count(sb, gdp) - ac->ac_b_ex.fe_len;
 	ext4_free_blks_set(sb, gdp, len);
 	gdp->bg_checksum = ext4_group_desc_csum(sbi, ac->ac_b_ex.fe_group, gdp);
-- 
1.7.5.1

[PATCH 2/2] ext4: remove ext4_free_blocks_after_init()

[Yongqiang Yang]

This patch removes ext4_free_blocks_after_init() and simplifies
ext4_init_block_bitmap() a lot.

Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
---
 fs/ext4/balloc.c |  122 ++++++++++++++++++-----------------------------------
 fs/ext4/ext4.h   |    4 +-
 2 files changed, 43 insertions(+), 83 deletions(-)

diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index f8224ad..2dd9820 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -55,60 +55,31 @@ static int ext4_block_in_group(struct super_block *sb, ext4_fsblk_t block,
 	return 0;
 }
 
-static int ext4_group_used_meta_blocks(struct super_block *sb,
-				       ext4_group_t block_group,
-				       struct ext4_group_desc *gdp)
-{
-	ext4_fsblk_t tmp;
-	struct ext4_sb_info *sbi = EXT4_SB(sb);
-	/* block bitmap, inode bitmap, and inode table blocks */
-	int used_blocks = sbi->s_itb_per_group + 2;
-
-	if (EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_FLEX_BG)) {
-		if (!ext4_block_in_group(sb, ext4_block_bitmap(sb, gdp),
-					block_group))
-			used_blocks--;
-
-		if (!ext4_block_in_group(sb, ext4_inode_bitmap(sb, gdp),
-					block_group))
-			used_blocks--;
-
-		tmp = ext4_inode_table(sb, gdp);
-		for (; tmp < ext4_inode_table(sb, gdp) +
-				sbi->s_itb_per_group; tmp++) {
-			if (!ext4_block_in_group(sb, tmp, block_group))
-				used_blocks -= 1;
-		}
-	}
-	return used_blocks;
-}
-
-/* Initializes an uninitialized block bitmap if given, and returns the
- * number of blocks free in the group. */
-unsigned ext4_init_block_bitmap(struct super_block *sb, struct buffer_head *bh,
+/* Initializes an uninitialized block bitmap */
+void ext4_init_block_bitmap(struct super_block *sb, struct buffer_head *bh,
 		 ext4_group_t block_group, struct ext4_group_desc *gdp)
 {
+	ext4_fsblk_t start, tmp;
 	int bit, bit_max;
 	ext4_group_t ngroups = ext4_get_groups_count(sb);
-	unsigned free_blocks, group_blocks;
+	unsigned group_blocks;
 	struct ext4_sb_info *sbi = EXT4_SB(sb);
-
-	if (bh) {
-		J_ASSERT_BH(bh, buffer_locked(bh));
-
-		/* If checksum is bad mark all blocks used to prevent allocation
-		 * essentially implementing a per-group read-only flag. */
-		if (!ext4_group_desc_csum_verify(sbi, block_group, gdp)) {
-			ext4_error(sb, "Checksum bad for group %u",
-					block_group);
-			ext4_free_blks_set(sb, gdp, 0);
-			ext4_free_inodes_set(sb, gdp, 0);
-			ext4_itable_unused_set(sb, gdp, 0);
-			memset(bh->b_data, 0xff, sb->s_blocksize);
-			return 0;
-		}
-		memset(bh->b_data, 0, sb->s_blocksize);
+	int flex_bg = 0;
+
+	J_ASSERT_BH(bh, bh && buffer_locked(bh));
+
+	/* If checksum is bad mark all blocks used to prevent allocation
+	 * essentially implementing a per-group read-only flag. */
+	if (!ext4_group_desc_csum_verify(sbi, block_group, gdp)) {
+		ext4_error(sb, "Checksum bad for group %u",
+				block_group);
+		ext4_free_blks_set(sb, gdp, 0);
+		ext4_free_inodes_set(sb, gdp, 0);
+		ext4_itable_unused_set(sb, gdp, 0);
+		memset(bh->b_data, 0xff, sb->s_blocksize);
+		return; 
 	}
+	memset(bh->b_data, 0, sb->s_blocksize);
 
 	/* Check for superblock and gdt backups in this group */
 	bit_max = ext4_bg_has_super(sb, block_group);
@@ -137,46 +108,37 @@ unsigned ext4_init_block_bitmap(struct super_block *sb, struct buffer_head *bh,
 		group_blocks = EXT4_BLOCKS_PER_GROUP(sb);
 	}
 
-	free_blocks = group_blocks - bit_max;
 
-	if (bh) {
-		ext4_fsblk_t start, tmp;
-		int flex_bg = 0;
+	for (bit = 0; bit < bit_max; bit++)
+		ext4_set_bit(bit, bh->b_data);
 
-		for (bit = 0; bit < bit_max; bit++)
-			ext4_set_bit(bit, bh->b_data);
+	start = ext4_group_first_block_no(sb, block_group);
 
-		start = ext4_group_first_block_no(sb, block_group);
+	flex_bg = EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_FLEX_BG);
 
-		if (EXT4_HAS_INCOMPAT_FEATURE(sb,
-					      EXT4_FEATURE_INCOMPAT_FLEX_BG))
-			flex_bg = 1;
+	/* Set bits for block and inode bitmaps, and inode table */
+	tmp = ext4_block_bitmap(sb, gdp);
+	if (!flex_bg || ext4_block_in_group(sb, tmp, block_group))
+		ext4_set_bit(tmp - start, bh->b_data);
 
-		/* Set bits for block and inode bitmaps, and inode table */
-		tmp = ext4_block_bitmap(sb, gdp);
-		if (!flex_bg || ext4_block_in_group(sb, tmp, block_group))
-			ext4_set_bit(tmp - start, bh->b_data);
+	tmp = ext4_inode_bitmap(sb, gdp);
+	if (!flex_bg || ext4_block_in_group(sb, tmp, block_group))
+		ext4_set_bit(tmp - start, bh->b_data);
 
-		tmp = ext4_inode_bitmap(sb, gdp);
-		if (!flex_bg || ext4_block_in_group(sb, tmp, block_group))
+	tmp = ext4_inode_table(sb, gdp);
+	for (; tmp < ext4_inode_table(sb, gdp) +
+			sbi->s_itb_per_group; tmp++) {
+		if (!flex_bg ||
+			ext4_block_in_group(sb, tmp, block_group))
 			ext4_set_bit(tmp - start, bh->b_data);
-
-		tmp = ext4_inode_table(sb, gdp);
-		for (; tmp < ext4_inode_table(sb, gdp) +
-				sbi->s_itb_per_group; tmp++) {
-			if (!flex_bg ||
-				ext4_block_in_group(sb, tmp, block_group))
-				ext4_set_bit(tmp - start, bh->b_data);
-		}
-		/*
-		 * Also if the number of blocks within the group is
-		 * less than the blocksize * 8 ( which is the size
-		 * of bitmap ), set rest of the block bitmap to 1
-		 */
-		ext4_mark_bitmap_end(group_blocks, sb->s_blocksize * 8,
-				     bh->b_data);
 	}
-	return free_blocks - ext4_group_used_meta_blocks(sb, block_group, gdp);
+	/*
+	 * Also if the number of blocks within the group is
+	 * less than the blocksize * 8 ( which is the size
+	 * of bitmap ), set rest of the block bitmap to 1
+	 */
+	ext4_mark_bitmap_end(group_blocks, sb->s_blocksize * 8,
+			     bh->b_data);
 }
 
 
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 62cee2b..eacaad2 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1741,12 +1741,10 @@ extern struct ext4_group_desc * ext4_get_group_desc(struct super_block * sb,
 extern int ext4_should_retry_alloc(struct super_block *sb, int *retries);
 struct buffer_head *ext4_read_block_bitmap(struct super_block *sb,
 				      ext4_group_t block_group);
-extern unsigned ext4_init_block_bitmap(struct super_block *sb,
+extern void ext4_init_block_bitmap(struct super_block *sb,
 				       struct buffer_head *bh,
 				       ext4_group_t group,
 				       struct ext4_group_desc *desc);
-#define ext4_free_blocks_after_init(sb, group, desc)			\
-		ext4_init_block_bitmap(sb, NULL, group, desc)
 ext4_fsblk_t ext4_inode_to_goal_block(struct inode *);
 
 /* dir.c */
-- 
1.7.5.1

Re: [PATCH 2/2] ext4: remove ext4_free_blocks_after_init()

[Andreas Dilger]

On 2011-07-20, at 7:32 AM, Yongqiang Yang <xiaoqiangnk@gmail.com> wrote:
> This patch removes ext4_free_blocks_after_init() and simplifies
> ext4_init_block_bitmap() a lot.

Why not use the ext4_set_bits() function, exported in another patch, to set multiple bits?

> Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
> ---
> fs/ext4/balloc.c |  122 ++++++++++++++++++-----------------------------------
> fs/ext4/ext4.h   |    4 +-
> 2 files changed, 43 insertions(+), 83 deletions(-)
> 
> diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
> index f8224ad..2dd9820 100644
> --- a/fs/ext4/balloc.c
> +++ b/fs/ext4/balloc.c
> @@ -55,60 +55,31 @@ static int ext4_block_in_group(struct super_block *sb, ext4_fsblk_t block,
>    return 0;
> }
> 
> -static int ext4_group_used_meta_blocks(struct super_block *sb,
> -                       ext4_group_t block_group,
> -                       struct ext4_group_desc *gdp)
> -{
> -    ext4_fsblk_t tmp;
> -    struct ext4_sb_info *sbi = EXT4_SB(sb);
> -    /* block bitmap, inode bitmap, and inode table blocks */
> -    int used_blocks = sbi->s_itb_per_group + 2;
> -
> -    if (EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_FLEX_BG)) {
> -        if (!ext4_block_in_group(sb, ext4_block_bitmap(sb, gdp),
> -                    block_group))
> -            used_blocks--;
> -
> -        if (!ext4_block_in_group(sb, ext4_inode_bitmap(sb, gdp),
> -                    block_group))
> -            used_blocks--;
> -
> -        tmp = ext4_inode_table(sb, gdp);
> -        for (; tmp < ext4_inode_table(sb, gdp) +
> -                sbi->s_itb_per_group; tmp++) {
> -            if (!ext4_block_in_group(sb, tmp, block_group))
> -                used_blocks -= 1;
> -        }
> -    }
> -    return used_blocks;
> -}
> -
> -/* Initializes an uninitialized block bitmap if given, and returns the
> - * number of blocks free in the group. */
> -unsigned ext4_init_block_bitmap(struct super_block *sb, struct buffer_head *bh,
> +/* Initializes an uninitialized block bitmap */
> +void ext4_init_block_bitmap(struct super_block *sb, struct buffer_head *bh,
>         ext4_group_t block_group, struct ext4_group_desc *gdp)
> {
> +    ext4_fsblk_t start, tmp;
>    int bit, bit_max;
>    ext4_group_t ngroups = ext4_get_groups_count(sb);
> -    unsigned free_blocks, group_blocks;
> +    unsigned group_blocks;
>    struct ext4_sb_info *sbi = EXT4_SB(sb);
> -
> -    if (bh) {
> -        J_ASSERT_BH(bh, buffer_locked(bh));
> -
> -        /* If checksum is bad mark all blocks used to prevent allocation
> -         * essentially implementing a per-group read-only flag. */
> -        if (!ext4_group_desc_csum_verify(sbi, block_group, gdp)) {
> -            ext4_error(sb, "Checksum bad for group %u",
> -                    block_group);
> -            ext4_free_blks_set(sb, gdp, 0);
> -            ext4_free_inodes_set(sb, gdp, 0);
> -            ext4_itable_unused_set(sb, gdp, 0);
> -            memset(bh->b_data, 0xff, sb->s_blocksize);
> -            return 0;
> -        }
> -        memset(bh->b_data, 0, sb->s_blocksize);
> +    int flex_bg = 0;
> +
> +    J_ASSERT_BH(bh, bh && buffer_locked(bh));
> +
> +    /* If checksum is bad mark all blocks used to prevent allocation
> +     * essentially implementing a per-group read-only flag. */
> +    if (!ext4_group_desc_csum_verify(sbi, block_group, gdp)) {
> +        ext4_error(sb, "Checksum bad for group %u",
> +                block_group);
> +        ext4_free_blks_set(sb, gdp, 0);
> +        ext4_free_inodes_set(sb, gdp, 0);
> +        ext4_itable_unused_set(sb, gdp, 0);
> +        memset(bh->b_data, 0xff, sb->s_blocksize);
> +        return; 
>    }
> +    memset(bh->b_data, 0, sb->s_blocksize);
> 
>    /* Check for superblock and gdt backups in this group */
>    bit_max = ext4_bg_has_super(sb, block_group);
> @@ -137,46 +108,37 @@ unsigned ext4_init_block_bitmap(struct super_block *sb, struct buffer_head *bh,
>        group_blocks = EXT4_BLOCKS_PER_GROUP(sb);
>    }
> 
> -    free_blocks = group_blocks - bit_max;
> 
> -    if (bh) {
> -        ext4_fsblk_t start, tmp;
> -        int flex_bg = 0;
> +    for (bit = 0; bit < bit_max; bit++)
> +        ext4_set_bit(bit, bh->b_data);
> 
> -        for (bit = 0; bit < bit_max; bit++)
> -            ext4_set_bit(bit, bh->b_data);
> +    start = ext4_group_first_block_no(sb, block_group);
> 
> -        start = ext4_group_first_block_no(sb, block_group);
> +    flex_bg = EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_FLEX_BG);
> 
> -        if (EXT4_HAS_INCOMPAT_FEATURE(sb,
> -                          EXT4_FEATURE_INCOMPAT_FLEX_BG))
> -            flex_bg = 1;
> +    /* Set bits for block and inode bitmaps, and inode table */
> +    tmp = ext4_block_bitmap(sb, gdp);
> +    if (!flex_bg || ext4_block_in_group(sb, tmp, block_group))
> +        ext4_set_bit(tmp - start, bh->b_data);
> 
> -        /* Set bits for block and inode bitmaps, and inode table */
> -        tmp = ext4_block_bitmap(sb, gdp);
> -        if (!flex_bg || ext4_block_in_group(sb, tmp, block_group))
> -            ext4_set_bit(tmp - start, bh->b_data);
> +    tmp = ext4_inode_bitmap(sb, gdp);
> +    if (!flex_bg || ext4_block_in_group(sb, tmp, block_group))
> +        ext4_set_bit(tmp - start, bh->b_data);
> 
> -        tmp = ext4_inode_bitmap(sb, gdp);
> -        if (!flex_bg || ext4_block_in_group(sb, tmp, block_group))
> +    tmp = ext4_inode_table(sb, gdp);
> +    for (; tmp < ext4_inode_table(sb, gdp) +
> +            sbi->s_itb_per_group; tmp++) {
> +        if (!flex_bg ||
> +            ext4_block_in_group(sb, tmp, block_group))
>            ext4_set_bit(tmp - start, bh->b_data);
> -
> -        tmp = ext4_inode_table(sb, gdp);
> -        for (; tmp < ext4_inode_table(sb, gdp) +
> -                sbi->s_itb_per_group; tmp++) {
> -            if (!flex_bg ||
> -                ext4_block_in_group(sb, tmp, block_group))
> -                ext4_set_bit(tmp - start, bh->b_data);
> -        }
> -        /*
> -         * Also if the number of blocks within the group is
> -         * less than the blocksize * 8 ( which is the size
> -         * of bitmap ), set rest of the block bitmap to 1
> -         */
> -        ext4_mark_bitmap_end(group_blocks, sb->s_blocksize * 8,
> -                     bh->b_data);
>    }
> -    return free_blocks - ext4_group_used_meta_blocks(sb, block_group, gdp);
> +    /*
> +     * Also if the number of blocks within the group is
> +     * less than the blocksize * 8 ( which is the size
> +     * of bitmap ), set rest of the block bitmap to 1
> +     */
> +    ext4_mark_bitmap_end(group_blocks, sb->s_blocksize * 8,
> +                 bh->b_data);
> }
> 
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 62cee2b..eacaad2 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1741,12 +1741,10 @@ extern struct ext4_group_desc * ext4_get_group_desc(struct super_block * sb,
> extern int ext4_should_retry_alloc(struct super_block *sb, int *retries);
> struct buffer_head *ext4_read_block_bitmap(struct super_block *sb,
>                      ext4_group_t block_group);
> -extern unsigned ext4_init_block_bitmap(struct super_block *sb,
> +extern void ext4_init_block_bitmap(struct super_block *sb,
>                       struct buffer_head *bh,
>                       ext4_group_t group,
>                       struct ext4_group_desc *desc);
> -#define ext4_free_blocks_after_init(sb, group, desc)            \
> -        ext4_init_block_bitmap(sb, NULL, group, desc)
> ext4_fsblk_t ext4_inode_to_goal_block(struct inode *);
> 
> /* dir.c */
> -- 
> 1.7.5.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] ext4: remove ext4_free_blocks_after_init()

[Yongqiang Yang]

On Wed, Jul 20, 2011 at 11:41 PM, Andreas Dilger <aedilger@gmail.com> wrote:
> On 2011-07-20, at 7:32 AM, Yongqiang Yang <xiaoqiangnk@gmail.com> wrote:
>> This patch removes ext4_free_blocks_after_init() and simplifies
>> ext4_init_block_bitmap() a lot.
>
> Why not use the ext4_set_bits() function, exported in another patch, to set multiple bits?
That patch has not been merged.  After it is merged, I will post
another patch doing this, this can easy Ted's work.  That patch is a
part of resize, but this patch has nothing to do with it logically.


Thank you,

Yongqiang.
>
>> Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
>> ---
>> fs/ext4/balloc.c |  122 ++++++++++++++++++-----------------------------------
>> fs/ext4/ext4.h   |    4 +-
>> 2 files changed, 43 insertions(+), 83 deletions(-)
>>
>> diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
>> index f8224ad..2dd9820 100644
>> --- a/fs/ext4/balloc.c
>> +++ b/fs/ext4/balloc.c
>> @@ -55,60 +55,31 @@ static int ext4_block_in_group(struct super_block *sb, ext4_fsblk_t block,
>>    return 0;
>> }
>>
>> -static int ext4_group_used_meta_blocks(struct super_block *sb,
>> -                       ext4_group_t block_group,
>> -                       struct ext4_group_desc *gdp)
>> -{
>> -    ext4_fsblk_t tmp;
>> -    struct ext4_sb_info *sbi = EXT4_SB(sb);
>> -    /* block bitmap, inode bitmap, and inode table blocks */
>> -    int used_blocks = sbi->s_itb_per_group + 2;
>> -
>> -    if (EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_FLEX_BG)) {
>> -        if (!ext4_block_in_group(sb, ext4_block_bitmap(sb, gdp),
>> -                    block_group))
>> -            used_blocks--;
>> -
>> -        if (!ext4_block_in_group(sb, ext4_inode_bitmap(sb, gdp),
>> -                    block_group))
>> -            used_blocks--;
>> -
>> -        tmp = ext4_inode_table(sb, gdp);
>> -        for (; tmp < ext4_inode_table(sb, gdp) +
>> -                sbi->s_itb_per_group; tmp++) {
>> -            if (!ext4_block_in_group(sb, tmp, block_group))
>> -                used_blocks -= 1;
>> -        }
>> -    }
>> -    return used_blocks;
>> -}
>> -
>> -/* Initializes an uninitialized block bitmap if given, and returns the
>> - * number of blocks free in the group. */
>> -unsigned ext4_init_block_bitmap(struct super_block *sb, struct buffer_head *bh,
>> +/* Initializes an uninitialized block bitmap */
>> +void ext4_init_block_bitmap(struct super_block *sb, struct buffer_head *bh,
>>         ext4_group_t block_group, struct ext4_group_desc *gdp)
>> {
>> +    ext4_fsblk_t start, tmp;
>>    int bit, bit_max;
>>    ext4_group_t ngroups = ext4_get_groups_count(sb);
>> -    unsigned free_blocks, group_blocks;
>> +    unsigned group_blocks;
>>    struct ext4_sb_info *sbi = EXT4_SB(sb);
>> -
>> -    if (bh) {
>> -        J_ASSERT_BH(bh, buffer_locked(bh));
>> -
>> -        /* If checksum is bad mark all blocks used to prevent allocation
>> -         * essentially implementing a per-group read-only flag. */
>> -        if (!ext4_group_desc_csum_verify(sbi, block_group, gdp)) {
>> -            ext4_error(sb, "Checksum bad for group %u",
>> -                    block_group);
>> -            ext4_free_blks_set(sb, gdp, 0);
>> -            ext4_free_inodes_set(sb, gdp, 0);
>> -            ext4_itable_unused_set(sb, gdp, 0);
>> -            memset(bh->b_data, 0xff, sb->s_blocksize);
>> -            return 0;
>> -        }
>> -        memset(bh->b_data, 0, sb->s_blocksize);
>> +    int flex_bg = 0;
>> +
>> +    J_ASSERT_BH(bh, bh && buffer_locked(bh));
>> +
>> +    /* If checksum is bad mark all blocks used to prevent allocation
>> +     * essentially implementing a per-group read-only flag. */
>> +    if (!ext4_group_desc_csum_verify(sbi, block_group, gdp)) {
>> +        ext4_error(sb, "Checksum bad for group %u",
>> +                block_group);
>> +        ext4_free_blks_set(sb, gdp, 0);
>> +        ext4_free_inodes_set(sb, gdp, 0);
>> +        ext4_itable_unused_set(sb, gdp, 0);
>> +        memset(bh->b_data, 0xff, sb->s_blocksize);
>> +        return;
>>    }
>> +    memset(bh->b_data, 0, sb->s_blocksize);
>>
>>    /* Check for superblock and gdt backups in this group */
>>    bit_max = ext4_bg_has_super(sb, block_group);
>> @@ -137,46 +108,37 @@ unsigned ext4_init_block_bitmap(struct super_block *sb, struct buffer_head *bh,
>>        group_blocks = EXT4_BLOCKS_PER_GROUP(sb);
>>    }
>>
>> -    free_blocks = group_blocks - bit_max;
>>
>> -    if (bh) {
>> -        ext4_fsblk_t start, tmp;
>> -        int flex_bg = 0;
>> +    for (bit = 0; bit < bit_max; bit++)
>> +        ext4_set_bit(bit, bh->b_data);
>>
>> -        for (bit = 0; bit < bit_max; bit++)
>> -            ext4_set_bit(bit, bh->b_data);
>> +    start = ext4_group_first_block_no(sb, block_group);
>>
>> -        start = ext4_group_first_block_no(sb, block_group);
>> +    flex_bg = EXT4_HAS_INCOMPAT_FEATURE(sb, EXT4_FEATURE_INCOMPAT_FLEX_BG);
>>
>> -        if (EXT4_HAS_INCOMPAT_FEATURE(sb,
>> -                          EXT4_FEATURE_INCOMPAT_FLEX_BG))
>> -            flex_bg = 1;
>> +    /* Set bits for block and inode bitmaps, and inode table */
>> +    tmp = ext4_block_bitmap(sb, gdp);
>> +    if (!flex_bg || ext4_block_in_group(sb, tmp, block_group))
>> +        ext4_set_bit(tmp - start, bh->b_data);
>>
>> -        /* Set bits for block and inode bitmaps, and inode table */
>> -        tmp = ext4_block_bitmap(sb, gdp);
>> -        if (!flex_bg || ext4_block_in_group(sb, tmp, block_group))
>> -            ext4_set_bit(tmp - start, bh->b_data);
>> +    tmp = ext4_inode_bitmap(sb, gdp);
>> +    if (!flex_bg || ext4_block_in_group(sb, tmp, block_group))
>> +        ext4_set_bit(tmp - start, bh->b_data);
>>
>> -        tmp = ext4_inode_bitmap(sb, gdp);
>> -        if (!flex_bg || ext4_block_in_group(sb, tmp, block_group))
>> +    tmp = ext4_inode_table(sb, gdp);
>> +    for (; tmp < ext4_inode_table(sb, gdp) +
>> +            sbi->s_itb_per_group; tmp++) {
>> +        if (!flex_bg ||
>> +            ext4_block_in_group(sb, tmp, block_group))
>>            ext4_set_bit(tmp - start, bh->b_data);
>> -
>> -        tmp = ext4_inode_table(sb, gdp);
>> -        for (; tmp < ext4_inode_table(sb, gdp) +
>> -                sbi->s_itb_per_group; tmp++) {
>> -            if (!flex_bg ||
>> -                ext4_block_in_group(sb, tmp, block_group))
>> -                ext4_set_bit(tmp - start, bh->b_data);
>> -        }
>> -        /*
>> -         * Also if the number of blocks within the group is
>> -         * less than the blocksize * 8 ( which is the size
>> -         * of bitmap ), set rest of the block bitmap to 1
>> -         */
>> -        ext4_mark_bitmap_end(group_blocks, sb->s_blocksize * 8,
>> -                     bh->b_data);
>>    }
>> -    return free_blocks - ext4_group_used_meta_blocks(sb, block_group, gdp);
>> +    /*
>> +     * Also if the number of blocks within the group is
>> +     * less than the blocksize * 8 ( which is the size
>> +     * of bitmap ), set rest of the block bitmap to 1
>> +     */
>> +    ext4_mark_bitmap_end(group_blocks, sb->s_blocksize * 8,
>> +                 bh->b_data);
>> }
>>
>>
>> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
>> index 62cee2b..eacaad2 100644
>> --- a/fs/ext4/ext4.h
>> +++ b/fs/ext4/ext4.h
>> @@ -1741,12 +1741,10 @@ extern struct ext4_group_desc * ext4_get_group_desc(struct super_block * sb,
>> extern int ext4_should_retry_alloc(struct super_block *sb, int *retries);
>> struct buffer_head *ext4_read_block_bitmap(struct super_block *sb,
>>                      ext4_group_t block_group);
>> -extern unsigned ext4_init_block_bitmap(struct super_block *sb,
>> +extern void ext4_init_block_bitmap(struct super_block *sb,
>>                       struct buffer_head *bh,
>>                       ext4_group_t group,
>>                       struct ext4_group_desc *desc);
>> -#define ext4_free_blocks_after_init(sb, group, desc)            \
>> -        ext4_init_block_bitmap(sb, NULL, group, desc)
>> ext4_fsblk_t ext4_inode_to_goal_block(struct inode *);
>>
>> /* dir.c */
>> --
>> 1.7.5.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Best Wishes
Yongqiang Yang
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] ext4: get free blocks count of BLOCK_UNINIT groups from group desc directly

[Ted Ts'o]

On Wed, Jul 20, 2011 at 09:32:53PM +0800, Yongqiang Yang wrote:
> This patch teaches ext4 to get free blocks count of BLOCK_UNINIT groups from
> group desc directly.
> 
> Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>

The reason why we weren't doing this is because older versions of
e2fsprogs I don't think set the free blocks count correctly.  Or at
least, I have a vague memory that this was the case.  I'll have to do
some code archeology before can be 100% sure this is safe to do.

Regards,                      	              	

					- Ted

Re: [PATCH 1/2] ext4: get free blocks count of BLOCK_UNINIT groups from group desc directly

[Andreas Dilger]

On 2011-08-01, at 5:20 AM, Ted Ts'o <tytso@mit.edu> wrote:
> On Wed, Jul 20, 2011 at 09:32:53PM +0800, Yongqiang Yang wrote:
>> This patch teaches ext4 to get free blocks count of BLOCK_UNINIT groups from
>> group desc directly.
>> 
>> Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com>
> 
> The reason why we weren't doing this is because older versions of
> e2fsprogs I don't think set the free blocks count correctly.  Or at
> least, I have a vague memory that this was the case.  I'll have to do
> some code archeology before can be 100% sure this is safe to do.

I think the other reason is that this gives the kernel code some flexibility in how the group is formatted (e.g. flex_bg or whatever may move allocated blocks around).

Cheers, Andreas