Re: [PATCH v4 8/9] md/raid1,raid10: set IO_BLOCKED in case of BLK_STS_P2PDMA

[Logan Gunthorpe]

On 2026-07-22 12:58, Mykola Marzhan wrote:
> From: Logan Gunthorpe <logang@xxxxxxxxxxxx>
> 
> A read that fails with BLK_STS_P2PDMA cannot succeed against that
> member, so mark the leg IO_BLOCKED and let the retry redirect elsewhere.
> Skip the read-error machinery: there is nothing on the medium to fix,
> fix_read_error()'s probe reads into host pages and would "succeed", and
> charging the read-error budget would evict a healthy member under a P2P
> read workload.  FailFast eviction is skipped for the same reason -- the
> request never reached the wire.
> 
> Fixes: 02666132403a ("md: propagate BLK_FEAT_PCI_P2PDMA from member devices to RAID device")
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Logan Gunthorpe <logang@xxxxxxxxxxxx>
> Co-developed-by: Mykola Marzhan <mykola@xxxxxxxxxxx>
> Signed-off-by: Mykola Marzhan <mykola@xxxxxxxxxxx>
> ---
>  drivers/md/raid1.c  | 4 +++-
>  drivers/md/raid10.c | 5 ++++-
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 917d694ef401..76a1426e64f0 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -2658,6 +2658,8 @@ static void handle_read_error(struct r1conf *conf, struct r1bio *r1_bio)
>  {
>  	struct md_rdev *rdev = conf->mirrors[r1_bio->read_disk].rdev;
>  	struct bio *bio = r1_bio->bios[r1_bio->read_disk];
> +	/* evaluate before the bio_put() below */
> +	bool p2pdma_error = bio->bi_status == BLK_STS_P2PDMA;
>  	struct mddev *mddev = conf->mddev;
>  	sector_t sector;
>  
> @@ -2675,7 +2677,7 @@ static void handle_read_error(struct r1conf *conf, struct r1bio *r1_bio)
>  	 * read error.  This is all done synchronously while the array is
>  	 * frozen.
>  	 */
> -	if (mddev->ro) {
> +	if (mddev->ro || p2pdma_error) {
>  		r1_bio->bios[r1_bio->read_disk] = IO_BLOCKED;
>  	} else if (test_bit(FailFast, &rdev->flags)) {
>  		md_error(mddev, rdev);
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 9045a3f02dae..e35aeac29f06 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -2848,6 +2848,7 @@ static void handle_read_error(struct mddev *mddev, struct r10bio *r10_bio)
>  {
>  	int slot = r10_bio->read_slot;
>  	struct bio *bio;
> +	bool p2pdma_error;
>  	struct r10conf *conf = mddev->private;
>  	struct md_rdev *rdev = r10_bio->devs[slot].rdev;
>  
> @@ -2860,10 +2861,12 @@ static void handle_read_error(struct mddev *mddev, struct r10bio *r10_bio)
>  	 * frozen.
>  	 */
>  	bio = r10_bio->devs[slot].bio;
> +	/* evaluate before the bio_put() below */
> +	p2pdma_error = bio->bi_status == BLK_STS_P2PDMA;

I see why we needed to add the p2pdma_error variable now. I guess I
missed that. But can we maybe do both raid10 and raid1 the same even if
it means moving the `bio =` line up a bit? I think it would be better to
move the code more towards similarity with raid1 instead of diverging it
further. Maybe someday it can be cleaned up into more common code and
that will be easier if we make both changes the same.

Thanks,

Logan