Re: [PATCH v4 07/25] md: skip bitmap accounting for empty write ranges

Thank you for your contribution! Sashiko AI review found 1 potential issue(=
s) to consider:

Pre-existing issues:
- [High] Uninitialized `start_time` leads to corrupted IO accounting, and u=
sing `0` as a sentinel causes in-flight IO leaks when `jiffies` wraps to 0.
--

--- Patch [7]: [PATCH v4 07/25] md: skip bitmap accounting for empty write =
ranges ---
commit b61151db244d77a3d38d7fc791d0c00d7e6a2723
Author: Yu Kuai <[email protected]>

md: skip bitmap accounting for empty write ranges

This commit prevents zero-sector flush/FUA bios from dirtying bitmap bits.
It skips bitmap accounting for empty write ranges by verifying the bio has
sectors in both md_bitmap_start() and md_clone_bio().

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 3b20a57b8c7ed..0b59c676f7c03 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
[ ... ]
> @@ -9423,10 +9426,12 @@ static void md_clone_bio(struct mddev *mddev, str=
uct bio **bio)
>  	md_io_clone =3D container_of(clone, struct md_io_clone, bio_clone);
>  	md_io_clone->orig_bio =3D *bio;
>  	md_io_clone->mddev =3D mddev;
> +	md_io_clone->sectors =3D 0;
>  	if (blk_queue_io_stat(bdev->bd_disk->queue))
>  		md_io_clone->start_time =3D bio_start_io_acct(*bio);

[Severity: High]
This is a pre-existing issue, but does md_clone_bio() leave start_time
uninitialized when IO stats are disabled?

Since md_io_clone is part of a bio allocation from a mempool, it might not
be zero-initialized. If blk_queue_io_stat() returns false, start_time could=
=20
retain garbage memory.=20

Later, in md_end_clone_io(), the completion path checks:

md_end_clone_io() {
        ...
        if (md_io_clone->start_time)
                bio_end_io_acct(orig_bio, md_io_clone->start_time);
}

If start_time contains garbage, couldn't this falsely trigger=20
bio_end_io_acct() and decrement counters that were never incremented,=20
corrupting the block layer metrics?

Additionally, if jiffies happens to be exactly 0 when bio_start_io_acct()=20
is called, start_time becomes 0. The sentinel check in md_end_clone_io()=20
would then evaluate to false, skipping bio_end_io_acct() and permanently=20
leaking the in-flight IO counter. This leak prevents q_usage_counter from=20
reaching zero, which would cause blk_mq_freeze_queue() to hang indefinitely=
=20
during block device teardown.

Could start_time be explicitly initialized, or perhaps tracked with a=20
separate flag instead of relying on 0 as a sentinel?

> -	if (bio_data_dir(*bio) =3D=3D WRITE && md_bitmap_enabled(mddev, false))=
 {
> +	if (bio_data_dir(*bio) =3D=3D WRITE && bio_sectors(*bio) &&
> +	    md_bitmap_enabled(mddev, false)) {
>  		md_io_clone->offset =3D (*bio)->bi_iter.bi_sector;
>  		md_io_clone->sectors =3D bio_sectors(*bio);
>  		md_io_clone->rw =3D op_stat_group(bio_op(*bio));

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260801172519.2982=
[email protected]?part=3D7