Re: [PATCH v6 06/12] md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones

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

Pre-existing issues:
- [High] Unconditional assignment to `wbio->bi_opf` destroys all request flags cloned from the master bio, leading to potential data corruption.
--

--- Patch [6]: [PATCH v6 06/12] md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones ---
commit 333e1430f8770afc53cf111be8c36899e6c4c5e3
Author: Mykola Marzhan <[email protected]>

md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones

This commit restores REQ_NOMERGE on P2PDMA retry clones in
narrow_write_error() to prevent member queues from improperly merging
segments.

> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index f07edeac6e5a3..19c4dec450de0 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
[ ... ]
> @@ -2561,6 +2561,9 @@ static void narrow_write_error(struct r1bio *r1_bio, int i)
>  		}
>  
>  		wbio->bi_opf = REQ_OP_WRITE;

[Severity: High]
This isn't a bug introduced by this patch, but does this assignment
unintentionally overwrite all request flags?

Since bio_alloc_clone() copies the original bi_opf, setting it directly to
REQ_OP_WRITE zeroes out the upper 24 flag bits, which drops flags like
REQ_FUA and REQ_PREFLUSH.

Could this result in data corruption on power loss if the original write
requested forced unit access?

> +		/* Keep P2PDMA retry bios unmergeable, like the original */
> +		if (md_bio_is_p2pdma(wbio))
> +			wbio->bi_opf |= REQ_NOMERGE;
>  		wbio->bi_iter.bi_sector = r1_bio->sector;
>  		wbio->bi_iter.bi_size = r1_bio->sectors << 9;
>  
[ ... ]
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 5b63175fb3826..428bddddfe71d 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
[ ... ]
> @@ -2823,6 +2823,9 @@ static void narrow_write_error(struct r10bio *r10_bio, int i)
>  		wbio->bi_iter.bi_sector = wsector +
>  				   choose_data_offset(r10_bio, rdev);
>  		wbio->bi_opf = REQ_OP_WRITE;

[Severity: High]
This is the same pre-existing issue as in raid1.c. Does overwriting bi_opf
drop critical data integrity flags like REQ_FUA cloned from the master bio?

> +		/* Keep P2PDMA retry bios unmergeable, like the original */
> +		if (md_bio_is_p2pdma(wbio))
> +			wbio->bi_opf |= REQ_NOMERGE;
>  
>  		if (submit_bio_wait(wbio) &&
>  		    !rdev_set_badblocks(rdev, wsector, sectors, 0)) {

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