[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <13F2FE76-C681-4E45-803A-7E5B46BF196A@dilger.ca>
Date: Thu, 19 Apr 2018 14:42:36 -0600
From: Andreas Dilger <adilger@...ger.ca>
To: Wang Shilong <wangshilong1991@...il.com>
Cc: Ext4 Developers List <linux-ext4@...r.kernel.org>,
Wang Shilong <wshilong@....com>,
Shuichi Ihara <sihara@....com>, tytso@....edu
Subject: Re: [PATCH 1/2] ext4: wrap error handling and corrupted bitmaps
setting
On Apr 19, 2018, at 7:39 AM, Wang Shilong <wangshilong1991@...il.com> wrote:
>
> From: Wang Shilong <wshilong@....com>
>
> Cleanup to handle ext4 error codes together with
> bitmaps corrupted bits setting.
>
> Signed-off-by: Wang Shilong <wshilong@....com>
Reviewed-by: Andreas Dilger <adilger@...ger.ca>
> ---
> fs/ext4/balloc.c | 30 +++++++++++++++++-------------
> fs/ext4/ext4.h | 35 ++++++++++++++++++++++++-----------
> fs/ext4/ialloc.c | 37 +++++++++++++++++++------------------
> fs/ext4/mballoc.c | 20 ++++++++------------
> fs/ext4/super.c | 11 +++++++----
> 5 files changed, 75 insertions(+), 58 deletions(-)
>
> diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
> index 8ef65251d019..76ae05993390 100644
> --- a/fs/ext4/balloc.c
> +++ b/fs/ext4/balloc.c
> @@ -193,7 +193,9 @@ static int ext4_init_block_bitmap(struct super_block *sb,
> if (!ext4_group_desc_csum_verify(sb, block_group, gdp)) {
> ext4_mark_group_bitmap_corrupted(sb, block_group,
> EXT4_GROUP_INFO_BBITMAP_CORRUPT |
> - EXT4_GROUP_INFO_IBITMAP_CORRUPT);
> + EXT4_GROUP_INFO_IBITMAP_CORRUPT,
> + "bg %u: bad block bitmap checksum",
> + block_group);
> return -EFSBADCRC;
> }
> memset(bh->b_data, 0, sb->s_blocksize);
> @@ -374,18 +376,19 @@ static int ext4_validate_block_bitmap(struct super_block *sb,
> if (unlikely(!ext4_block_bitmap_csum_verify(sb, block_group,
> desc, bh))) {
> ext4_unlock_group(sb, block_group);
> - ext4_error(sb, "bg %u: bad block bitmap checksum", block_group);
> ext4_mark_group_bitmap_corrupted(sb, block_group,
> - EXT4_GROUP_INFO_BBITMAP_CORRUPT);
> + EXT4_GROUP_INFO_BBITMAP_CORRUPT,
> + "bg %u: bad block bitmap checksum",
> + block_group);
> return -EFSBADCRC;
> }
> blk = ext4_valid_block_bitmap(sb, desc, block_group, bh);
> if (unlikely(blk != 0)) {
> ext4_unlock_group(sb, block_group);
> - ext4_error(sb, "bg %u: block %llu: invalid block bitmap",
> - block_group, blk);
> ext4_mark_group_bitmap_corrupted(sb, block_group,
> - EXT4_GROUP_INFO_BBITMAP_CORRUPT);
> + EXT4_GROUP_INFO_BBITMAP_CORRUPT,
> + "bg %u: block %llu: invalid block bitmap",
> + block_group, blk);
> return -EFSCORRUPTED;
> }
> set_buffer_verified(bh);
> @@ -418,10 +421,10 @@ ext4_read_block_bitmap_nowait(struct super_block *sb, ext4_group_t block_group)
> bitmap_blk = ext4_block_bitmap(sb, desc);
> if ((bitmap_blk <= le32_to_cpu(sbi->s_es->s_first_data_block)) ||
> (bitmap_blk >= ext4_blocks_count(sbi->s_es))) {
> - ext4_error(sb, "Invalid block bitmap block %llu in "
> - "block_group %u", bitmap_blk, block_group);
> ext4_mark_group_bitmap_corrupted(sb, block_group,
> - EXT4_GROUP_INFO_BBITMAP_CORRUPT);
> + EXT4_GROUP_INFO_BBITMAP_CORRUPT,
> + "Invalid block bitmap block %llu in "
> + "block_group %u", bitmap_blk, block_group);
> return ERR_PTR(-EFSCORRUPTED);
> }
> bh = sb_getblk(sb, bitmap_blk);
> @@ -497,11 +500,12 @@ int ext4_wait_block_bitmap(struct super_block *sb, ext4_group_t block_group,
> return -EFSCORRUPTED;
> wait_on_buffer(bh);
> if (!buffer_uptodate(bh)) {
> - ext4_error(sb, "Cannot read block bitmap - "
> - "block_group = %u, block_bitmap = %llu",
> - block_group, (unsigned long long) bh->b_blocknr);
> ext4_mark_group_bitmap_corrupted(sb, block_group,
> - EXT4_GROUP_INFO_BBITMAP_CORRUPT);
> + EXT4_GROUP_INFO_BBITMAP_CORRUPT,
> + "Cannot read block bitmap - "
> + "block_group = %u, block_bitmap = %llu",
> + block_group,
> + (unsigned long long)bh->b_blocknr);
> return -EIO;
> }
> clear_buffer_new(bh);
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index fa52b7dd4542..ed5b988d3f8e 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2530,9 +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 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,
> @@ -2558,10 +2558,10 @@ extern __printf(3, 4)
> void __ext4_msg(struct super_block *, const char *, const char *, ...);
> extern void __dump_mmp_msg(struct super_block *, struct mmp_struct *mmp,
> const char *, unsigned int, const char *);
> -extern __printf(7, 8)
> +extern __printf(8, 9)
> void __ext4_grp_locked_error(const char *, unsigned int,
> struct super_block *, ext4_group_t,
> - unsigned long, ext4_fsblk_t,
> + unsigned long, ext4_fsblk_t, unsigned int,
> const char *, ...);
>
> #define EXT4_ERROR_INODE(inode, fmt, a...) \
> @@ -2591,9 +2591,16 @@ void __ext4_grp_locked_error(const char *, unsigned int,
> __ext4_msg(sb, level, fmt, ##__VA_ARGS__)
> #define dump_mmp_msg(sb, mmp, msg) \
> __dump_mmp_msg(sb, mmp, __func__, __LINE__, msg)
> -#define ext4_grp_locked_error(sb, grp, ino, block, fmt, ...) \
> - __ext4_grp_locked_error(__func__, __LINE__, sb, grp, ino, block, \
> - fmt, ##__VA_ARGS__)
> +#define ext4_grp_locked_error(sb, grp, ino, block, flags, fmt, ...) \
> + __ext4_grp_locked_error(__func__, __LINE__, sb, grp, ino, block,\
> + flags, fmt, ##__VA_ARGS__)
> +#define ext4_mark_group_bitmap_corrupted(sb, group, flags, fmt, ...) \
> + do { \
> + __ext4_error(sb, __func__, __LINE__, \
> + fmt, ##__VA_ARGS__); \
> + __ext4_mark_group_bitmap_corrupted(sb, group, flags); \
> + } while (0)
> +
>
> #else
>
> @@ -2634,11 +2641,17 @@ do { \
> } while (0)
> #define dump_mmp_msg(sb, mmp, msg) \
> __dump_mmp_msg(sb, mmp, "", 0, "")
> -#define ext4_grp_locked_error(sb, grp, ino, block, fmt, ...) \
> +#define ext4_grp_locked_error(sb, grp, ino, block, flags, fmt, ...) \
> do { \
> - no_printk(fmt, ##__VA_ARGS__); \
> - __ext4_grp_locked_error("", 0, sb, grp, ino, block, " "); \
> + no_printk(fmt, ##__VA_ARGS__); \
> + __ext4_grp_locked_error("", 0, sb, grp, ino, block, flags, " ");\
> } while (0)
> +#define ext4_mark_group_bitmap_corrupted(sb, group, flags, fmt, ...) \
> + do { \
> + no_printk(fmt, ##__VA_ARGS__); \
> + __ext4_error(sb, "", 0, " "); \
> + __ext4_mark_group_bitmap_corrupted(sb, group, flags); \
> + } while (0)
>
> #endif
>
> diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
> index 4d6e007f3569..b7f729995978 100644
> --- a/fs/ext4/ialloc.c
> +++ b/fs/ext4/ialloc.c
> @@ -94,10 +94,10 @@ static int ext4_validate_inode_bitmap(struct super_block *sb,
> if (!ext4_inode_bitmap_csum_verify(sb, block_group, desc, bh,
> EXT4_INODES_PER_GROUP(sb) / 8)) {
> ext4_unlock_group(sb, block_group);
> - ext4_error(sb, "Corrupt inode bitmap - block_group = %u, "
> - "inode_bitmap = %llu", block_group, blk);
> ext4_mark_group_bitmap_corrupted(sb, block_group,
> - EXT4_GROUP_INFO_IBITMAP_CORRUPT);
> + EXT4_GROUP_INFO_IBITMAP_CORRUPT,
> + "Corrupt inode bitmap - block_group = %u, "
> + "inode_bitmap = %llu", block_group, blk);
> return -EFSBADCRC;
> }
> set_buffer_verified(bh);
> @@ -127,10 +127,10 @@ ext4_read_inode_bitmap(struct super_block *sb, ext4_group_t block_group)
> bitmap_blk = ext4_inode_bitmap(sb, desc);
> if ((bitmap_blk <= le32_to_cpu(sbi->s_es->s_first_data_block)) ||
> (bitmap_blk >= ext4_blocks_count(sbi->s_es))) {
> - ext4_error(sb, "Invalid inode bitmap blk %llu in "
> - "block_group %u", bitmap_blk, block_group);
> ext4_mark_group_bitmap_corrupted(sb, block_group,
> - EXT4_GROUP_INFO_IBITMAP_CORRUPT);
> + EXT4_GROUP_INFO_IBITMAP_CORRUPT,
> + "Invalid inode bitmap blk %llu in "
> + "block_group %u", bitmap_blk, block_group);
> return ERR_PTR(-EFSCORRUPTED);
> }
> bh = sb_getblk(sb, bitmap_blk);
> @@ -182,11 +182,11 @@ ext4_read_inode_bitmap(struct super_block *sb, ext4_group_t block_group)
> wait_on_buffer(bh);
> if (!buffer_uptodate(bh)) {
> put_bh(bh);
> - ext4_error(sb, "Cannot read inode bitmap - "
> - "block_group = %u, inode_bitmap = %llu",
> - block_group, bitmap_blk);
> ext4_mark_group_bitmap_corrupted(sb, block_group,
> - EXT4_GROUP_INFO_IBITMAP_CORRUPT);
> + EXT4_GROUP_INFO_IBITMAP_CORRUPT,
> + "Cannot read inode bitmap - "
> + "block_group = %u, inode_bitmap = %llu",
> + block_group, bitmap_blk);
> return ERR_PTR(-EIO);
> }
>
> @@ -333,9 +333,10 @@ void ext4_free_inode(handle_t *handle, struct inode *inode)
> if (!fatal)
> fatal = err;
> } else {
> - ext4_error(sb, "bit already cleared for inode %lu", ino);
> ext4_mark_group_bitmap_corrupted(sb, block_group,
> - EXT4_GROUP_INFO_IBITMAP_CORRUPT);
> + EXT4_GROUP_INFO_IBITMAP_CORRUPT,
> + "bit already cleared for inode %lu",
> + ino);
> }
>
> error_return:
> @@ -904,10 +905,10 @@ struct inode *__ext4_new_inode(handle_t *handle, struct inode *dir,
> goto next_group;
>
> if (group == 0 && (ino + 1) < EXT4_FIRST_INO(sb)) {
> - ext4_error(sb, "reserved inode found cleared - "
> - "inode=%lu", ino + 1);
> ext4_mark_group_bitmap_corrupted(sb, group,
> - EXT4_GROUP_INFO_IBITMAP_CORRUPT);
> + EXT4_GROUP_INFO_IBITMAP_CORRUPT,
> + "reserved inode found cleared - "
> + "inode=%lu", ino + 1);
> goto next_group;
> }
>
> @@ -1097,10 +1098,10 @@ struct inode *__ext4_new_inode(handle_t *handle, struct inode *dir,
> * twice.
> */
> err = -EIO;
> - ext4_error(sb, "failed to insert inode %lu: doubly allocated?",
> - inode->i_ino);
> ext4_mark_group_bitmap_corrupted(sb, group,
> - EXT4_GROUP_INFO_IBITMAP_CORRUPT);
> + EXT4_GROUP_INFO_IBITMAP_CORRUPT,
> + "failed to insert inode %lu: doubly allocated?",
> + inode->i_ino);
> goto out;
> }
> inode->i_generation = prandom_u32();
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 0d473991eebd..065bce671de4 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -467,11 +467,10 @@ static void mb_free_blocks_double(struct inode *inode, struct ext4_buddy *e4b,
> ext4_grp_locked_error(sb, e4b->bd_group,
> inode ? inode->i_ino : 0,
> blocknr,
> + EXT4_GROUP_INFO_BBITMAP_CORRUPT,
> "freeing block already freed "
> "(bit %u)",
> first + i);
> - ext4_mark_group_bitmap_corrupted(sb, e4b->bd_group,
> - EXT4_GROUP_INFO_BBITMAP_CORRUPT);
> }
> mb_clear_bit(first + i, e4b->bd_info->bb_bitmap);
> }
> @@ -741,6 +740,7 @@ void ext4_mb_generate_buddy(struct super_block *sb,
>
> if (free != grp->bb_free) {
> ext4_grp_locked_error(sb, group, 0, 0,
> + EXT4_GROUP_INFO_BBITMAP_CORRUPT,
> "block bitmap and bg descriptor "
> "inconsistent: %u vs %u free clusters",
> free, grp->bb_free);
> @@ -749,8 +749,6 @@ void ext4_mb_generate_buddy(struct super_block *sb,
> * corrupt and update bb_free using bitmap value
> */
> grp->bb_free = free;
> - ext4_mark_group_bitmap_corrupted(sb, group,
> - EXT4_GROUP_INFO_BBITMAP_CORRUPT);
> }
> mb_set_largest_free_order(sb, grp);
>
> @@ -1451,11 +1449,10 @@ static void mb_free_blocks(struct inode *inode, struct ext4_buddy *e4b,
> ext4_grp_locked_error(sb, e4b->bd_group,
> inode ? inode->i_ino : 0,
> blocknr,
> + EXT4_GROUP_INFO_BBITMAP_CORRUPT,
> "freeing already freed block "
> "(bit %u); block bitmap corrupt.",
> block);
> - ext4_mark_group_bitmap_corrupted(sb, e4b->bd_group,
> - EXT4_GROUP_INFO_BBITMAP_CORRUPT);
> mb_regenerate_buddy(e4b);
> goto done;
> }
> @@ -1949,11 +1946,10 @@ void ext4_mb_complex_scan_group(struct ext4_allocation_context *ac,
> * we have free blocks
> */
> ext4_grp_locked_error(sb, e4b->bd_group, 0, 0,
> + EXT4_GROUP_INFO_BBITMAP_CORRUPT,
> "%d free clusters as per "
> "group info. But bitmap says 0",
> free);
> - ext4_mark_group_bitmap_corrupted(sb, e4b->bd_group,
> - EXT4_GROUP_INFO_BBITMAP_CORRUPT);
> break;
> }
>
> @@ -1961,11 +1957,10 @@ void ext4_mb_complex_scan_group(struct ext4_allocation_context *ac,
> BUG_ON(ex.fe_len <= 0);
> if (free < ex.fe_len) {
> ext4_grp_locked_error(sb, e4b->bd_group, 0, 0,
> + EXT4_GROUP_INFO_BBITMAP_CORRUPT,
> "%d free clusters as per "
> "group info. But got %d blocks",
> free, ex.fe_len);
> - ext4_mark_group_bitmap_corrupted(sb, e4b->bd_group,
> - EXT4_GROUP_INFO_BBITMAP_CORRUPT);
> /*
> * The number of free blocks differs. This mostly
> * indicate that the bitmap is corrupt. So exit
> @@ -3851,7 +3846,8 @@ ext4_mb_release_inode_pa(struct ext4_buddy *e4b, struct buffer_head *bitmap_bh,
> pa, (unsigned long) pa->pa_lstart,
> (unsigned long) pa->pa_pstart,
> (unsigned long) pa->pa_len);
> - ext4_grp_locked_error(sb, group, 0, 0, "free %u, pa_free %u",
> + ext4_grp_locked_error(sb, group, 0, 0, 0,
> + "free %u, pa_free %u",
> free, pa->pa_free);
> /*
> * pa is already deleted so we use the value obtained
> @@ -4679,7 +4675,7 @@ ext4_mb_free_metadata(handle_t *handle, struct ext4_buddy *e4b,
> else {
> ext4_grp_locked_error(sb, group, 0,
> ext4_group_first_block_no(sb, group) +
> - EXT4_C2B(sbi, cluster),
> + EXT4_C2B(sbi, cluster), 0,
> "Block already on to-be-freed list");
> return 0;
> }
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index f5e9a1af067a..96981df03385 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -709,7 +709,7 @@ void __ext4_warning_inode(const struct inode *inode, const char *function,
> void __ext4_grp_locked_error(const char *function, unsigned int line,
> struct super_block *sb, ext4_group_t grp,
> unsigned long ino, ext4_fsblk_t block,
> - const char *fmt, ...)
> + unsigned int flags, const char *fmt, ...)
> __releases(bitlock)
> __acquires(bitlock)
> {
> @@ -740,6 +740,9 @@ __acquires(bitlock)
> va_end(args);
> }
>
> + if (flags)
> + __ext4_mark_group_bitmap_corrupted(sb, grp, flags);
> +
> if (test_opt(sb, ERRORS_CONT)) {
> ext4_commit_super(sb, 0);
> return;
> @@ -763,9 +766,9 @@ __acquires(bitlock)
> return;
> }
>
> -void ext4_mark_group_bitmap_corrupted(struct super_block *sb,
> - ext4_group_t group,
> - unsigned int flags)
> +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);
> --
> 2.14.3
>
Cheers, Andreas
Download attachment "signature.asc" of type "application/pgp-signature" (874 bytes)
Powered by blists - more mailing lists