On Mon 29-03-21 14:19:55, Zhang Yi wrote:
> Commit <50122847007> ("ext4: fix check to prevent initializing reserved
> inodes") check the block group zero and prevent initializing reserved
> inodes. But in some special cases, the reserved inode may not all belong
> to the group zero, it may exist into the second group if we format
> filesystem below.
>
> mkfs.ext4 -b 4096 -g 8192 -N 1024 -I 4096 /dev/sda
>
> So, it will end up triggering a false positive report of a corrupted
> file system. This patch fix it by avoid check reserved inodes if no free
> inode blocks will be zeroed.
>
> Fixes: 50122847007 ("ext4: fix check to prevent initializing reserved inodes")
> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
Thanks! The patch looks correct but maybe the code can be made more
comprehensible like I suggest below?
> @@ -1543,22 +1544,25 @@ int ext4_init_inode_table(struct super_block *sb, ext4_group_t group,
> * used inodes so we need to skip blocks with used inodes in
> * inode table.
> */
> - if (!(gdp->bg_flags & cpu_to_le16(EXT4_BG_INODE_UNINIT)))
> - used_blks = DIV_ROUND_UP((EXT4_INODES_PER_GROUP(sb) -
> - ext4_itable_unused_count(sb, gdp)),
> - sbi->s_inodes_per_block);
> -
> - if ((used_blks < 0) || (used_blks > sbi->s_itb_per_group) ||
> - ((group == 0) && ((EXT4_INODES_PER_GROUP(sb) -
> - ext4_itable_unused_count(sb, gdp)) <
> - EXT4_FIRST_INO(sb)))) {
> - ext4_error(sb, "Something is wrong with group %u: "
> - "used itable blocks: %d; "
> - "itable unused count: %u",
> - group, used_blks,
> - ext4_itable_unused_count(sb, gdp));
> - ret = 1;
> - goto err_out;
> + if (!(gdp->bg_flags & cpu_to_le16(EXT4_BG_INODE_UNINIT))) {
> + used_inos = EXT4_INODES_PER_GROUP(sb) -
> + ext4_itable_unused_count(sb, gdp);
> + used_blks = DIV_ROUND_UP(used_inos, sbi->s_inodes_per_block);
> +
> + if (used_blks >= 0 && used_blks <= sbi->s_itb_per_group)
> + used_inos += group * EXT4_INODES_PER_GROUP(sb);
Maybe if would be more comprehensible like:
/* Bogus inode unused count? */
if (used_blks < 0 || used_blks > sbi->s_itb_per_group) {
ext4_error(...);
ret = 1;
goto err_out;
}
used_inos += EXT4_INODES_PER_GROUP(sb);
/*
* Are there some uninitialized inodes in the inode table
* before the first normal inode?
*/
if (used_blks != sbi->s_itb_per_group &&
used_inos < EXT4_FIRST_INO(sb)) {
ext4_error(...);
ret = 1;
goto err_out;
}
> +
> + if ((used_blks < 0) || (used_blks > sbi->s_itb_per_group) ||
> + ((used_blks != sbi->s_itb_per_group) &&
> + (used_inos < EXT4_FIRST_INO(sb)))) {
> + ext4_error(sb, "Something is wrong with group %u: "
> + "used itable blocks: %d; "
> + "itable unused count: %u",
> + group, used_blks,
> + ext4_itable_unused_count(sb, gdp));
> + ret = 1;
> + goto err_out;
> + }
> }
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
