[PATCH v3 0/8] block,md,nvme: correct handling of unsupported P2PDMA transfers

[PATCH v3 0/8] block,md,nvme: correct handling of unsupported P2PDMA transfers

[Mykola Marzhan]

md treats an unsupported P2PDMA transfer to a member as success, and
nvme-rdma retries one forever.  Restore BLK_STS_TARGET for these
failures (lost in the blk_rq_dma_map conversion, v6.17) and fix what
md and nvme-rdma do around it.

Whether peer memory can be DMA-mapped depends on the PCIe topology
between the two devices: the same buffer may map fine for one
array member or nvme path and fail for another.  Since v6.17 that
failure completes as BLK_STS_INVAL.

md deliberately ignores INVAL member failures (commit
f7b24c7b41f2) and accepts P2PDMA bios since v7.2-rc1.  So a peer
write to an unreachable member simply counts as written: mirrors
silently diverge, and with no member reachable the write still
reports success.  nvme-rdma never even sees the errno --
ib_dma_map_sg() returns 0 -- and reports a path error, which
default multipath requeues forever.

  1  blk-mq-dma: restore BLK_STS_TARGET (block; stable, v6.17)
  2  md: keep REQ_NOMERGE on P2PDMA bios
  3  md/raid1: serialize non-write-behind writes on CollisionCheck
     rdevs (pre-existing bug patch 4 would widen; stable)
  4  md/raid1: no write-behind for P2PDMA bios
  5  md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error()
     retry clones
  6  md/raid1: skip futile retries on P2PDMA mapping failures
  7  md/raid10: same
  8  nvme-rdma: preserve the DMA errno, return BLK_STS_TARGET
     (stable, v7.1)

Routing: patch 1 block, 2-7 md, 8 nvme; independently applicable
(patch 8 takes the errno from dma_map_sgtable() in rdma.c, not
from patch 1).

The patches were developed with AI assistance (see the Assisted-by
trailers); all code was human-reviewed and tested (result tables in
the v2 cover, linked below).

Against v7.2-rc2; merges clean onto current master.

Changes in v3:
 - split the raid1,raid10 mapping-failure patch per personality and
   folded its completion-path checks into one branch (Logan)
 - much shorter commit messages, comments and cover (Christoph,
   Keith, Leon); repro description added (Christoph)
 - picked up Logan's Reviewed-by on 1, 2, 4 and 5; since his review
   only a code comment in 1 and 2 was shortened (no code change)

Changes in v2: new serialization fix (patch 3), submission-time
state bit, metadata scatterlist, WantReplacement dropped for
mapping failures -- details in the v2 cover.

Link: https://lore.kernel.org/linux-raid/20260718162547.448892-1-mykola@meshstor.io/ [v1]
Link: https://lore.kernel.org/linux-raid/20260719105327.864949-1-mykola@meshstor.io/ [v2]

Mykola Marzhan (8):
  blk-mq-dma: restore BLK_STS_TARGET for unsupported P2P transfers
  md: ensure REQ_NOMERGE is set on P2PDMA bios
  md/raid1: serialize non-write-behind writes on CollisionCheck rdevs
  md/raid1: don't use write-behind for P2PDMA bios
  md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones
  md/raid1: skip futile retries on P2PDMA mapping failures
  md/raid10: skip futile retries on P2PDMA mapping failures
  nvme-rdma: return BLK_STS_TARGET for unsupported P2P transfers

 block/blk-mq-dma.c       |  6 +++-
 drivers/md/md.c          | 10 ++++--
 drivers/md/md.h          | 15 +++++++++
 drivers/md/raid1.c       | 73 ++++++++++++++++++++++++++++++----------
 drivers/md/raid1.h       |  2 ++
 drivers/md/raid10.c      | 66 +++++++++++++++++++++++++++---------
 drivers/md/raid10.h      |  2 ++
 drivers/nvme/host/rdma.c | 38 ++++++++++++---------
 8 files changed, 160 insertions(+), 52 deletions(-)


base-commit: 8cdeaa50eae8dad34885515f62559ee83e7e8dda
-- 
2.52.0

[PATCH v3 1/8] blk-mq-dma: restore BLK_STS_TARGET for unsupported P2P transfers

[Mykola Marzhan]

Unsupported P2P transfers used to fail with BLK_STS_TARGET, chosen
by commit 91fb2b6052f7 ("nvme-pci: convert to using
dma_map_sgtable()") for dma_map_sgtable()'s -EREMOTEIO: an I/O that
can never succeed on this device must not be retried.  The
blk_rq_dma_map conversion changed it to BLK_STS_INVAL, which
blk_path_error() treats as retryable and md deliberately ignores
for member failures -- a P2P write to a member the peer cannot
reach is counted as written and the mirrors silently diverge.

Restore BLK_STS_TARGET.  Hit e.g. with CMB memory of one NVMe
device used as the data buffer for I/O to a second one behind a
different host bridge.

Fixes: 858299dc6160 ("block: add scatterlist-less DMA mapping helpers")
Fixes: 7ce3c1dd78fc ("nvme-pci: convert the data mapping to blk_rq_dma_map")
Cc: stable@vger.kernel.org # v6.17
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
 block/blk-mq-dma.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/block/blk-mq-dma.c b/block/blk-mq-dma.c
index bfdb9ed70741..c17e4c49900c 100644
--- a/block/blk-mq-dma.c
+++ b/block/blk-mq-dma.c
@@ -190,7 +190,11 @@ static bool blk_dma_map_iter_start(struct request *req, struct device *dma_dev,
 	case PCI_P2PDMA_MAP_NONE:
 		break;
 	default:
-		iter->status = BLK_STS_INVAL;
+		/*
+		 * Match dma_map_sgtable()'s -EREMOTEIO: this transfer
+		 * can never succeed, so don't let it be retried.
+		 */
+		iter->status = BLK_STS_TARGET;
 		return false;
 	}
 
-- 
2.52.0

Re: [PATCH v3 1/8] blk-mq-dma: restore BLK_STS_TARGET for unsupported P2P transfers

[sashiko-bot]

> Unsupported P2P transfers used to fail with BLK_STS_TARGET, chosen
> by commit 91fb2b6052f7 ("nvme-pci: convert to using
> dma_map_sgtable()") for dma_map_sgtable()'s -EREMOTEIO: an I/O that
> can never succeed on this device must not be retried.  The
> blk_rq_dma_map conversion changed it to BLK_STS_INVAL, which
> blk_path_error() treats as retryable and md deliberately ignores
> for member failures -- a P2P write to a member the peer cannot
> reach is counted as written and the mirrors silently diverge.
> 
> Restore BLK_STS_TARGET.  Hit e.g. with CMB memory of one NVMe
> device used as the data buffer for I/O to a second one behind a
> different host bridge.
> 
> Fixes: 858299dc6160 ("block: add scatterlist-less DMA mapping helpers")
> Fixes: 7ce3c1dd78fc ("nvme-pci: convert the data mapping to blk_rq_dma_map")
> Cc: stable@vger.kernel.org # v6.17
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Mykola Marzhan <mykola@meshstor.io>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721174502.111503-1-mykola@meshstor.io?part=1

[PATCH v3 2/8] md: ensure REQ_NOMERGE is set on P2PDMA bios

[Mykola Marzhan]

md_submit_bio() strips REQ_NOMERGE from every bio.  For P2PDMA bios
the flag is what keeps requests single-provider (see
__bio_add_page()): merging a P2PDMA bio with one over a different
pgmap, or over host memory, maps the merged request with the wrong
bus address.  Set the flag on P2PDMA bios -- bios built via
bio_iov_bvec_set() arrive without it -- and keep stripping it
otherwise.

Fixes: 02666132403a ("md: propagate BLK_FEAT_PCI_P2PDMA from member devices to RAID device")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/md/md.c | 10 ++++++++--
 drivers/md/md.h | 15 +++++++++++++++
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index d1465bcd86c8..3770bdb4d4b2 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -451,8 +451,14 @@ static void md_submit_bio(struct bio *bio)
 		return;
 	}
 
-	/* bio could be mergeable after passing to underlayer */
-	bio->bi_opf &= ~REQ_NOMERGE;
+	/*
+	 * A bio md split may merge again below md -- except P2PDMA bios,
+	 * which must stay single-provider (see __bio_add_page()).
+	 */
+	if (md_bio_is_p2pdma(bio))
+		bio->bi_opf |= REQ_NOMERGE;
+	else
+		bio->bi_opf &= ~REQ_NOMERGE;
 
 	md_handle_request(mddev, bio);
 }
diff --git a/drivers/md/md.h b/drivers/md/md.h
index d8daf0f75cbb..73df1a6eccd8 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -11,8 +11,10 @@
 #include <linux/blkdev.h>
 #include <linux/backing-dev.h>
 #include <linux/badblocks.h>
+#include <linux/bio.h>
 #include <linux/kobject.h>
 #include <linux/list.h>
+#include <linux/memremap.h>
 #include <linux/mm.h>
 #include <linux/mutex.h>
 #include <linux/timer.h>
@@ -22,6 +24,19 @@
 #include <trace/events/block.h>
 
 #define MaxSector (~(sector_t)0)
+
+/*
+ * P2P and host pages never mix within a bio, so the first bvec is
+ * representative.  Read bi_io_vec directly: bio_first_bvec_all()
+ * WARNs on the split clones md handles, and data-less bios have no
+ * bi_io_vec.  Not valid after the bio's iterator is consumed.
+ */
+static inline bool md_bio_is_p2pdma(struct bio *bio)
+{
+	return bio_has_data(bio) && bio->bi_io_vec &&
+	       is_pci_p2pdma_page(bio->bi_io_vec->bv_page);
+}
+
 /*
  * Number of guaranteed raid bios in case of extreme VM load:
  */
-- 
2.52.0

Re: [PATCH v3 2/8] md: ensure REQ_NOMERGE is set on P2PDMA bios

[sashiko-bot]

> md_submit_bio() strips REQ_NOMERGE from every bio.  For P2PDMA bios
> the flag is what keeps requests single-provider (see
> __bio_add_page()): merging a P2PDMA bio with one over a different
> pgmap, or over host memory, maps the merged request with the wrong
> bus address.  Set the flag on P2PDMA bios -- bios built via
> bio_iov_bvec_set() arrive without it -- and keep stripping it
> otherwise.
> 
> Fixes: 02666132403a ("md: propagate BLK_FEAT_PCI_P2PDMA from member devices to RAID device")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Mykola Marzhan <mykola@meshstor.io>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721174502.111503-1-mykola@meshstor.io?part=2

[PATCH v3 3/8] md/raid1: serialize non-write-behind writes on CollisionCheck rdevs

[Mykola Marzhan]

A write that skips write-behind (full behind queue, a waiting
reader, failed behind-bio allocation) races the in-flight behind
write it overlaps: nothing orders the two on the write-mostly
member, and if the older behind data lands last the member keeps
stale data for sectors already acknowledged as rewritten.

Serialize on CollisionCheck, which marks exactly the rdevs owning a
serial tree: serialize_policy rdevs plus write-mostly members when
write-behind arms serialization -- the case the MD_SERIALIZE_POLICY
test misses.  remove_serial() under the same condition.  This also
avoids the old gate's latent NULL deref for rdevs hot-added under
serialize_policy (they never get a serial tree).

Fixes: d0d2d8ba0494 ("md/raid1: introduce wait_for_serialization")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-fable-5
Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
---
 drivers/md/raid1.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index afe2ca96ad8c..997f79c05db7 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -564,7 +564,7 @@ static void raid1_end_write_request(struct bio *bio)
 				call_bio_endio(r1_bio);
 			}
 		}
-	} else if (test_bit(MD_SERIALIZE_POLICY, &rdev->mddev->flags))
+	} else if (test_bit(CollisionCheck, &rdev->flags))
 		remove_serial(rdev, lo, hi);
 	if (r1_bio->bios[mirror] == NULL)
 		rdev_dec_pending(rdev, conf->mddev);
@@ -1677,7 +1677,11 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
 			mbio = bio_alloc_clone(rdev->bdev, bio, GFP_NOIO,
 					       &mddev->bio_set);
 
-			if (test_bit(MD_SERIALIZE_POLICY, &mddev->flags))
+			/*
+			 * CollisionCheck marks every rdev with a serial
+			 * tree; order against in-flight write-behind I/O.
+			 */
+			if (test_bit(CollisionCheck, &rdev->flags))
 				wait_for_serialization(rdev, r1_bio);
 		}
 
-- 
2.52.0

Re: [PATCH v3 3/8] md/raid1: serialize non-write-behind writes on CollisionCheck rdevs

[sashiko-bot]

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

Pre-existing issues:
- [High] Using CollisionCheck to gate serialization for normal writes exposes a pre-existing serial_info_pool allocation failure, causing continuous WARN_ON log spam on the hot path.
- [High] Hot-added devices silently bypass write serialization under MD_SERIALIZE_POLICY, risking data corruption.
--

--- Patch [3]: [PATCH v3 3/8] md/raid1: serialize non-write-behind writes on CollisionCheck rdevs ---
commit d02939f61be1abbb8081faa2ba5a3540f9714678
Author: Mykola Marzhan <mykola@meshstor.io>

md/raid1: serialize non-write-behind writes on CollisionCheck rdevs

This commit modifies the write path to serialize non-write-behind writes
based on the CollisionCheck flag rather than MD_SERIALIZE_POLICY. This
ensures they are properly ordered against in-flight behind writes. It also
updates the completion path to use the same condition for removing
serialization.

> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index afe2ca96ad8c2..997f79c05db7b 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -564,7 +564,7 @@ static void raid1_end_write_request(struct bio *bio)
>  				call_bio_endio(r1_bio);
>  			}
>  		}
> -	} else if (test_bit(MD_SERIALIZE_POLICY, &rdev->mddev->flags))
> +	} else if (test_bit(CollisionCheck, &rdev->flags))
>  		remove_serial(rdev, lo, hi);

[Severity: High]
This isn't a bug introduced by this patch, but changing the gate to
CollisionCheck exposes the normal write path to a pre-existing error handling
bug that results in continuous WARN_ON spam on the hot path.

If mempool allocation fails during a hot-add of a WriteMostly device,
mddev_create_serial_pool() calls rdevs_uninit_serial(). However,
rdevs_uninit_serial() only clears CollisionCheck on devices already in
mddev->disks, leaving the new device with CollisionCheck set but a NULL
serial_info_pool.

Before this patch, normal writes checked MD_SERIALIZE_POLICY and ignored
CollisionCheck, avoiding the bug. Now, does this cause every normal write to
call wait_for_serialization() (triggering the first WARN_ON) and then
remove_serial() (triggering a second WARN(1) since the IO was never recorded)?

>  	if (r1_bio->bios[mirror] == NULL)
>  		rdev_dec_pending(rdev, conf->mddev);

[ ... ]

> @@ -1677,7 +1677,11 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
>  			mbio = bio_alloc_clone(rdev->bdev, bio, GFP_NOIO,
>  					       &mddev->bio_set);
>  
> -			if (test_bit(MD_SERIALIZE_POLICY, &mddev->flags))
> +			/*
> +			 * CollisionCheck marks every rdev with a serial
> +			 * tree; order against in-flight write-behind I/O.
> +			 */
> +			if (test_bit(CollisionCheck, &rdev->flags))
>  				wait_for_serialization(rdev, r1_bio);

[Severity: High]
This is a pre-existing issue, but does checking CollisionCheck instead of
MD_SERIALIZE_POLICY silently bypass write serialization for hot-added devices
under MD_SERIALIZE_POLICY?

If MD_SERIALIZE_POLICY is enabled and a normal device is hot-added,
mddev_create_serial_pool() currently lacks a check for MD_SERIALIZE_POLICY
and returns early without creating a serial tree or setting CollisionCheck:

drivers/md/md.c:mddev_create_serial_pool() {
    ...
    if (rdev && !rdev_need_serial(rdev) &&
        !test_bit(CollisionCheck, &rdev->flags))
        return;
    ...
}

Previously, checking MD_SERIALIZE_POLICY here caused a NULL pointer
dereference when trying to serialize the hot-added device without a serial
tree. By checking CollisionCheck instead, the crash is avoided, but does
this mean overlapping writes can now complete out of order on the
reconstructed mirror, leading to silent data corruption?

>  		}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721174502.111503-1-mykola@meshstor.io?part=3

[PATCH v3 4/8] md/raid1: don't use write-behind for P2PDMA bios

[Mykola Marzhan]

alloc_behind_master_bio() copies the bio's data with bio_copy_data(),
a CPU copy. P2PDMA pages are peer device (BAR) memory; generic code
must not assume CPU load/store access to them is safe or fast on
every architecture, and bouncing peer memory through the CPU defeats
the point of a peer-to-peer transfer.

Skip write-behind for P2PDMA bios: they are written directly to all
members, including write-mostly ones.  Ordering against write-behind
I/O in flight to overlapping sectors is preserved: the non-behind
clone path serializes on CollisionCheck rdevs (see the preceding
fix), which covers these bios like any other write that bypasses
write-behind.

Fixes: 02666132403a ("md: propagate BLK_FEAT_PCI_P2PDMA from member devices to RAID device")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Mykola Marzhan <mykola@meshstor.io>

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/md/raid1.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 997f79c05db7..7d769b6460ca 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1523,6 +1523,7 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
 	bool write_behind = false;
 	bool nowait = bio->bi_opf & REQ_NOWAIT;
 	bool is_discard = op_is_discard(bio->bi_opf);
+	bool is_p2pdma = md_bio_is_p2pdma(bio);
 	sector_t sector = bio->bi_iter.bi_sector;
 
 	if (mddev_is_clustered(mddev) &&
@@ -1575,9 +1576,12 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
 		/*
 		 * The write-behind io is only attempted on drives marked as
 		 * write-mostly, which means we could allocate write behind
-		 * bio later.
+		 * bio later. P2PDMA bios are excluded: write-behind copies
+		 * the data with bio_copy_data(), a CPU copy that cannot be
+		 * assumed safe or fast on P2PDMA (device BAR) pages.
 		 */
-		if (!is_discard && rdev && test_bit(WriteMostly, &rdev->flags))
+		if (!is_discard && !is_p2pdma && rdev &&
+		    test_bit(WriteMostly, &rdev->flags))
 			write_behind = true;
 
 		r1_bio->bios[i] = NULL;
-- 
2.52.0

Re: [PATCH v3 4/8] md/raid1: don't use write-behind for P2PDMA bios

[sashiko-bot]

> alloc_behind_master_bio() copies the bio's data with bio_copy_data(),
> a CPU copy. P2PDMA pages are peer device (BAR) memory; generic code
> must not assume CPU load/store access to them is safe or fast on
> every architecture, and bouncing peer memory through the CPU defeats
> the point of a peer-to-peer transfer.
> 
> Skip write-behind for P2PDMA bios: they are written directly to all
> members, including write-mostly ones.  Ordering against write-behind
> I/O in flight to overlapping sectors is preserved: the non-behind
> clone path serializes on CollisionCheck rdevs (see the preceding
> fix), which covers these bios like any other write that bypasses
> write-behind.
> 
> Fixes: 02666132403a ("md: propagate BLK_FEAT_PCI_P2PDMA from member devices to RAID device")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Mykola Marzhan <mykola@meshstor.io>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721174502.111503-1-mykola@meshstor.io?part=4

[PATCH v3 5/8] md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones

[Mykola Marzhan]

narrow_write_error() re-issues a failed write in badblock-granularity
chunks, cloning from the master bio and resetting bi_opf to a bare
REQ_OP_WRITE. For a P2PDMA bio that reset drops REQ_NOMERGE, which is
the only request-level protection against the member queue merging
P2PDMA segments across pgmaps or with host memory (see the preceding
md_submit_bio() fix): the retry path would quietly reopen the hole
the submission path closes. Restore the flag on P2PDMA retry clones.

Fixes: 02666132403a ("md: propagate BLK_FEAT_PCI_P2PDMA from member devices to RAID device")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Mykola Marzhan <mykola@meshstor.io>

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/md/raid1.c  | 3 +++
 drivers/md/raid10.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 7d769b6460ca..16b7465c233a 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2570,6 +2570,9 @@ static void narrow_write_error(struct r1bio *r1_bio, int i)
 		}
 
 		wbio->bi_opf = REQ_OP_WRITE;
+		/* 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 0a3cfdd3f5df..f7ef903a3d4e 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -2831,6 +2831,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;
+		/* 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)) {
-- 
2.52.0

Re: [PATCH v3 5/8] md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones

[sashiko-bot]

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

Pre-existing issues:
- [High] The patch addresses the loss of the REQ_NOMERGE flag by explicitly checking for P2PDMA, but fails to fix the root cause: the hardcoded assignment wbio->bi_opf = REQ_OP_WRITE wipes all critical durability and behavioral flags like REQ_FUA and REQ_SYNC.
--

--- Patch [5]: [PATCH v3 5/8] md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones ---
commit 02b26b8d91beb002faa116497233b6b12f8d369f
Author: Mykola Marzhan <mykola@meshstor.io>

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

This commit restores the REQ_NOMERGE flag on P2PDMA bios during bad block
isolation retries in narrow_write_error(). The flag is needed to prevent
member queue merging of P2PDMA segments after the bio operation is reset
to a bare REQ_OP_WRITE.

> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 7d769b6460ca7..16b7465c233ac 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
[ ... ]
> @@ -2570,6 +2570,9 @@ static void narrow_write_error(struct r1bio *r1_bio, int i)
>  		}
>  
>  		wbio->bi_opf = REQ_OP_WRITE;
> +		/* Keep P2PDMA retry bios unmergeable, like the original */
> +		if (md_bio_is_p2pdma(wbio))
> +			wbio->bi_opf |= REQ_NOMERGE;

[Severity: High]
This is a pre-existing issue, but does hardcoding wbio->bi_opf to
REQ_OP_WRITE clear all other upper flag bits inherited from the master bio?

If the original write was synchronous and required Force Unit Access
(REQ_FUA), stripping it would cause the retry to be treated as a normal
write, which might be absorbed by the disk's volatile cache. This could
allow a retry of a physically defective block to erroneously succeed,
bypassing the bad block record and risking data loss if power is lost.

Would it be safer to mask the operation instead, preserving all durability
and behavioral flags, like:
wbio->bi_opf = (wbio->bi_opf & ~REQ_OP_MASK) | REQ_OP_WRITE;

This approach would naturally preserve REQ_NOMERGE as well, without needing
a P2PDMA-specific check.

>  		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 0a3cfdd3f5df8..f7ef903a3d4ec 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
[ ... ]
> @@ -2831,6 +2831,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;
> +		/* Keep P2PDMA retry bios unmergeable, like the original */
> +		if (md_bio_is_p2pdma(wbio))
> +			wbio->bi_opf |= REQ_NOMERGE;

[Severity: High]
This is a pre-existing issue, but the same logic flaw appears here in the
RAID10 bad block isolation path. Does overwriting bi_opf silently drop
important flags like REQ_FUA and REQ_SYNC?

>  
>  		if (submit_bio_wait(wbio) &&
>  		    !rdev_set_badblocks(rdev, wsector, sectors, 0)) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721174502.111503-1-mykola@meshstor.io?part=5

[PATCH v3 6/8] md/raid1: skip futile retries on P2PDMA mapping failures

[Mykola Marzhan]

A member that cannot DMA-map peer memory fails the leg bio with
BLK_STS_TARGET.  The failure is a property of the peer/member
pairing -- retrying the same pages against the same member cannot
succeed, and there is nothing on the medium to repair -- but the
error machinery treats it as a medium error: narrow_write_error()
grinds through hundreds of doomed chunk retries per write,
WantReplacement burns a spare and evicts the healthy member,
fix_read_error()'s host-page probe "succeeds" so mixed host/P2P
reads charge the read-error budget until a healthy member is
kicked (~1s in testing), and FailFast evicts on the first
unroutable I/O.

Flag P2PDMA master bios with a new R1BIO_P2PDMA state bit at
submission.  On a BLK_STS_TARGET leg failure for a flagged bio:
retry writes once as a single whole range, recording one bad range
if that also fails (TARGET can be a transient device condition,
e.g. NVME_SC_NS_NOT_READY, so the retry is not skipped outright);
don't set WantReplacement; mark failed read legs IO_BLOCKED without
charging the read-error budget; and don't treat the failure as
FailFast evidence -- the request never reached the wire -- as
commit f7b24c7b41f2 ("md/raid1,raid10: don't fail devices for
invalid IO errors") did for BLK_STS_INVAL.

Fixes: 02666132403a ("md: propagate BLK_FEAT_PCI_P2PDMA from member devices to RAID device")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
---
 drivers/md/raid1.c | 54 ++++++++++++++++++++++++++++++++++------------
 drivers/md/raid1.h |  2 ++
 2 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 16b7465c233a..6334032c4bb8 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -483,16 +483,22 @@ static void raid1_end_write_request(struct bio *bio)
 	 * 'one mirror IO has finished' event handler:
 	 */
 	if (bio->bi_status && !ignore_error) {
-		set_bit(WriteErrorSeen,	&rdev->flags);
-		if (!test_and_set_bit(WantReplacement, &rdev->flags))
-			set_bit(MD_RECOVERY_NEEDED, &
-				conf->mddev->recovery);
+		/* Peer/member pairing failure, not member health. */
+		bool p2pdma_unmappable = bio->bi_status == BLK_STS_TARGET &&
+			test_bit(R1BIO_P2PDMA, &r1_bio->state);
 
-		if (test_bit(FailFast, &rdev->flags) &&
-		    (bio->bi_opf & MD_FAILFAST) &&
-		    /* We never try FailFast to WriteMostly devices */
-		    !test_bit(WriteMostly, &rdev->flags)) {
-			md_error(r1_bio->mddev, rdev);
+		set_bit(WriteErrorSeen,	&rdev->flags);
+		if (!p2pdma_unmappable) {
+			if (!test_and_set_bit(WantReplacement, &rdev->flags))
+				set_bit(MD_RECOVERY_NEEDED,
+					&conf->mddev->recovery);
+
+			if (test_bit(FailFast, &rdev->flags) &&
+			    (bio->bi_opf & MD_FAILFAST) &&
+			    /* We never try FailFast to WriteMostly devices */
+			    !test_bit(WriteMostly, &rdev->flags)) {
+				md_error(r1_bio->mddev, rdev);
+			}
 		}
 
 		/*
@@ -1378,6 +1384,8 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
 	else
 		init_r1bio(r1_bio, mddev, bio);
 	r1_bio->sectors = max_read_sectors;
+	if (md_bio_is_p2pdma(bio))
+		set_bit(R1BIO_P2PDMA, &r1_bio->state);
 
 	/*
 	 * make_request() can abort the operation when read-ahead is being
@@ -1557,6 +1565,8 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
 
 	r1_bio = alloc_r1bio(mddev, bio);
 	r1_bio->sectors = max_sectors;
+	if (md_bio_is_p2pdma(bio))
+		set_bit(R1BIO_P2PDMA, &r1_bio->state);
 
 	/* first select target devices under rcu_lock and
 	 * inc refcount on their rdev.  Record them by setting
@@ -2522,7 +2532,7 @@ static void fix_read_error(struct r1conf *conf, struct r1bio *r1_bio)
 	}
 }
 
-static void narrow_write_error(struct r1bio *r1_bio, int i)
+static void narrow_write_error(struct r1bio *r1_bio, int i, bool coarse)
 {
 	struct mddev *mddev = r1_bio->mddev;
 	struct r1conf *conf = mddev->private;
@@ -2536,6 +2546,9 @@ static void narrow_write_error(struct r1bio *r1_bio, int i)
 	 * It is conceivable that the bio doesn't exactly align with
 	 * blocks.  We must handle this somehow.
 	 *
+	 * With 'coarse', retry the whole range as one bio: P2PDMA
+	 * mapping failures fail every block identically.
+	 *
 	 * We currently own a reference on the rdev.
 	 */
 
@@ -2550,9 +2563,12 @@ static void narrow_write_error(struct r1bio *r1_bio, int i)
 		block_sectors = roundup(1 << rdev->badblocks.shift, lbs);
 
 	sector = r1_bio->sector;
-	sectors = ((sector + block_sectors)
-		   & ~(sector_t)(block_sectors - 1))
-		- sector;
+	if (coarse)
+		sectors = sect_to_write;
+	else
+		sectors = ((sector + block_sectors)
+			   & ~(sector_t)(block_sectors - 1))
+			- sector;
 
 	while (sect_to_write) {
 		struct bio *wbio;
@@ -2633,8 +2649,12 @@ static void handle_write_finished(struct r1conf *conf, struct r1bio *r1_bio)
 			 * narrow down and record precise write
 			 * errors.
 			 */
+			bool coarse = r1_bio->bios[m]->bi_status ==
+					BLK_STS_TARGET &&
+				test_bit(R1BIO_P2PDMA, &r1_bio->state);
+
 			fail = true;
-			narrow_write_error(r1_bio, m);
+			narrow_write_error(r1_bio, m, coarse);
 			rdev_dec_pending(conf->mirrors[m].rdev,
 					 conf->mddev);
 		}
@@ -2661,6 +2681,9 @@ 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_TARGET &&
+		test_bit(R1BIO_P2PDMA, &r1_bio->state);
 	struct mddev *mddev = conf->mddev;
 	sector_t sector;
 
@@ -2680,6 +2703,9 @@ static void handle_read_error(struct r1conf *conf, struct r1bio *r1_bio)
 	 */
 	if (mddev->ro) {
 		r1_bio->bios[r1_bio->read_disk] = IO_BLOCKED;
+	} else if (p2pdma_error) {
+		/* Peer can't reach this member: just redirect the read. */
+		r1_bio->bios[r1_bio->read_disk] = IO_BLOCKED;
 	} else if (test_bit(FailFast, &rdev->flags)) {
 		md_error(mddev, rdev);
 	} else {
diff --git a/drivers/md/raid1.h b/drivers/md/raid1.h
index c98d43a7ae99..61b788a99d14 100644
--- a/drivers/md/raid1.h
+++ b/drivers/md/raid1.h
@@ -184,6 +184,8 @@ enum r1bio_state {
 	R1BIO_MadeGood,
 	R1BIO_WriteError,
 	R1BIO_FailFast,
+/* the master bio carries PCI P2PDMA (peer device memory) pages */
+	R1BIO_P2PDMA,
 };
 
 static inline int sector_to_idx(sector_t sector)
-- 
2.52.0

Re: [PATCH v3 6/8] md/raid1: skip futile retries on P2PDMA mapping failures

[sashiko-bot]

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

New issues:
- [High] Coarse retries for persistent P2PDMA mapping failures exhaust the array's badblock list, forcing healthy device eviction.

Pre-existing issues:
- [High] Data corruption and kernel panics result from wbio->bi_opf = REQ_OP_WRITE; stripping critical flags and modifying bodiless bios.
- [High] A NULL pointer dereference occurs on rdev->bdev during concurrent read errors and hot-removes.
--

--- Patch [6]: [PATCH v3 6/8] md/raid1: skip futile retries on P2PDMA mapping failures ---
commit 808452a00e434c54e8f26591e906bbf52dd9e7f0
Author: Mykola Marzhan <mykola@meshstor.io>

md/raid1: skip futile retries on P2PDMA mapping failures

A member that cannot DMA-map peer memory fails the leg bio with
BLK_STS_TARGET.  The failure is a property of the peer/member
pairing -- retrying the same pages against the same member cannot
succeed, and there is nothing on the medium to repair. This patch
flags P2PDMA master bios with a new R1BIO_P2PDMA state bit at
submission to avoid evicting healthy members during unroutable I/O.

> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 16b7465c233ac..6334032c4bb8e 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
[ ... ]
> @@ -1378,6 +1384,8 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
>  	else
>  		init_r1bio(r1_bio, mddev, bio);
>  	r1_bio->sectors = max_read_sectors;
> +	if (md_bio_is_p2pdma(bio))
> +		set_bit(R1BIO_P2PDMA, &r1_bio->state);
>  
>  	/*
>  	 * make_request() can abort the operation when read-ahead is being

[Severity: High]
This isn't a bug introduced by this patch, but could this lead to a NULL
pointer dereference?

When a read fails, handle_read_error() drops the reference to the rdev via
rdev_dec_pending(), which may trigger device removal if it's faulty. It then
calls raid1_read_request() to retry.

In raid1_read_request(), if read_balance() returns -1 (no other disks are
available), we hit a pr_crit_ratelimited statement:

    rdisk = read_balance(conf, r1_bio, &max_sectors);
    if (rdisk < 0) {
        /* couldn't find anywhere to read from */
        if (r1bio_existed)
            pr_crit_ratelimited("md/raid1:%s: %pg: unrecoverable I/O read error for block %llu\n",
                        mdname(mddev),
                        conf->mirrors[r1_bio->read_disk].rdev->bdev,
                        r1_bio->sector);

Is it possible that conf->mirrors[r1_bio->read_disk].rdev is NULL here due
to a concurrent hot-remove, causing a crash when dereferencing ->bdev?

[ ... ]
> @@ -2550,9 +2563,12 @@ static void narrow_write_error(struct r1bio *r1_bio, int i)
>  		block_sectors = roundup(1 << rdev->badblocks.shift, lbs);
>  
>  	sector = r1_bio->sector;
> -	sectors = ((sector + block_sectors)
> -		   & ~(sector_t)(block_sectors - 1))
> -		- sector;
> +	if (coarse)
> +		sectors = sect_to_write;
> +	else
> +		sectors = ((sector + block_sectors)
> +			   & ~(sector_t)(block_sectors - 1))
> +			- sector;
>  
>  	while (sect_to_write) {
>  		struct bio *wbio;

[Severity: High]
This is a pre-existing issue, but does overwriting the operation entirely
strip critical upper durability flags here?

In narrow_write_error(), the cloned wbio is modified like this:

        wbio->bi_opf = REQ_OP_WRITE;

Could this strip flags like REQ_FUA or REQ_PREFLUSH, leading to potential
data loss on power failure? Furthermore, if a bodiless bio like
REQ_OP_WRITE_ZEROES is converted into a regular REQ_OP_WRITE, could this
cause a NULL pointer dereference in the block layer during segment mapping
(e.g., bio_for_each_segment) since it has no data vectors?

[Severity: High]
With the new coarse retry mechanism, what happens when persistent P2PDMA
mapping failures repeatedly occur?

If a P2PDMA write fails with BLK_STS_TARGET, we enter narrow_write_error()
with coarse = true. Later in this loop, if the retried coarse I/O also
fails, it adds a badblock:

        if (submit_bio_wait(wbio) &&
            !rdev_set_badblocks(rdev, sector, sectors, 0)) {

Since P2PDMA pairing failures are a persistent property of the peer/member
combination, repeated coarse retries will keep adding large badblock entries.

If this quickly exhausts the array's finite badblocks list, won't
rdev_set_badblocks() return 0, triggering a call to md_error() that forces
the eviction of a healthy device? Does this defeat the patch's purpose of
preventing healthy devices from being evicted?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721174502.111503-1-mykola@meshstor.io?part=6

[PATCH v3 7/8] md/raid10: skip futile retries on P2PDMA mapping failures

[Mykola Marzhan]

Same handling as the preceding raid1 commit: flag P2PDMA master
bios with R10BIO_P2PDMA at submission; on a BLK_STS_TARGET leg
failure retry writes once as a whole range, don't set
WantReplacement, block the leg for reads without charging the
read-error budget, don't trip FailFast.

Replacement legs keep the stock fail-on-write-error policy:
badblocks are never recorded on a replacement, so failing it is
the only outcome that cannot leave a silent hole in a rebuild.

Fixes: 02666132403a ("md: propagate BLK_FEAT_PCI_P2PDMA from member devices to RAID device")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
---
 drivers/md/raid10.c | 63 ++++++++++++++++++++++++++++++++++-----------
 drivers/md/raid10.h |  2 ++
 2 files changed, 50 insertions(+), 15 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index f7ef903a3d4e..4c3da50250a5 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -482,15 +482,23 @@ static void raid10_end_write_request(struct bio *bio)
 			 */
 			md_error(rdev->mddev, rdev);
 		else {
-			set_bit(WriteErrorSeen,	&rdev->flags);
-			if (!test_and_set_bit(WantReplacement, &rdev->flags))
-				set_bit(MD_RECOVERY_NEEDED,
-					&rdev->mddev->recovery);
+			/* Peer/member pairing failure, not member health. */
+			bool p2pdma_unmappable =
+				bio->bi_status == BLK_STS_TARGET &&
+				test_bit(R10BIO_P2PDMA, &r10_bio->state);
 
+			set_bit(WriteErrorSeen,	&rdev->flags);
 			dec_rdev = 0;
-			if (test_bit(FailFast, &rdev->flags) &&
-			    (bio->bi_opf & MD_FAILFAST)) {
-				md_error(rdev->mddev, rdev);
+			if (!p2pdma_unmappable) {
+				if (!test_and_set_bit(WantReplacement,
+						      &rdev->flags))
+					set_bit(MD_RECOVERY_NEEDED,
+						&rdev->mddev->recovery);
+
+				if (test_bit(FailFast, &rdev->flags) &&
+				    (bio->bi_opf & MD_FAILFAST)) {
+					md_error(rdev->mddev, rdev);
+				}
 			}
 
 			/*
@@ -1170,6 +1178,9 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
 	 */
 	gfp_t gfp = err_path ? (GFP_NOIO | __GFP_HIGH) : GFP_NOIO;
 
+	if (md_bio_is_p2pdma(bio))
+		set_bit(R10BIO_P2PDMA, &r10_bio->state);
+
 	if (slot >= 0 && r10_bio->devs[slot].rdev) {
 		/*
 		 * This is an error retry, but we cannot
@@ -1357,6 +1368,9 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
 	sector_t sectors;
 	int max_sectors;
 
+	if (md_bio_is_p2pdma(bio))
+		set_bit(R10BIO_P2PDMA, &r10_bio->state);
+
 	if ((mddev_is_clustered(mddev) &&
 	     mddev->cluster_ops->area_resyncing(mddev, WRITE,
 						bio->bi_iter.bi_sector,
@@ -2786,7 +2800,7 @@ static void fix_read_error(struct r10conf *conf, struct mddev *mddev, struct r10
 	}
 }
 
-static void narrow_write_error(struct r10bio *r10_bio, int i)
+static void narrow_write_error(struct r10bio *r10_bio, int i, bool coarse)
 {
 	struct bio *bio = r10_bio->master_bio;
 	struct mddev *mddev = r10_bio->mddev;
@@ -2800,6 +2814,9 @@ static void narrow_write_error(struct r10bio *r10_bio, int i)
 	 * It is conceivable that the bio doesn't exactly align with
 	 * blocks.  We must handle this.
 	 *
+	 * With 'coarse', retry the whole range as one bio: P2PDMA
+	 * mapping failures fail every block identically.
+	 *
 	 * We currently own a reference to the rdev.
 	 */
 
@@ -2814,9 +2831,12 @@ static void narrow_write_error(struct r10bio *r10_bio, int i)
 		block_sectors = roundup(1 << rdev->badblocks.shift, lbs);
 
 	sector = r10_bio->sector;
-	sectors = ((r10_bio->sector + block_sectors)
-		   & ~(sector_t)(block_sectors - 1))
-		- sector;
+	if (coarse)
+		sectors = sect_to_write;
+	else
+		sectors = ((r10_bio->sector + block_sectors)
+			   & ~(sector_t)(block_sectors - 1))
+			- sector;
 
 	while (sect_to_write) {
 		struct bio *wbio;
@@ -2856,6 +2876,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;
 
@@ -2868,17 +2889,24 @@ 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_TARGET &&
+		test_bit(R10BIO_P2PDMA, &r10_bio->state);
 	bio_put(bio);
 	r10_bio->devs[slot].bio = NULL;
 
 	if (mddev->ro)
 		r10_bio->devs[slot].bio = IO_BLOCKED;
-	else if (!test_bit(FailFast, &rdev->flags)) {
+	else if (p2pdma_error) {
+		/* Peer can't reach this member: just redirect the read. */
+		r10_bio->devs[slot].bio = IO_BLOCKED;
+	} else if (test_bit(FailFast, &rdev->flags)) {
+		md_error(mddev, rdev);
+	} else {
 		freeze_array(conf, 1);
 		fix_read_error(conf, mddev, r10_bio);
 		unfreeze_array(conf);
-	} else
-		md_error(mddev, rdev);
+	}
 
 	rdev_dec_pending(rdev, mddev);
 	r10_bio->state = 0;
@@ -2947,8 +2975,13 @@ static void handle_write_completed(struct r10conf *conf, struct r10bio *r10_bio)
 					r10_bio->sectors, 0);
 				rdev_dec_pending(rdev, conf->mddev);
 			} else if (bio != NULL && bio->bi_status) {
+				bool coarse = bio->bi_status ==
+						BLK_STS_TARGET &&
+					test_bit(R10BIO_P2PDMA,
+						 &r10_bio->state);
+
 				fail = true;
-				narrow_write_error(r10_bio, m);
+				narrow_write_error(r10_bio, m, coarse);
 				rdev_dec_pending(rdev, conf->mddev);
 			}
 			bio = r10_bio->devs[m].repl_bio;
diff --git a/drivers/md/raid10.h b/drivers/md/raid10.h
index ec79d87fb92f..a2e1554f77db 100644
--- a/drivers/md/raid10.h
+++ b/drivers/md/raid10.h
@@ -174,6 +174,8 @@ enum r10bio_state {
 	R10BIO_Previous,
 /* failfast devices did receive failfast requests. */
 	R10BIO_FailFast,
+/* the master bio carries PCI P2PDMA (peer device memory) pages */
+	R10BIO_P2PDMA,
 	R10BIO_Discard,
 };
 #endif
-- 
2.52.0

Re: [PATCH v3 7/8] md/raid10: skip futile retries on P2PDMA mapping failures

[sashiko-bot]

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

Pre-existing issues:
- [High] narrow_write_error() drops critical bio flags (e.g., REQ_FUA, REQ_PREFLUSH) during write error retries.
- [High] fix_read_error() incorrectly attributes read errors on a replacement device to the primary device.
--

--- Patch [7]: [PATCH v3 7/8] md/raid10: skip futile retries on P2PDMA mapping failures ---
commit 850a2ebdf179522973aa4c7332fa7233fdc47fc1
Author: Mykola Marzhan <mykola@meshstor.io>
Subject: md/raid10: skip futile retries on P2PDMA mapping failures

This commit handles P2PDMA mapping failures by skipping futile retries
and retrying the write once as a whole range. It also leaves replacement
legs with the default fail-on-write-error policy to avoid silent holes
during rebuilds.

Fixes: 02666132403a ("md: propagate BLK_FEAT_PCI_P2PDMA from member devices to RAID device")

> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index f7ef903a3d4ec..4c3da50250a5a 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
[ ... ]
> @@ -2814,9 +2831,12 @@ static void narrow_write_error(struct r10bio *r10_bio, int i)
>  		block_sectors = roundup(1 << rdev->badblocks.shift, lbs);
>  
>  	sector = r10_bio->sector;
> -	sectors = ((r10_bio->sector + block_sectors)
> -		   & ~(sector_t)(block_sectors - 1))
> -		- sector;
> +	if (coarse)
> +		sectors = sect_to_write;
> +	else
> +		sectors = ((r10_bio->sector + block_sectors)
> +			   & ~(sector_t)(block_sectors - 1))
> +			- sector;
>  
>  	while (sect_to_write) {
>  		struct bio *wbio;

[Severity: High]
This is a pre-existing issue, but does narrow_write_error() drop critical
bio flags like REQ_FUA and REQ_PREFLUSH here?

Further down in narrow_write_error(), the clone's operation flags are
overwritten with a bare write:

drivers/md/raid10.c:narrow_write_error() {
    ...
    wbio = bio_alloc_clone(rdev->bdev, bio, GFP_NOIO, &mddev->bio_set);
    bio_trim(wbio, sector - bio->bi_iter.bi_sector, sectors);
    ...
    wbio->bi_opf = REQ_OP_WRITE;
    ...
}

If the master bio was submitted with REQ_FUA to guarantee data persistence,
these flags are stripped and the retry is issued as a normal asynchronous
write.

If the retry succeeds by placing data in the drive's volatile cache, the
master bio completes successfully. Could a power failure before the cache
is flushed to stable media result in silent data loss, violating the
original REQ_FUA guarantees?

(Note: The identical logic appears to be present in drivers/md/raid1.c in
narrow_write_error() as well).

[ ... ]
> @@ -2856,6 +2876,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;
>  
> @@ -2868,17 +2889,24 @@ 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_TARGET &&
> +		test_bit(R10BIO_P2PDMA, &r10_bio->state);
>  	bio_put(bio);
>  	r10_bio->devs[slot].bio = NULL;
>  
>  	if (mddev->ro)
>  		r10_bio->devs[slot].bio = IO_BLOCKED;
> -	else if (!test_bit(FailFast, &rdev->flags)) {
> +	else if (p2pdma_error) {
> +		/* Peer can't reach this member: just redirect the read. */
> +		r10_bio->devs[slot].bio = IO_BLOCKED;
> +	} else if (test_bit(FailFast, &rdev->flags)) {
> +		md_error(mddev, rdev);
> +	} else {
>  		freeze_array(conf, 1);
>  		fix_read_error(conf, mddev, r10_bio);
>  		unfreeze_array(conf);
> -	} else
> -		md_error(mddev, rdev);
> +	}
>  
>  	rdev_dec_pending(rdev, mddev);
>  	r10_bio->state = 0;

[Severity: High]
This is a pre-existing issue, but does fix_read_error() incorrectly attribute
read errors on a replacement device to the primary device?

When read_balance() selects the replacement device for a read and it fails,
handle_read_error() passes the context to fix_read_error().

However, fix_read_error() ignores the actual device that failed
(r10_bio->devs[slot].rdev) and blindly selects the primary device:

drivers/md/raid10.c:fix_read_error() {
    ...
    /* still own a reference to this rdev, so it cannot
     * have been cleared recently.
     */
    rdev = conf->mirrors[d].rdev;

    if (test_bit(Faulty, &rdev->flags))
    ...
}

Later, bad blocks are also recorded on the primary device:

drivers/md/raid10.c:fix_read_error() {
    ...
    int dn = r10_bio->devs[slot].devnum;
    rdev = conf->mirrors[dn].rdev;

    if (!rdev_set_badblocks(
                rdev,
                r10_bio->devs[slot].addr
                + sect,
                s, 0)) {
    ...
}

Could this result in the healthy primary device being penalized via
exceed_read_errors() and incorrectly targeted for bad block recording,
while the faulty replacement device continues to operate without
correction?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721174502.111503-1-mykola@meshstor.io?part=7

[PATCH v3 8/8] nvme-rdma: return BLK_STS_TARGET for unsupported P2P transfers

[Mykola Marzhan]

ib_dma_map_sg() returns 0 on a P2P mapping failure, losing the DMA
layer's -EREMOTEIO, so nvme-rdma reports a path error.
Default multipath requeues the I/O forever, and without multipath
it burns nvme_max_retries requeues, then completes as retryable
BLK_STS_TRANSPORT.

Map the data and metadata scatterlists with
ib_dma_map_sgtable_attrs() and return BLK_STS_TARGET for -EREMOTEIO,
matching nvme-pci (commit 91fb2b6052f7 ("nvme-pci: convert to using
dma_map_sgtable()")).
Scatterlists stay: ib_map_mr_sg() consumes them, so blk_rq_dma_map
does not apply.

Start the request only after mapping succeeds so multipath
accounting is never taken for an unmapped request, and ratelimit
the map-failure message.

Fixes: 23528aa3320a ("nvme: enable PCI P2PDMA support for RDMA transport")
Cc: stable@vger.kernel.org # v7.1
Assisted-by: Claude:claude-fable-5
Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
---
 drivers/nvme/host/rdma.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 6909e3542794..f8edb23552ad 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -1469,6 +1469,7 @@ static int nvme_rdma_dma_map_req(struct ib_device *ibdev, struct request *rq,
 		int *count, int *pi_count)
 {
 	struct nvme_rdma_request *req = blk_mq_rq_to_pdu(rq);
+	struct sg_table sgt;
 	int ret;
 
 	req->data_sgl.sg_table.sgl = (struct scatterlist *)(req + 1);
@@ -1480,12 +1481,14 @@ static int nvme_rdma_dma_map_req(struct ib_device *ibdev, struct request *rq,
 
 	req->data_sgl.nents = blk_rq_map_sg(rq, req->data_sgl.sg_table.sgl);
 
-	*count = ib_dma_map_sg(ibdev, req->data_sgl.sg_table.sgl,
-			       req->data_sgl.nents, rq_dma_dir(rq));
-	if (unlikely(*count <= 0)) {
-		ret = -EIO;
+	sgt = (struct sg_table) {
+		.sgl		= req->data_sgl.sg_table.sgl,
+		.orig_nents	= req->data_sgl.nents,
+	};
+	ret = ib_dma_map_sgtable_attrs(ibdev, &sgt, rq_dma_dir(rq), 0);
+	if (unlikely(ret))
 		goto out_free_table;
-	}
+	*count = sgt.nents;
 
 	if (blk_integrity_rq(rq)) {
 		req->metadata_sgl->sg_table.sgl =
@@ -1501,14 +1504,14 @@ static int nvme_rdma_dma_map_req(struct ib_device *ibdev, struct request *rq,
 
 		req->metadata_sgl->nents = blk_rq_map_integrity_sg(rq,
 				req->metadata_sgl->sg_table.sgl);
-		*pi_count = ib_dma_map_sg(ibdev,
-					  req->metadata_sgl->sg_table.sgl,
-					  req->metadata_sgl->nents,
-					  rq_dma_dir(rq));
-		if (unlikely(*pi_count <= 0)) {
-			ret = -EIO;
+		sgt = (struct sg_table) {
+			.sgl		= req->metadata_sgl->sg_table.sgl,
+			.orig_nents	= req->metadata_sgl->nents,
+		};
+		ret = ib_dma_map_sgtable_attrs(ibdev, &sgt, rq_dma_dir(rq), 0);
+		if (unlikely(ret))
 			goto out_free_pi_table;
-		}
+		*pi_count = sgt.nents;
 	}
 
 	return 0;
@@ -2026,8 +2029,6 @@ static blk_status_t nvme_rdma_queue_rq(struct blk_mq_hw_ctx *hctx,
 	if (ret)
 		goto unmap_qe;
 
-	nvme_start_request(rq);
-
 	if (IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY) &&
 	    queue->pi_support &&
 	    (c->common.opcode == nvme_cmd_write ||
@@ -2039,11 +2040,13 @@ static blk_status_t nvme_rdma_queue_rq(struct blk_mq_hw_ctx *hctx,
 
 	err = nvme_rdma_map_data(queue, rq, c);
 	if (unlikely(err < 0)) {
-		dev_err(queue->ctrl->ctrl.device,
-			     "Failed to map data (%d)\n", err);
+		dev_err_ratelimited(queue->ctrl->ctrl.device,
+				    "Failed to map data (%d)\n", err);
 		goto err;
 	}
 
+	nvme_start_request(rq);
+
 	sqe->cqe.done = nvme_rdma_send_done;
 
 	ib_dma_sync_single_for_device(dev, sqe->dma,
@@ -2063,6 +2066,9 @@ static blk_status_t nvme_rdma_queue_rq(struct blk_mq_hw_ctx *hctx,
 		ret = nvme_host_path_error(rq);
 	else if (err == -ENOMEM || err == -EAGAIN)
 		ret = BLK_STS_RESOURCE;
+	/* Peer memory unreachable from this device: don't retry. */
+	else if (err == -EREMOTEIO)
+		ret = BLK_STS_TARGET;
 	else
 		ret = BLK_STS_IOERR;
 	nvme_cleanup_cmd(rq);
-- 
2.52.0

Re: [PATCH v3 8/8] nvme-rdma: return BLK_STS_TARGET for unsupported P2P transfers

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Calling `nvme_cleanup_cmd(rq)` and unmapping the command after `nvme_host_path_error(rq)` causes a Use-After-Free.
- [Medium] Multipath accounting and I/O stats leak when `nvme_rdma_post_send` fails with unhandled errors.
--

--- Patch [8]: [PATCH v3 8/8] nvme-rdma: return BLK_STS_TARGET for unsupported P2P transfers ---
commit 2c3b755710303fa594bcae5bdde333ab1b8dc356
Author: Mykola Marzhan <mykola@meshstor.io>

nvme-rdma: return BLK_STS_TARGET for unsupported P2P transfers

This commit updates nvme-rdma to map scatterlists using
ib_dma_map_sgtable_attrs() to properly capture -EREMOTEIO. It returns
BLK_STS_TARGET for this error to avoid infinite requeuing of unsupported
P2P transfers. Additionally, it delays starting the request until after
mapping succeeds to avoid taking multipath accounting for unmapped requests.

Cc: stable@vger.kernel.org # v7.1

> diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
> index 6909e35427942..f8edb23552ad2 100644
> --- a/drivers/nvme/host/rdma.c
> +++ b/drivers/nvme/host/rdma.c
[ ... ]
> @@ -2063,6 +2066,9 @@ static blk_status_t nvme_rdma_queue_rq(struct blk_mq_hw_ctx *hctx,
>  		ret = nvme_host_path_error(rq);

[Severity: Critical]
This is a pre-existing issue, but does this error path cause a use-after-free
if err == -EIO?

Looking at nvme_host_path_error(), it synchronously completes and frees the
request back to the blk-mq tag pool:

drivers/nvme/host/core.c:nvme_host_path_error() {
	nvme_req(req)->status = NVME_SC_HOST_PATH_ERROR;
	blk_mq_set_request_complete(req);
	__nvme_complete_rq(req);
	return BLK_STS_OK;
}

After it returns, the code falls through to nvme_cleanup_cmd(rq) and
unmaps the sqe.dma:

unmap_qe:
	ib_dma_unmap_single(dev, req->sqe.dma, sizeof(struct nvme_command),
			    DMA_TO_DEVICE);

Could a concurrent thread allocate this freed request and map a new DMA
address before we unmap it here, leading to an IOMMU fault or corruption?

>  	else if (err == -ENOMEM || err == -EAGAIN)
>  		ret = BLK_STS_RESOURCE;
> +	/* Peer memory unreachable from this device: don't retry. */
> +	else if (err == -EREMOTEIO)
> +		ret = BLK_STS_TARGET;
>  	else
>  		ret = BLK_STS_IOERR;

[Severity: Medium]
This is a pre-existing issue, but does returning BLK_STS_IOERR for unhandled
errors bypass NVMe multipath cleanup?

If nvme_rdma_post_send() fails with an unexpected error code (like -EINVAL),
we fall through here and return BLK_STS_IOERR. The block layer will directly
complete the request via blk_mq_end_request().

Since nvme_start_request(rq) was called earlier and set NVME_MPATH_CNT_ACTIVE,
does bypassing the NVMe-specific nvme_mpath_end_request() cause multipath
accounting to leak (ns->ctrl->nr_active is never decremented)?

>  	nvme_cleanup_cmd(rq);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721174502.111503-1-mykola@meshstor.io?part=8