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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Nested double allocation from the same `conf->bio_split` mempool can cause a self-deadlock when a bio crosses a chunk or reshape boundary and encounters a bad block.
- [Low] Passing 'int *' to 'unsigned int *' triggers a compiler error with -Werror=incompatible-pointer-types.
--

--- Patch [21]: [PATCH v4 21/25] md/raid10: split reshape bios before bitmap accounting ---
commit ac17607a8057c3c9040bf0443e50aa65062e0d48
Author: Yu Kuai <yukuai@fygo.io>

md/raid10: split reshape bios before bitmap accounting

This commit updates raid10 to submit only one-side bios to llbitmap
during reshape. It accomplishes this by using the shared helper
mddev_bio_split_at_reshape_offset() to split bios at reshape boundaries.

> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index e50e4adee389f..61900ace190fd 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -1886,6 +1886,7 @@ static bool raid10_make_request(struct mddev *mddev, struct bio *bio)
>  {
>  	struct r10conf *conf = mddev->private;
>  	sector_t chunk_mask = (conf->geo.chunk_mask & conf->prev.chunk_mask);
> +	const int rw = bio_data_dir(bio);
>  	int chunk_sects = chunk_mask + 1;
>  	int sectors = bio_sectors(bio);
>  
[ ... ]
> @@ -1911,6 +1912,15 @@ static bool raid10_make_request(struct mddev *mddev, struct bio *bio)
>  		sectors = chunk_sects -
>  			(bio->bi_iter.bi_sector &
>  			 (chunk_sects - 1));
> +
> +	bio = mddev_bio_split_at_reshape_offset(mddev, bio, &sectors,
> +						&conf->bio_split);

[Severity: Low]
Will this cause a build failure with -Werror=incompatible-pointer-types?

The sectors variable is declared as an int, but
mddev_bio_split_at_reshape_offset() expects an unsigned int pointer for
its third argument.

[Severity: High]
Could this introduce a mempool self-deadlock when encountering a bad block?

If the bio crosses a chunk or reshape boundary, it allocates a split bio from
conf->bio_split here. This newly allocated bio is then passed down the stack:

raid10_make_request() -> __make_request() -> raid10_write_request()

If a partial bad block is encountered in raid10_write_request():

drivers/md/raid10.c:raid10_write_request() {
    ...
    if (r10_bio->sectors < bio_sectors(bio)) {
        ...
        bio = bio_submit_split_bioset(bio, r10_bio->sectors, &conf->bio_split);
    ...
}

It attempts to split the bio a second time using the exact same
conf->bio_split mempool. Because the original bio is already holding an
element from conf->bio_split, nested allocations from the same pool by
concurrent threads can exhaust it and cause the I/O threads to hang.

> +	if (!bio) {
> +		if (rw == WRITE)
> +			md_write_end(mddev);
> +		return true;
> +	}
> +
>  	if (!__make_request(mddev, bio, sectors))
>  		md_write_end(mddev);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260801172519.2982121-1-yukuai@kernel.org?part=21