RE: [PATCH 2/4] ext4: new ext4_mark_group_bitmap_corrupted() helper

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Ted,

 Any updates for these reviewed patches? it is cleanup and fixes which should be
fine to merge unless you have some objections ^_^.

  I will update last one that Andreas said need improvement though, but prepare and reviewed
patch should be fine?


Thanks,
Shilong

________________________________________
From: Andreas Dilger [adilger@xxxxxxxxx]
Sent: Wednesday, April 18, 2018 13:12
To: Wang Shilong
Cc: linux-ext4@xxxxxxxxxxxxxxx; bo.liu@xxxxxxxxxxxxxxxxx; Wang Shilong; Shuichi Ihara; tytso@xxxxxxx
Subject: Re: [PATCH 2/4] ext4: new ext4_mark_group_bitmap_corrupted() helper

On Apr 17, 2018, at 6:08 PM, Wang Shilong <wangshilong1991@xxxxxxxxx> wrote:
>
> From: Wang Shilong <wshilong@xxxxxxx>
>
> Since there are many places to set inode/block bitmap
> corrupt bit, add a new helper for it, which will make
> codes more clear.
>
> Signed-off-by: Wang Shilong <wshilong@xxxxxxx>

Reviewed-by: Andreas Dilger <adilger@xxxxxxxxx>

> ---
> fs/ext4/balloc.c  | 29 +++++++----------------------
> fs/ext4/ext4.h    |  7 +++++++
> fs/ext4/ialloc.c  | 20 ++++----------------
> fs/ext4/mballoc.c | 14 ++++----------
> fs/ext4/super.c   | 30 ++++++++++++++++++++++++++++++
> 5 files changed, 52 insertions(+), 48 deletions(-)
>
> diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
> index a33d8fb1bf2a..0ac442aff7b8 100644
> --- a/fs/ext4/balloc.c
> +++ b/fs/ext4/balloc.c
> @@ -185,25 +185,15 @@ static int ext4_init_block_bitmap(struct super_block *sb,
>       struct ext4_sb_info *sbi = EXT4_SB(sb);
>       ext4_fsblk_t start, tmp;
>       int flex_bg = 0;
> -     struct ext4_group_info *grp;
>
>       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(sb, block_group, gdp)) {
> -             grp = ext4_get_group_info(sb, block_group);
> -             if (!EXT4_MB_GRP_BBITMAP_CORRUPT(grp))
> -                     percpu_counter_sub(&sbi->s_freeclusters_counter,
> -                                        grp->bb_free);
> -             set_bit(EXT4_GROUP_INFO_BBITMAP_CORRUPT_BIT, &grp->bb_state);
> -             if (!EXT4_MB_GRP_IBITMAP_CORRUPT(grp)) {
> -                     int count;
> -                     count = ext4_free_inodes_count(sb, gdp);
> -                     percpu_counter_sub(&sbi->s_freeinodes_counter,
> -                                        count);
> -             }
> -             set_bit(EXT4_GROUP_INFO_IBITMAP_CORRUPT_BIT, &grp->bb_state);
> +             ext4_mark_group_bitmap_corrupted(sb, block_group,
> +                                     EXT4_GROUP_INFO_BBITMAP_CORRUPT |
> +                                     EXT4_GROUP_INFO_IBITMAP_CORRUPT);
>               return -EFSBADCRC;
>       }
>       memset(bh->b_data, 0, sb->s_blocksize);
> @@ -374,7 +364,6 @@ static int ext4_validate_block_bitmap(struct super_block *sb,
> {
>       ext4_fsblk_t    blk;
>       struct ext4_group_info *grp = ext4_get_group_info(sb, block_group);
> -     struct ext4_sb_info *sbi = EXT4_SB(sb);
>
>       if (buffer_verified(bh))
>               return 0;
> @@ -386,10 +375,8 @@ static int ext4_validate_block_bitmap(struct super_block *sb,
>                       desc, bh))) {
>               ext4_unlock_group(sb, block_group);
>               ext4_error(sb, "bg %u: bad block bitmap checksum", block_group);
> -             if (!EXT4_MB_GRP_BBITMAP_CORRUPT(grp))
> -                     percpu_counter_sub(&sbi->s_freeclusters_counter,
> -                                        grp->bb_free);
> -             set_bit(EXT4_GROUP_INFO_BBITMAP_CORRUPT_BIT, &grp->bb_state);
> +             ext4_mark_group_bitmap_corrupted(sb, block_group,
> +                                     EXT4_GROUP_INFO_BBITMAP_CORRUPT);
>               return -EFSBADCRC;
>       }
>       blk = ext4_valid_block_bitmap(sb, desc, block_group, bh);
> @@ -397,10 +384,8 @@ static int ext4_validate_block_bitmap(struct super_block *sb,
>               ext4_unlock_group(sb, block_group);
>               ext4_error(sb, "bg %u: block %llu: invalid block bitmap",
>                          block_group, blk);
> -             if (!EXT4_MB_GRP_BBITMAP_CORRUPT(grp))
> -                     percpu_counter_sub(&sbi->s_freeclusters_counter,
> -                                        grp->bb_free);
> -             set_bit(EXT4_GROUP_INFO_BBITMAP_CORRUPT_BIT, &grp->bb_state);
> +             ext4_mark_group_bitmap_corrupted(sb, block_group,
> +                                     EXT4_GROUP_INFO_BBITMAP_CORRUPT);
>               return -EFSCORRUPTED;
>       }
>       set_buffer_verified(bh);
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index a42e71203e53..fa52b7dd4542 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2530,6 +2530,9 @@ extern int ext4_alloc_flex_bg_array(struct super_block *sb,
>                                   ext4_group_t ngroup);
> extern const char *ext4_decode_error(struct super_block *sb, int errno,
>                                    char nbuf[16]);
> +extern void ext4_mark_group_bitmap_corrupted(struct super_block *sb,
> +                                          ext4_group_t block_group,
> +                                          unsigned int flags);
>
> extern __printf(4, 5)
> void __ext4_error(struct super_block *, const char *, unsigned int,
> @@ -2857,6 +2860,10 @@ struct ext4_group_info {
> #define EXT4_GROUP_INFO_WAS_TRIMMED_BIT               1
> #define EXT4_GROUP_INFO_BBITMAP_CORRUPT_BIT   2
> #define EXT4_GROUP_INFO_IBITMAP_CORRUPT_BIT   3
> +#define EXT4_GROUP_INFO_BBITMAP_CORRUPT              \
> +     (1 << EXT4_GROUP_INFO_BBITMAP_CORRUPT_BIT)
> +#define EXT4_GROUP_INFO_IBITMAP_CORRUPT              \
> +     (1 << EXT4_GROUP_INFO_IBITMAP_CORRUPT_BIT)
>
> #define EXT4_MB_GRP_NEED_INIT(grp)    \
>       (test_bit(EXT4_GROUP_INFO_NEED_INIT_BIT, &((grp)->bb_state)))
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index 33a2c98ce1ff..95611cf9f552 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -83,7 +83,6 @@ static int ext4_validate_inode_bitmap(struct super_block *sb,
> {
>       ext4_fsblk_t    blk;
>       struct ext4_group_info *grp = ext4_get_group_info(sb, block_group);
> -     struct ext4_sb_info *sbi = EXT4_SB(sb);
>
>       if (buffer_verified(bh))
>               return 0;
> @@ -97,14 +96,8 @@ static int ext4_validate_inode_bitmap(struct super_block *sb,
>               ext4_unlock_group(sb, block_group);
>               ext4_error(sb, "Corrupt inode bitmap - block_group = %u, "
>                          "inode_bitmap = %llu", block_group, blk);
> -             grp = ext4_get_group_info(sb, block_group);
> -             if (!EXT4_MB_GRP_IBITMAP_CORRUPT(grp)) {
> -                     int count;
> -                     count = ext4_free_inodes_count(sb, desc);
> -                     percpu_counter_sub(&sbi->s_freeinodes_counter,
> -                                        count);
> -             }
> -             set_bit(EXT4_GROUP_INFO_IBITMAP_CORRUPT_BIT, &grp->bb_state);
> +             ext4_mark_group_bitmap_corrupted(sb, block_group,
> +                                     EXT4_GROUP_INFO_IBITMAP_CORRUPT);
>               return -EFSBADCRC;
>       }
>       set_buffer_verified(bh);
> @@ -337,13 +330,8 @@ void ext4_free_inode(handle_t *handle, struct inode *inode)
>                       fatal = err;
>       } else {
>               ext4_error(sb, "bit already cleared for inode %lu", ino);
> -             if (gdp && !EXT4_MB_GRP_IBITMAP_CORRUPT(grp)) {
> -                     int count;
> -                     count = ext4_free_inodes_count(sb, gdp);
> -                     percpu_counter_sub(&sbi->s_freeinodes_counter,
> -                                        count);
> -             }
> -             set_bit(EXT4_GROUP_INFO_IBITMAP_CORRUPT_BIT, &grp->bb_state);
> +             ext4_mark_group_bitmap_corrupted(sb, block_group,
> +                                     EXT4_GROUP_INFO_IBITMAP_CORRUPT);
>       }
>
> error_return:
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 769a62708b1c..bc2d1eb9fd5d 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -747,10 +747,8 @@ void ext4_mb_generate_buddy(struct super_block *sb,
>                * corrupt and update bb_free using bitmap value
>                */
>               grp->bb_free = free;
> -             if (!EXT4_MB_GRP_BBITMAP_CORRUPT(grp))
> -                     percpu_counter_sub(&sbi->s_freeclusters_counter,
> -                                        grp->bb_free);
> -             set_bit(EXT4_GROUP_INFO_BBITMAP_CORRUPT_BIT, &grp->bb_state);
> +             ext4_mark_group_bitmap_corrupted(sb, group,
> +                                     EXT4_GROUP_INFO_BBITMAP_CORRUPT);
>       }
>       mb_set_largest_free_order(sb, grp);
>
> @@ -1454,12 +1452,8 @@ static void mb_free_blocks(struct inode *inode, struct ext4_buddy *e4b,
>                                     "freeing already freed block "
>                                     "(bit %u); block bitmap corrupt.",
>                                     block);
> -             if (!EXT4_MB_GRP_BBITMAP_CORRUPT(e4b->bd_info))
> -                     percpu_counter_sub(&sbi->s_freeclusters_counter,
> -                                        e4b->bd_info->bb_free);
> -             /* Mark the block group as corrupt. */
> -             set_bit(EXT4_GROUP_INFO_BBITMAP_CORRUPT_BIT,
> -                     &e4b->bd_info->bb_state);
> +             ext4_mark_group_bitmap_corrupted(sb, e4b->bd_group,
> +                             EXT4_GROUP_INFO_BBITMAP_CORRUPT);
>               mb_regenerate_buddy(e4b);
>               goto done;
>       }
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 185f7e61f4cf..f5e9a1af067a 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -763,6 +763,36 @@ __acquires(bitlock)
>       return;
> }
>
> +void ext4_mark_group_bitmap_corrupted(struct super_block *sb,
> +                                  ext4_group_t group,
> +                                  unsigned int flags)
> +{
> +     struct ext4_sb_info *sbi = EXT4_SB(sb);
> +     struct ext4_group_info *grp = ext4_get_group_info(sb, group);
> +     struct ext4_group_desc *gdp = ext4_get_group_desc(sb, group, NULL);
> +
> +     if (flags & EXT4_GROUP_INFO_BBITMAP_CORRUPT &&
> +         !EXT4_MB_GRP_BBITMAP_CORRUPT(grp)) {
> +             percpu_counter_sub(&sbi->s_freeclusters_counter,
> +                                     grp->bb_free);
> +             set_bit(EXT4_GROUP_INFO_BBITMAP_CORRUPT_BIT,
> +                     &grp->bb_state);
> +     }
> +
> +     if (flags & EXT4_GROUP_INFO_IBITMAP_CORRUPT &&
> +         !EXT4_MB_GRP_IBITMAP_CORRUPT(grp)) {
> +             if (gdp) {
> +                     int count;
> +
> +                     count = ext4_free_inodes_count(sb, gdp);
> +                     percpu_counter_sub(&sbi->s_freeinodes_counter,
> +                                        count);
> +             }
> +             set_bit(EXT4_GROUP_INFO_IBITMAP_CORRUPT_BIT,
> +                     &grp->bb_state);
> +     }
> +}
> +
> void ext4_update_dynamic_rev(struct super_block *sb)
> {
>       struct ext4_super_block *es = EXT4_SB(sb)->s_es;
> --
> 2.14.3
>


Cheers, Andreas







[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux