2011-07-20 13:37:26

by Yongqiang Yang

[permalink] [raw]
Subject: ext4: let ext4 get free blocks count of uninit groups from desc

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.



2011-07-20 13:37:31

by Yongqiang Yang

[permalink] [raw]
Subject: [PATCH 2/2] ext4: remove ext4_free_blocks_after_init()

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

Signed-off-by: Yongqiang Yang <[email protected]>
---
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


2011-07-20 13:37:28

by Yongqiang Yang

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

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

Signed-off-by: Yongqiang Yang <[email protected]>
---
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


2011-07-20 15:41:40

by Andreas Dilger

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

On 2011-07-20, at 7:32 AM, Yongqiang Yang <[email protected]> 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 <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2011-07-21 01:37:18

by Yongqiang Yang

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

On Wed, Jul 20, 2011 at 11:41 PM, Andreas Dilger <[email protected]> wrote:
> On 2011-07-20, at 7:32 AM, Yongqiang Yang <[email protected]> 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 <[email protected]>
>> ---
>> 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 [email protected]
>> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>



--
Best Wishes
Yongqiang Yang

2011-08-01 11:20:17

by Theodore Ts'o

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

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 <[email protected]>

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

2011-08-02 03:14:43

by Andreas Dilger

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

On 2011-08-01, at 5:20 AM, Ted Ts'o <[email protected]> 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 <[email protected]>
>
> 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