Re: [PATCH v4 25/25] md/raid5: split reshape bios before bitmap accounting

Thank you for your contribution! Sashiko AI review found 2 potential issue(=
s) to consider:
- [Medium] Double accounting of cgroup I/O statistics and double throttling=
 for resubmitted split bios.
- [Low] The error handling path for mddev_bio_split_at_reshape_offset() ret=
urning NULL is unreachable dead code.
--

--- Patch [25]: [PATCH v4 25/25] md/raid5: split reshape bios before bitmap=
 accounting ---
commit d432335f5c2f577d3656123bc3166f0ab441619d
Author: Yu Kuai <[email protected]>

md/raid5: split reshape bios before bitmap accounting

RAID5 maps array sectors through different geometries before and after the
reshape position. This commit splits bios that cross the reshape boundary
before md_account_bio() so the bitmap only sees ranges belonging to one
side. The split front bio is resubmitted directly after the reshape wait.

> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c

[ ... ]

> @@ -6184,6 +6186,18 @@ static bool raid5_make_request(struct mddev *mddev=
, struct bio * bi)
>  		return true;
>  	}
> =20
> +	front_bio =3D bi;
> +	bi =3D mddev_bio_split_at_reshape_offset(mddev, bi, NULL,
> +					       &conf->bio_split);
> +	if (!bi) {
> +		if (rw =3D=3D WRITE)
> +			md_write_end(mddev);
> +		return true;
> +	}

[Severity: Low]
Is this error handling path reachable?

mddev_bio_split_at_reshape_offset() uses bio_submit_split_bioset() to split
the bio using the conf->bio_split bioset. Since this is a mempool-backed
allocation with GFP_NOIO that reuses the original bvecs, the allocation
cannot fail.

> +	if (bi !=3D front_bio)
> +		split =3D true;
> +	front_bio =3D bi;
> +
>  	logical_sector =3D bi->bi_iter.bi_sector & ~((sector_t)RAID5_STRIPE_SEC=
TORS(conf)-1);
>  	bi->bi_next =3D NULL;

[ ... ]

> @@ -6284,6 +6298,11 @@ static bool raid5_make_request(struct mddev *mddev=
, struct bio * bi)
>  		bio_endio(bi);
> =20
>  		wait_for_completion(&done);
> +		front_bio->bi_status =3D BLK_STS_OK;
> +		if (split) {
> +			submit_bio_noacct(front_bio);

[Severity: Medium]
Will this cause the front split bio to be accounted and throttled twice?

When mddev_bio_split_at_reshape_offset() splits the bio, the newly allocated
clone (the front bio) does not inherit the BIO_CGROUP_ACCT or
BIO_BPS_THROTTLED flags from the original bio.

When make_stripe_request() returns STRIPE_WAIT_RESHAPE, this code resubmits
the split bio through submit_bio_noacct(). Because the flags are missing,
blk_cgroup_bio_start() and blk_throtl_bio() will account and throttle this
same I/O a second time.

> +			return true;
> +		}
>  		return false;
>  	}

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