Re: [PATCH RFC 09/17] ext4: add ext4_inode_bitmap_csum_set_fast() for incremental checksum update

在 2026/5/9 06:58, [email protected] 写道:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Mismatched checksum calculation size in ext4_inode_bitmap_csum_set_fast() for filesystems with s_inodes_per_group not aligned to 8.
> --
>
> commit 5850c42dcf8baa647b659620b26a9dfeda77192c
> Author: Baokun Li <[email protected]>
>
> ext4: add ext4_inode_bitmap_csum_set_fast() for incremental checksum update
>
> This commit adds a helper function to incrementally update the inode bitmap
> checksum using crc32c_flip_range() after flipping a single bit.
>
>> diff --git a/fs/ext4/bitmap.c b/fs/ext4/bitmap.c
>> --- a/fs/ext4/bitmap.c
>> +++ b/fs/ext4/bitmap.c
> [ ... ]
>> @@ -71,6 +71,29 @@ void ext4_inode_bitmap_csum_set(struct super_block *sb,
>>  	ext4_inode_bitmap_csum_store(sb, gdp, csum);
>>  }
>>  
>> +/*
>> + * Update inode bitmap checksum for a single flipped bit.
>> + *
>> + * Use crc32c_flip_range() to incrementally update the checksum after
>> + * flipping the bit at @offset, avoiding a full bitmap CRC rescan.
>> + * The csum_seed cancels out in the XOR delta, so it is not needed here.
>> + */
>> +void ext4_inode_bitmap_csum_set_fast(struct super_block *sb,
>> +				     struct ext4_group_desc *gdp,
>> +				     ext4_grpblk_t offset)
>> +{
>> +	__u32 new_csum, old_csum;
>> +
>> +	if (!ext4_has_feature_metadata_csum(sb))
>> +		return;
>> +
>> +	old_csum = ext4_inode_bitmap_csum_get(sb, gdp);
>> +	new_csum = crc32c_flip_range(old_csum, EXT4_INODES_PER_GROUP(sb),
>> +				     offset, 1);
> Does this code properly handle filesystems where s_inodes_per_group is not
> a multiple of 8?
>
> Looking at the full recalculation path in ext4_inode_bitmap_csum_set(), the
> checksum is calculated over a byte-aligned size:
>
> fs/ext4/bitmap.c:ext4_inode_bitmap_csum_set() {
>     ...
>     sz = EXT4_INODES_PER_GROUP(sb) >> 3;
>     csum = ext4_chksum(sbi->s_csum_seed, (__u8 *)bh->b_data, sz);
>     ...
> }
>
> Since ext4_inode_bitmap_csum_set() effectively truncates the length to a
> multiple of 8 bytes, while the fast path uses the exact EXT4_INODES_PER_GROUP
> value as total_bits, could this mismatch cause the trailing_bits calculation
> in crc32c_flip_range() to shift the XOR delta incorrectly?
>
> If a filesystem image has an unaligned s_inodes_per_group, it appears this
> could result in a checksum mismatch on subsequent verifications.
>
> Additionally, if the offset falls in the unaligned bits (for example, if
> offset >= sz * 8), would this fast path apply a delta while the full rescan
> ignores those bits entirely?
>

When formatting, it is ensured that EXT4_INODES_PER_GROUP is a multiple
of 8, so this is an implicit guarantee, and there are no explicit checks
in the kernel or fsck.

To be safe, in the next version, I will, like ext4_inode_bitmap_csum_set,
align it down to 8.


Thanks,
Baokun