Re: [PATCH v2 16/20] md/raid10: split reshape bios before bitmap accounting

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing `md_write_end()` on `mddev_bio_split_at_reshape_offset()` failure path causes `writes_pending` leak.
- [Medium] Lockless read of `mddev->reshape_position` can result in a torn read on 32-bit architectures, leading to incorrect bio splitting.
--

commit 2a6f82a757276f6bbae0c22cb819c9c6d6b3e4b0
Author: Yu Kuai <[email protected]>

md/raid10: split reshape bios before bitmap accounting

Use the shared mddev_bio_split_at_reshape_offset() helper so RAID10
submits only one-side bios to llbitmap during reshape.

> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -1911,6 +1911,12 @@ 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: Medium]
Can the lockless read of mddev->reshape_position inside this helper
result in a torn read on 32-bit architectures?

In mddev_bio_split_at_reshape_offset(), mddev->reshape_position (a 64-bit
sector_t) is read without READ_ONCE() or a seqcount:

drivers/md/md.c:mddev_bio_split_at_reshape_offset() {
    ...
    boundary = mddev->reshape_position;
    ...
}

On 32-bit systems, could racing with the reshape thread's update (for
example, at the 2TB boundary) tear the 64-bit load and yield a garbage
sector value?

If so, this might cause bio_split to slice the bio at an unaligned offset,
leading to IO errors when the underlying block device rejects it.

> +	if (!bio)
> +		return true;

[Severity: High]
Does this early return bypass the md_write_end() cleanup required to
balance the earlier md_write_start() for write requests?

If mddev_bio_split_at_reshape_offset() fails and returns NULL, it appears
the writes_pending percpu reference is leaked.

Will this prevent the array from reaching the clean safemode state, causing
it to perpetually appear dirty and trigger a full resync on the next
assembly?

> +
>  	if (!__make_request(mddev, bio, sectors))
>  		md_write_end(mddev);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=16