All of lore.kernel.org
 help / color / mirror / Atom feed
From: Baokun Li <libaokun@linux.alibaba.com>
To: sashiko@lists.linux.dev
Cc: tytso@mit.edu, libaokun@linux.alibaba.com
Subject: Re: [PATCH RFC 09/17] ext4: add ext4_inode_bitmap_csum_set_fast() for incremental checksum update
Date: Mon, 11 May 2026 16:35:06 +0800	[thread overview]
Message-ID: <8fd8ab9b-a4e9-4857-90f5-55ba7e5b9070@linux.alibaba.com> (raw)
In-Reply-To: <20260508225807.71D9FC2BCB0@smtp.kernel.org>

在 2026/5/9 06:58, sashiko-bot@kernel.org 写道:
> 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 <libaokun@linux.alibaba.com>
>
> 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



  parent reply	other threads:[~2026-05-11  8:35 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-08 12:15 [PATCH RFC 00/17] ext4/lib-crc: LBS performance part 1 - incremental CRC32c for bitmap checksums Baokun Li
2026-05-08 12:15 ` [PATCH RFC 01/17] lib/crc: add crc32c_flip_range() for incremental CRC update Baokun Li
     [not found]   ` <20260508204019.9E5A5C2BCB0@smtp.kernel.org>
2026-05-10  9:44     ` Baokun Li
2026-05-14  3:52   ` Eric Biggers
2026-05-08 12:15 ` [PATCH RFC 02/17] lib/crc: crc_kunit: add kunit test for crc32c_flip_range() Baokun Li
2026-05-08 12:15 ` [PATCH RFC 03/17] lib/crc: crc_kunit: add benchmark " Baokun Li
     [not found]   ` <20260508205415.8B843C2BCB0@smtp.kernel.org>
2026-05-10 10:03     ` Baokun Li
2026-05-08 12:15 ` [PATCH RFC 04/17] ext4: fix incorrect block bitmap free clusters update on metadata overlap Baokun Li
     [not found]   ` <20260508211732.E50B4C2BCB0@smtp.kernel.org>
2026-05-11  6:17     ` Baokun Li
2026-05-08 12:15 ` [PATCH RFC 05/17] ext4: extract block bitmap checksum get and store helpers Baokun Li
2026-05-08 12:15 ` [PATCH RFC 06/17] ext4: add ext4_block_bitmap_csum_set_range() for incremental checksum update Baokun Li
     [not found]   ` <20260508214640.B3A74C2BCB0@smtp.kernel.org>
2026-05-11  8:09     ` Baokun Li
2026-05-11  8:31     ` Baokun Li
2026-05-08 12:15 ` [PATCH RFC 07/17] ext4: use fast incremental CRC update in ext4_mb_mark_context() Baokun Li
     [not found]   ` <20260508223130.20E7AC2BCB0@smtp.kernel.org>
2026-05-11  8:15     ` Baokun Li
2026-05-08 12:15 ` [PATCH RFC 08/17] ext4: extract inode bitmap checksum get and store helpers Baokun Li
2026-05-08 12:15 ` [PATCH RFC 09/17] ext4: add ext4_inode_bitmap_csum_set_fast() for incremental checksum update Baokun Li
     [not found]   ` <20260508225807.71D9FC2BCB0@smtp.kernel.org>
2026-05-11  8:35     ` Baokun Li [this message]
2026-05-08 12:15 ` [PATCH RFC 10/17] ext4: use fast incremental CRC update in ext4_free_inode() Baokun Li
2026-05-08 12:15 ` [PATCH RFC 11/17] ext4: fix missing bg_used_dirs_count update in fast commit replay Baokun Li
2026-05-08 12:15 ` [PATCH RFC 12/17] ext4: factor out ext4_might_init_block_bitmap() helper Baokun Li
2026-05-08 12:15 ` [PATCH RFC 13/17] ext4: use fast incremental CRC update in ext4_mark_inode_used() Baokun Li
2026-05-08 12:15 ` [PATCH RFC 14/17] ext4: rename ino to bit in __ext4_new_inode() Baokun Li
2026-05-08 12:15 ` [PATCH RFC 15/17] ext4: use fast incremental CRC update " Baokun Li
2026-05-08 12:15 ` [PATCH RFC 16/17] ext4: extract ext4_update_inode_group_desc() to reduce duplication Baokun Li
2026-05-08 12:15 ` [PATCH RFC 17/17] ext4: add ext4_get_flex_group() helper to simplify flex group lookups Baokun Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8fd8ab9b-a4e9-4857-90f5-55ba7e5b9070@linux.alibaba.com \
    --to=libaokun@linux.alibaba.com \
    --cc=sashiko@lists.linux.dev \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.