On Wed 16-11-22 15:28:01, 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").
> Therefore, roll back some modifications in the above commit.
>
> Fixes: 9a8c5b0d0615 ("ext4: update the backup superblock's at the end of the online resize")
> Signed-off-by: Baokun Li <libaokun1@xxxxxxxxxx>
So I agree commit 9a8c5b0d0615 is broken and does corrupt group
descriptors. However I don't see how PATCH 1/3 in this series would fix all
the problems commit 9a8c5b0d0615 is trying to fix. In particular checksums
on backup superblocks will not be properly set by the resize code AFAICT.
Honza
> ---
> fs/ext4/resize.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
> index cb99b410c9fa..32fbfc173571 100644
> --- a/fs/ext4/resize.c
> +++ b/fs/ext4/resize.c
> @@ -1158,7 +1158,6 @@ 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;
>
> /* Out of journal space, and can't get more - abort - so sad */
> err = ext4_resize_ensure_credits_batch(handle, 1);
> @@ -1187,10 +1186,6 @@ 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);
> 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
