On Thu 17-11-22 12:03:40, Baokun Li wrote:
> In commit 9a8c5b0d0615 ("ext4: update the backup superblock's at the end
> of the online resize"), it is assumed that update_backups() only updates
> backup superblocks, so each b_data is treated as a backupsuper block to
> update its s_block_group_nr and s_checksum. However, update_backups()
> also updates the backup group descriptors, which causes the backup group
> descriptors to be corrupted.
>
> The above commit fixes the problem of invalid checksum of the backup
> superblock. The root cause of this problem is that the checksum of
> ext4_update_super() is not set correctly. This problem has been fixed
> in the previous patch ("ext4: fix bad checksum after online resize").
>
> However, we do need to set block_group_nr for the backup superblock in
> update_backups(). When a block is in a group that contains a backup
> superblock, and the block is the first block in the group, the block is
> definitely a superblock. We add a helper function that includes setting
> s_block_group_nr and updating checksum, and then call it only when the
> above conditions are met to prevent the backup group descriptors from
> being incorrectly modified.
>
> Fixes: 9a8c5b0d0615 ("ext4: update the backup superblock's at the end of the online resize")
> Signed-off-by: Baokun Li <libaokun1@xxxxxxxxxx>
Looks good to me now. Feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> ---
> V2->V3:
> Modify the scheme and retain s_block_group_nr.
>
> fs/ext4/resize.c | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
> index cb99b410c9fa..66ceabbd83ad 100644
> --- a/fs/ext4/resize.c
> +++ b/fs/ext4/resize.c
> @@ -1110,6 +1110,16 @@ static int reserve_backup_gdb(handle_t *handle, struct inode *inode,
> return err;
> }
>
> +static inline void ext4_set_block_group_nr(struct super_block *sb, char *data,
> + ext4_group_t group)
> +{
> + struct ext4_super_block *es = (struct ext4_super_block *) data;
> +
> + es->s_block_group_nr = cpu_to_le16(group);
> + if (ext4_has_metadata_csum(sb))
> + es->s_checksum = ext4_superblock_csum(sb, es);
> +}
> +
> /*
> * Update the backup copies of the ext4 metadata. These don't need to be part
> * of the main resize transaction, because e2fsck will re-write them if there
> @@ -1158,7 +1168,8 @@ static void update_backups(struct super_block *sb, sector_t blk_off, char *data,
> while (group < sbi->s_groups_count) {
> struct buffer_head *bh;
> ext4_fsblk_t backup_block;
> - struct ext4_super_block *es;
> + int has_super = ext4_bg_has_super(sb, group);
> + ext4_fsblk_t first_block = ext4_group_first_block_no(sb, group);
>
> /* Out of journal space, and can't get more - abort - so sad */
> err = ext4_resize_ensure_credits_batch(handle, 1);
> @@ -1168,8 +1179,7 @@ static void update_backups(struct super_block *sb, sector_t blk_off, char *data,
> if (meta_bg == 0)
> backup_block = ((ext4_fsblk_t)group) * bpg + blk_off;
> else
> - backup_block = (ext4_group_first_block_no(sb, group) +
> - ext4_bg_has_super(sb, group));
> + backup_block = first_block + has_super;
>
> bh = sb_getblk(sb, backup_block);
> if (unlikely(!bh)) {
> @@ -1187,10 +1197,8 @@ static void update_backups(struct super_block *sb, sector_t blk_off, char *data,
> memcpy(bh->b_data, data, size);
> if (rest)
> memset(bh->b_data + size, 0, rest);
> - es = (struct ext4_super_block *) bh->b_data;
> - es->s_block_group_nr = cpu_to_le16(group);
> - if (ext4_has_metadata_csum(sb))
> - es->s_checksum = ext4_superblock_csum(sb, es);
> + if (has_super && (backup_block == first_block))
> + ext4_set_block_group_nr(sb, bh->b_data, group);
> set_buffer_uptodate(bh);
> unlock_buffer(bh);
> err = ext4_handle_dirty_metadata(handle, NULL, bh);
> --
> 2.31.1
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
