* [PATCH v4 1/9] block: add BLK_STS_P2PDMA for unsupported peer-to-peer transfers
2026-07-22 18:58 [PATCH v4 0/9] block,md,nvme: correct handling of unsupported P2PDMA transfers Mykola Marzhan
@ 2026-07-22 18:58 ` Mykola Marzhan
2026-07-22 19:06 ` sashiko-bot
2026-07-22 18:58 ` [PATCH v4 2/9] md: ensure REQ_NOMERGE is set on P2PDMA bios Mykola Marzhan
` (7 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Mykola Marzhan @ 2026-07-22 18:58 UTC (permalink / raw)
To: Jens Axboe, Song Liu, Yu Kuai, Keith Busch, Christoph Hellwig,
Sagi Grimberg, linux-block, linux-raid, linux-nvme
Cc: Li Nan, Xiao Ni, Guoqing Jiang, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Logan Gunthorpe,
Bjorn Helgaas, Shivaji Kant, Pranjal Shrivastava,
Henrique Carvalho, linux-kernel, linux-rdma, linux-pci
From: Logan Gunthorpe <logang@deltatee.com>
blk_dma_map_iter_start() reports BLK_STS_INVAL when a P2PDMA transfer
is attempted between two devices whose PCIe topology cannot route it.
blk_path_error() treats INVAL as retryable, so multipath requeues the
I/O forever, and callers cannot tell it apart from an invalid request.
md also ignores it for member failures, so a mirror silently diverges.
Restoring the old BLK_STS_TARGET would stay ambiguous -- nvme maps
NS_NOT_READY, LBA_RANGE and CMD_INTERRUPTED to it too. Add a dedicated
BLK_STS_P2PDMA, non-retryable in blk_path_error(), and return it from
blk_dma_map_iter_start().
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
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
---
block/blk-core.c | 3 +++
block/blk-mq-dma.c | 3 ++-
include/linux/blk_types.h | 10 ++++++++++
3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 365641266c9e..83cb5464265f 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -177,6 +177,9 @@ static const struct {
/* Command duration limit device-side timeout */
ENT(DURATION_LIMIT, -ETIME, "duration limit exceeded"),
+
+ /* no PCIe P2PDMA route between initiator and target */
+ ENT(P2PDMA, -EREMOTEIO, "peer-to-peer transfer unroutable"),
ENT(INVAL, -EINVAL, "invalid"),
/* everything else not covered above: */
diff --git a/block/blk-mq-dma.c b/block/blk-mq-dma.c
index bfdb9ed70741..e88d6b3524e6 100644
--- a/block/blk-mq-dma.c
+++ b/block/blk-mq-dma.c
@@ -189,8 +189,9 @@ static bool blk_dma_map_iter_start(struct request *req, struct device *dma_dev,
*/
case PCI_P2PDMA_MAP_NONE:
break;
+ case PCI_P2PDMA_MAP_NOT_SUPPORTED:
default:
- iter->status = BLK_STS_INVAL;
+ iter->status = BLK_STS_P2PDMA;
return false;
}
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 8808ee76e73c..c8de7809320a 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -165,6 +165,15 @@ typedef u16 blk_short_t;
*/
#define BLK_STS_DURATION_LIMIT ((__force blk_status_t)17)
+/*
+ * BLK_STS_P2PDMA is returned when a peer-to-peer DMA transfer cannot be
+ * mapped (typically because there is no route between the initiator and the
+ * target that the platform's PCIe topology and ACS settings allow). This
+ * is a property of the initiator/target pair, not of the target device's
+ * health. This error should never be retried.
+ */
+#define BLK_STS_P2PDMA ((__force blk_status_t)18)
+
/*
* Invalid size or alignment.
*/
@@ -188,6 +197,7 @@ static inline bool blk_path_error(blk_status_t error)
case BLK_STS_NOTSUPP:
case BLK_STS_NOSPC:
case BLK_STS_TARGET:
+ case BLK_STS_P2PDMA:
case BLK_STS_RESV_CONFLICT:
case BLK_STS_MEDIUM:
case BLK_STS_PROTECTION:
--
2.52.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v4 1/9] block: add BLK_STS_P2PDMA for unsupported peer-to-peer transfers
2026-07-22 18:58 ` [PATCH v4 1/9] block: add BLK_STS_P2PDMA for unsupported peer-to-peer transfers Mykola Marzhan
@ 2026-07-22 19:06 ` sashiko-bot
0 siblings, 0 replies; 23+ messages in thread
From: sashiko-bot @ 2026-07-22 19:06 UTC (permalink / raw)
To: Mykola Marzhan; +Cc: linux-raid, yukuai, linux-pci
> From: Logan Gunthorpe <logang@deltatee.com>
>
> blk_dma_map_iter_start() reports BLK_STS_INVAL when a P2PDMA transfer
> is attempted between two devices whose PCIe topology cannot route it.
> blk_path_error() treats INVAL as retryable, so multipath requeues the
> I/O forever, and callers cannot tell it apart from an invalid request.
> md also ignores it for member failures, so a mirror silently diverges.
>
> Restoring the old BLK_STS_TARGET would stay ambiguous -- nvme maps
> NS_NOT_READY, LBA_RANGE and CMD_INTERRUPTED to it too. Add a dedicated
> BLK_STS_P2PDMA, non-retryable in blk_path_error(), and return it from
> blk_dma_map_iter_start().
>
> 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
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> 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/20260722185841.449934-1-mykola@meshstor.io?part=1
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 2/9] md: ensure REQ_NOMERGE is set on P2PDMA bios
2026-07-22 18:58 [PATCH v4 0/9] block,md,nvme: correct handling of unsupported P2PDMA transfers Mykola Marzhan
2026-07-22 18:58 ` [PATCH v4 1/9] block: add BLK_STS_P2PDMA for unsupported peer-to-peer transfers Mykola Marzhan
@ 2026-07-22 18:58 ` Mykola Marzhan
2026-07-22 19:07 ` sashiko-bot
2026-07-22 18:58 ` [PATCH v4 3/9] md/raid1: serialize non-write-behind writes on CollisionCheck rdevs Mykola Marzhan
` (6 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Mykola Marzhan @ 2026-07-22 18:58 UTC (permalink / raw)
To: Jens Axboe, Song Liu, Yu Kuai, Keith Busch, Christoph Hellwig,
Sagi Grimberg, linux-block, linux-raid, linux-nvme
Cc: Li Nan, Xiao Ni, Guoqing Jiang, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Logan Gunthorpe,
Bjorn Helgaas, Shivaji Kant, Pranjal Shrivastava,
Henrique Carvalho, linux-kernel, linux-rdma, linux-pci
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v4 3/9] md/raid1: serialize non-write-behind writes on CollisionCheck rdevs
2026-07-22 18:58 [PATCH v4 0/9] block,md,nvme: correct handling of unsupported P2PDMA transfers Mykola Marzhan
2026-07-22 18:58 ` [PATCH v4 1/9] block: add BLK_STS_P2PDMA for unsupported peer-to-peer transfers Mykola Marzhan
2026-07-22 18:58 ` [PATCH v4 2/9] md: ensure REQ_NOMERGE is set on P2PDMA bios Mykola Marzhan
@ 2026-07-22 18:58 ` Mykola Marzhan
2026-07-22 19:14 ` sashiko-bot
2026-07-22 18:58 ` [PATCH v4 4/9] md/raid1: don't use write-behind for P2PDMA bios Mykola Marzhan
` (5 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Mykola Marzhan @ 2026-07-22 18:58 UTC (permalink / raw)
To: Jens Axboe, Song Liu, Yu Kuai, Keith Busch, Christoph Hellwig,
Sagi Grimberg, linux-block, linux-raid, linux-nvme
Cc: Li Nan, Xiao Ni, Guoqing Jiang, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Logan Gunthorpe,
Bjorn Helgaas, Shivaji Kant, Pranjal Shrivastava,
Henrique Carvalho, linux-kernel, linux-rdma, linux-pci
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v4 3/9] md/raid1: serialize non-write-behind writes on CollisionCheck rdevs
2026-07-22 18:58 ` [PATCH v4 3/9] md/raid1: serialize non-write-behind writes on CollisionCheck rdevs Mykola Marzhan
@ 2026-07-22 19:14 ` sashiko-bot
0 siblings, 0 replies; 23+ messages in thread
From: sashiko-bot @ 2026-07-22 19:14 UTC (permalink / raw)
To: Mykola Marzhan; +Cc: linux-pci, yukuai, linux-raid
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Medium] Hot-added devices silently bypass the MD_SERIALIZE_POLICY serialization.
Pre-existing issues:
- [Critical] Concurrent overlapping I/Os cause RB-tree double-insertion and corruption in `wait_for_serialization` and `remove_serial`.
- [High] Toggling `-writemostly` or `serialize_policy` via sysfs races with in-flight I/Os, causing a Use-After-Free of `rdev->serial` and leaking `serial_info`.
--
--- Patch [3]: [PATCH v4 3/9] md/raid1: serialize non-write-behind writes on CollisionCheck rdevs ---
commit cb65d8bde9fd6c1299852f1cafb33bef4da080a2
Author: Mykola Marzhan <mykola@meshstor.io>
md/raid1: serialize non-write-behind writes on CollisionCheck rdevs
This commit changes the write serialization gating from the global
MD_SERIALIZE_POLICY flag to the per-device CollisionCheck flag. It fixes
a race with write-behind I/O on write-mostly members and avoids a latent
NULL dereference for hot-added devices.
Fixes: d0d2d8ba0494 ("md/raid1: introduce wait_for_serialization")
Cc: stable@vger.kernel.org
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> --- 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 is a pre-existing issue, but does toggling -writemostly or
serialize_policy via sysfs race with in-flight I/Os here?
When mddev_suspend() is called during reconfiguration, it waits for new I/O
submissions to finish, but it does not wait for in-flight cloned bios on
the underlying disks to complete.
If mddev_destroy_serial_pool() clears CollisionCheck and frees rdev->serial
concurrently, and an in-flight bio completes, raid1_end_write_request()
might read CollisionCheck as true and dereference the freed rdev->serial
inside remove_serial().
If it reads the flag as false, it skips remove_serial(), leaking the
serial_info mempool element.
> 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: Medium]
Does this change allow hot-added devices to completely bypass serialization
when the global serialize policy is active?
If an administrator sets serialize_policy=1 and a new device is hot-added,
mddev_create_serial_pool() skips initializing the serial tree for the new
device because it checks rdev_need_serial() and CollisionCheck (which is
not set yet).
Since this gate in raid1_write_request() now only checks CollisionCheck
instead of MD_SERIALIZE_POLICY, the hot-added device skips serialization
entirely.
[Severity: Critical]
This isn't a bug introduced by this patch, but can concurrent overlapping
I/Os cause an RB-tree double-insertion during wait_for_serialization()?
If three overlapping I/Os (A, B, C) arrive in sequence (e.g. A=[25-35],
B=[20-30], C=[10-20]):
I/O A is inserted. I/O B overlaps A, and gets added to A's wait list.
I/O C overlaps B but not A, so C is inserted.
When A completes, it removes itself, inserts B into the tree, and wakes B.
B wakes up and calls check_and_add_serial(), which finds C. B is added to
C's wait list and sleeps again, but remains linked in the RB-tree.
When C completes, it removes itself, splices B's waiters, and calls
raid1_rb_insert() on B again, corrupting the tree structure.
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260722185841.449934-1-mykola@meshstor.io?part=3
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 4/9] md/raid1: don't use write-behind for P2PDMA bios
2026-07-22 18:58 [PATCH v4 0/9] block,md,nvme: correct handling of unsupported P2PDMA transfers Mykola Marzhan
` (2 preceding siblings ...)
2026-07-22 18:58 ` [PATCH v4 3/9] md/raid1: serialize non-write-behind writes on CollisionCheck rdevs Mykola Marzhan
@ 2026-07-22 18:58 ` Mykola Marzhan
2026-07-22 19:11 ` sashiko-bot
2026-07-22 18:58 ` [PATCH v4 5/9] md/raid1,raid10: factor out raid1_write_error() helper Mykola Marzhan
` (4 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Mykola Marzhan @ 2026-07-22 18:58 UTC (permalink / raw)
To: Jens Axboe, Song Liu, Yu Kuai, Keith Busch, Christoph Hellwig,
Sagi Grimberg, linux-block, linux-raid, linux-nvme
Cc: Li Nan, Xiao Ni, Guoqing Jiang, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Logan Gunthorpe,
Bjorn Helgaas, Shivaji Kant, Pranjal Shrivastava,
Henrique Carvalho, linux-kernel, linux-rdma, linux-pci
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v4 4/9] md/raid1: don't use write-behind for P2PDMA bios
2026-07-22 18:58 ` [PATCH v4 4/9] md/raid1: don't use write-behind for P2PDMA bios Mykola Marzhan
@ 2026-07-22 19:11 ` sashiko-bot
0 siblings, 0 replies; 23+ messages in thread
From: sashiko-bot @ 2026-07-22 19:11 UTC (permalink / raw)
To: Mykola Marzhan; +Cc: linux-raid, yukuai, linux-pci
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] raid1_write_request() can block on resource allocations and serialization when processing a REQ_NOWAIT bio, violating non-blocking guarantees.
--
--- Patch [4]: [PATCH v4 4/9] md/raid1: don't use write-behind for P2PDMA bios ---
commit 1a1224f9f3e6ee3f02e62a2f1a4ff02688f39f0d
Author: Mykola Marzhan <mykola@meshstor.io>
md/raid1: don't use write-behind for P2PDMA bios
This commit skips write-behind for P2PDMA bios as they are written directly
to all members, including write-mostly ones. A CPU copy via bio_copy_data()
defeats the point of a peer-to-peer transfer.
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 997f79c05db7b..7d769b6460ca7 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;
[Severity: High]
This is a pre-existing issue, but does raid1_write_request() properly support
REQ_NOWAIT semantics without blocking?
Earlier in raid1_write_request(), we allocate r1_bio unconditionally using
GFP_NOIO, which can sleep under memory pressure:
r1_bio = alloc_r1bio(mddev, bio);
alloc_r1bio() {
...
r1_bio = mempool_alloc(conf->r1bio_pool, GFP_NOIO);
...
}
Later down in raid1_write_request(), if CollisionCheck is set, we can
unconditionally put the thread to sleep, ignoring the REQ_NOWAIT context:
if (test_bit(CollisionCheck, &rdev->flags))
wait_for_serialization(rdev, r1_bio);
wait_for_serialization() {
...
si = mempool_alloc(mddev->serial_info_pool, GFP_NOIO);
...
wait_for_completion(&si->ready);
}
Could a REQ_NOWAIT write bio submitted to an md raid1 array block under
memory pressure or in-flight I/O overlapping, causing thread starvation in
io_uring workers?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260722185841.449934-1-mykola@meshstor.io?part=4
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 5/9] md/raid1,raid10: factor out raid1_write_error() helper
2026-07-22 18:58 [PATCH v4 0/9] block,md,nvme: correct handling of unsupported P2PDMA transfers Mykola Marzhan
` (3 preceding siblings ...)
2026-07-22 18:58 ` [PATCH v4 4/9] md/raid1: don't use write-behind for P2PDMA bios Mykola Marzhan
@ 2026-07-22 18:58 ` Mykola Marzhan
2026-07-22 19:05 ` sashiko-bot
2026-07-22 18:58 ` [PATCH v4 6/9] md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones Mykola Marzhan
` (3 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Mykola Marzhan @ 2026-07-22 18:58 UTC (permalink / raw)
To: Jens Axboe, Song Liu, Yu Kuai, Keith Busch, Christoph Hellwig,
Sagi Grimberg, linux-block, linux-raid, linux-nvme
Cc: Li Nan, Xiao Ni, Guoqing Jiang, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Logan Gunthorpe,
Bjorn Helgaas, Shivaji Kant, Pranjal Shrivastava,
Henrique Carvalho, linux-kernel, linux-rdma, linux-pci
From: Logan Gunthorpe <logang@deltatee.com>
Factor out a common raid1_write_error() helper from raid1.c and
raid10.c.
This will help reduce code duplication in subsequent patches.
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
---
drivers/md/raid1-10.c | 13 +++++++++++++
drivers/md/raid1.c | 13 ++-----------
drivers/md/raid10.c | 10 +---------
3 files changed, 16 insertions(+), 20 deletions(-)
diff --git a/drivers/md/raid1-10.c b/drivers/md/raid1-10.c
index 56a56a4da4f8..cee21452e066 100644
--- a/drivers/md/raid1-10.c
+++ b/drivers/md/raid1-10.c
@@ -303,3 +303,16 @@ static inline bool raid1_should_handle_error(struct bio *bio)
return !(bio->bi_opf & (REQ_RAHEAD | REQ_NOWAIT)) &&
bio->bi_status != BLK_STS_INVAL;
}
+
+static inline void raid1_write_error(struct mddev *mddev, struct md_rdev *rdev,
+ struct bio *bio, bool allow_failfast)
+{
+ set_bit(WriteErrorSeen, &rdev->flags);
+
+ if (!test_and_set_bit(WantReplacement, &rdev->flags))
+ set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
+
+ if (allow_failfast && test_bit(FailFast, &rdev->flags) &&
+ (bio->bi_opf & MD_FAILFAST))
+ md_error(mddev, rdev);
+}
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 7d769b6460ca..f07edeac6e5a 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -483,17 +483,8 @@ 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);
-
- 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);
- }
+ raid1_write_error(conf->mddev, rdev, bio,
+ !test_bit(WriteMostly, &rdev->flags));
/*
* When the device is faulty, it is not necessary to
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 0a3cfdd3f5df..5b63175fb382 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -482,16 +482,8 @@ 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);
-
dec_rdev = 0;
- if (test_bit(FailFast, &rdev->flags) &&
- (bio->bi_opf & MD_FAILFAST)) {
- md_error(rdev->mddev, rdev);
- }
+ raid1_write_error(rdev->mddev, rdev, bio, true);
/*
* When the device is faulty, it is not necessary to
--
2.52.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v4 6/9] md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones
2026-07-22 18:58 [PATCH v4 0/9] block,md,nvme: correct handling of unsupported P2PDMA transfers Mykola Marzhan
` (4 preceding siblings ...)
2026-07-22 18:58 ` [PATCH v4 5/9] md/raid1,raid10: factor out raid1_write_error() helper Mykola Marzhan
@ 2026-07-22 18:58 ` Mykola Marzhan
2026-07-22 19:16 ` sashiko-bot
2026-07-22 18:58 ` [PATCH v4 7/9] md/raid1,raid10: skip futile retries on P2PDMA mapping failures Mykola Marzhan
` (2 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Mykola Marzhan @ 2026-07-22 18:58 UTC (permalink / raw)
To: Jens Axboe, Song Liu, Yu Kuai, Keith Busch, Christoph Hellwig,
Sagi Grimberg, linux-block, linux-raid, linux-nvme
Cc: Li Nan, Xiao Ni, Guoqing Jiang, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Logan Gunthorpe,
Bjorn Helgaas, Shivaji Kant, Pranjal Shrivastava,
Henrique Carvalho, linux-kernel, linux-rdma, linux-pci
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 f07edeac6e5a..19c4dec450de 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;
+ /* 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 5b63175fb382..428bddddfe71 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;
+ /* 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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v4 6/9] md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones
2026-07-22 18:58 ` [PATCH v4 6/9] md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones Mykola Marzhan
@ 2026-07-22 19:16 ` sashiko-bot
0 siblings, 0 replies; 23+ messages in thread
From: sashiko-bot @ 2026-07-22 19:16 UTC (permalink / raw)
To: Mykola Marzhan; +Cc: linux-raid, linux-pci, yukuai
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] narrow_write_error() incorrectly coerces dataless operations into data-bearing REQ_OP_WRITE states, causing block layer crashes.
--
--- Patch [6]: [PATCH v4 6/9] md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones ---
commit 4017c1a8ef243dcaefd606ee7c22696e4aaccaa1
Author: Mykola Marzhan <mykola@meshstor.io>
md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones
This commit fixes a missing REQ_NOMERGE flag on P2PDMA retry clones in
narrow_write_error(). It ensures that member queues do not merge P2PDMA
segments incorrectly when a bio is retried after a write error.
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> --- 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;
> + /* 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 setting wbio->bi_opf unconditionally to
REQ_OP_WRITE here cause a NULL pointer dereference for dataless requests?
If the array is processing a REQ_OP_SECURE_ERASE request (which has bi_size > 0
but bi_io_vec is NULL) and hits a write error, narrow_write_error() will force
the operation to REQ_OP_WRITE. Because REQ_OP_WRITE expects a data vector, the
subsequent call to bio_trim() in this loop will attempt to advance a NULL
bi_io_vec during chunk iteration, resulting in a crash.
> 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
> --- 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;
> + /* 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 similar to the raid1.c case above, could
this coercion to REQ_OP_WRITE crash lower-level drivers?
If a REQ_OP_SECURE_ERASE request hits a write error and is retried here,
wbio->bi_opf is overridden to REQ_OP_WRITE before calling submit_bio_wait().
When this malformed write bio with a NULL bi_io_vec is sent to lower layers
(like NVMe or SCSI), it can cause a panic during DMA mapping since those
drivers expect a valid bvec for REQ_OP_WRITE.
>
> if (submit_bio_wait(wbio) &&
> !rdev_set_badblocks(rdev, wsector, sectors, 0)) {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260722185841.449934-1-mykola@meshstor.io?part=6
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 7/9] md/raid1,raid10: skip futile retries on P2PDMA mapping failures
2026-07-22 18:58 [PATCH v4 0/9] block,md,nvme: correct handling of unsupported P2PDMA transfers Mykola Marzhan
` (5 preceding siblings ...)
2026-07-22 18:58 ` [PATCH v4 6/9] md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones Mykola Marzhan
@ 2026-07-22 18:58 ` Mykola Marzhan
2026-07-22 19:10 ` sashiko-bot
2026-07-22 19:18 ` Logan Gunthorpe
2026-07-22 18:58 ` [PATCH v4 8/9] md/raid1,raid10: set IO_BLOCKED in case of BLK_STS_P2PDMA Mykola Marzhan
2026-07-22 18:58 ` [PATCH v4 9/9] nvme-rdma: return BLK_STS_P2PDMA for unsupported P2P transfers Mykola Marzhan
8 siblings, 2 replies; 23+ messages in thread
From: Mykola Marzhan @ 2026-07-22 18:58 UTC (permalink / raw)
To: Jens Axboe, Song Liu, Yu Kuai, Keith Busch, Christoph Hellwig,
Sagi Grimberg, linux-block, linux-raid, linux-nvme
Cc: Li Nan, Xiao Ni, Guoqing Jiang, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Logan Gunthorpe,
Bjorn Helgaas, Shivaji Kant, Pranjal Shrivastava,
Henrique Carvalho, linux-kernel, linux-rdma, linux-pci
From: Logan Gunthorpe <logang@deltatee.com>
BLK_STS_P2PDMA reflects the peer/member pairing, not member health:
retrying the same pages on the same member cannot succeed. Keep
WriteErrorSeen so the range stays fenced, but skip WantReplacement and
FailFast eviction, as commit f7b24c7b41f2 ("md/raid1,raid10: don't fail
devices for invalid IO errors") did for BLK_STS_INVAL.
The failure is whole-range and deterministic, so narrow_write_error()'s
block-by-block submit_bio_wait() retries are pointless. Record the
whole range as a bad block in the completion handler instead.
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@deltatee.com>
Co-developed-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
---
drivers/md/raid1-10.c | 3 +++
drivers/md/raid1.c | 12 +++++++++---
drivers/md/raid10.c | 8 +++++++-
3 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/md/raid1-10.c b/drivers/md/raid1-10.c
index cee21452e066..89daf8558251 100644
--- a/drivers/md/raid1-10.c
+++ b/drivers/md/raid1-10.c
@@ -309,6 +309,9 @@ static inline void raid1_write_error(struct mddev *mddev, struct md_rdev *rdev,
{
set_bit(WriteErrorSeen, &rdev->flags);
+ if (bio->bi_status == BLK_STS_P2PDMA)
+ return;
+
if (!test_and_set_bit(WantReplacement, &rdev->flags))
set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 19c4dec450de..917d694ef401 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2624,10 +2624,16 @@ static void handle_write_finished(struct r1conf *conf, struct r1bio *r1_bio)
* narrow down and record precise write
* errors.
*/
+ struct md_rdev *rdev = conf->mirrors[m].rdev;
+
fail = true;
- narrow_write_error(r1_bio, m);
- rdev_dec_pending(conf->mirrors[m].rdev,
- conf->mddev);
+ /* Mapping failures are whole-range and deterministic */
+ if (r1_bio->bios[m]->bi_status == BLK_STS_P2PDMA)
+ rdev_set_badblocks(rdev, r1_bio->sector,
+ r1_bio->sectors, 0);
+ else
+ narrow_write_error(r1_bio, m);
+ rdev_dec_pending(rdev, conf->mddev);
}
if (fail) {
spin_lock_irq(&conf->device_lock);
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 428bddddfe71..9045a3f02dae 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -2940,7 +2940,13 @@ static void handle_write_completed(struct r10conf *conf, struct r10bio *r10_bio)
rdev_dec_pending(rdev, conf->mddev);
} else if (bio != NULL && bio->bi_status) {
fail = true;
- narrow_write_error(r10_bio, m);
+ /* Mapping failures are whole-range and deterministic */
+ if (bio->bi_status == BLK_STS_P2PDMA)
+ rdev_set_badblocks(rdev,
+ r10_bio->devs[m].addr,
+ r10_bio->sectors, 0);
+ else
+ narrow_write_error(r10_bio, m);
rdev_dec_pending(rdev, conf->mddev);
}
bio = r10_bio->devs[m].repl_bio;
--
2.52.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v4 7/9] md/raid1,raid10: skip futile retries on P2PDMA mapping failures
2026-07-22 18:58 ` [PATCH v4 7/9] md/raid1,raid10: skip futile retries on P2PDMA mapping failures Mykola Marzhan
@ 2026-07-22 19:10 ` sashiko-bot
2026-07-22 19:18 ` Logan Gunthorpe
1 sibling, 0 replies; 23+ messages in thread
From: sashiko-bot @ 2026-07-22 19:10 UTC (permalink / raw)
To: Mykola Marzhan; +Cc: yukuai, linux-raid, linux-pci
> From: Logan Gunthorpe <logang@deltatee.com>
>
> BLK_STS_P2PDMA reflects the peer/member pairing, not member health:
> retrying the same pages on the same member cannot succeed. Keep
> WriteErrorSeen so the range stays fenced, but skip WantReplacement and
> FailFast eviction, as commit f7b24c7b41f2 ("md/raid1,raid10: don't fail
> devices for invalid IO errors") did for BLK_STS_INVAL.
>
> The failure is whole-range and deterministic, so narrow_write_error()'s
> block-by-block submit_bio_wait() retries are pointless. Record the
> whole range as a bad block in the completion handler instead.
>
> 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@deltatee.com>
> Co-developed-by: Mykola Marzhan <mykola@meshstor.io>
> 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/20260722185841.449934-1-mykola@meshstor.io?part=7
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v4 7/9] md/raid1,raid10: skip futile retries on P2PDMA mapping failures
2026-07-22 18:58 ` [PATCH v4 7/9] md/raid1,raid10: skip futile retries on P2PDMA mapping failures Mykola Marzhan
2026-07-22 19:10 ` sashiko-bot
@ 2026-07-22 19:18 ` Logan Gunthorpe
2026-07-22 19:50 ` Mykola Marzhan
1 sibling, 1 reply; 23+ messages in thread
From: Logan Gunthorpe @ 2026-07-22 19:18 UTC (permalink / raw)
To: Mykola Marzhan, Jens Axboe, Song Liu, Yu Kuai, Keith Busch,
Christoph Hellwig, Sagi Grimberg, linux-block, linux-raid,
linux-nvme
Cc: Li Nan, Xiao Ni, Guoqing Jiang, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Bjorn Helgaas,
Shivaji Kant, Pranjal Shrivastava, Henrique Carvalho,
linux-kernel, linux-rdma, linux-pci
On 2026-07-22 12:58, Mykola Marzhan wrote:
> From: Logan Gunthorpe <logang@deltatee.com>
>
> BLK_STS_P2PDMA reflects the peer/member pairing, not member health:
> retrying the same pages on the same member cannot succeed. Keep
> WriteErrorSeen so the range stays fenced, but skip WantReplacement and
> FailFast eviction, as commit f7b24c7b41f2 ("md/raid1,raid10: don't fail
> devices for invalid IO errors") did for BLK_STS_INVAL.
>
> The failure is whole-range and deterministic, so narrow_write_error()'s
> block-by-block submit_bio_wait() retries are pointless. Record the
> whole range as a bad block in the completion handler instead.
>
> 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@deltatee.com>
> Co-developed-by: Mykola Marzhan <mykola@meshstor.io>
> Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
> ---
> drivers/md/raid1-10.c | 3 +++
> drivers/md/raid1.c | 12 +++++++++---
> drivers/md/raid10.c | 8 +++++++-
> 3 files changed, 19 insertions(+), 4 deletions(-)
Maybe drop my authorship from this patch. I only did the first hunk and
don't fully understand the bulk of it.
> diff --git a/drivers/md/raid1-10.c b/drivers/md/raid1-10.c
> index cee21452e066..89daf8558251 100644
> --- a/drivers/md/raid1-10.c
> +++ b/drivers/md/raid1-10.c
> @@ -309,6 +309,9 @@ static inline void raid1_write_error(struct mddev *mddev, struct md_rdev *rdev,
> {
> set_bit(WriteErrorSeen, &rdev->flags);
>
> + if (bio->bi_status == BLK_STS_P2PDMA)
> + return;
> +
> if (!test_and_set_bit(WantReplacement, &rdev->flags))
> set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 19c4dec450de..917d694ef401 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -2624,10 +2624,16 @@ static void handle_write_finished(struct r1conf *conf, struct r1bio *r1_bio)
> * narrow down and record precise write
> * errors.
> */
> + struct md_rdev *rdev = conf->mirrors[m].rdev;
> +
> fail = true;
> - narrow_write_error(r1_bio, m);
> - rdev_dec_pending(conf->mirrors[m].rdev,
> - conf->mddev);
> + /* Mapping failures are whole-range and deterministic */
I'm not sure I follow the point of this new comment here.
Thanks,
Logan
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v4 7/9] md/raid1,raid10: skip futile retries on P2PDMA mapping failures
2026-07-22 19:18 ` Logan Gunthorpe
@ 2026-07-22 19:50 ` Mykola Marzhan
0 siblings, 0 replies; 23+ messages in thread
From: Mykola Marzhan @ 2026-07-22 19:50 UTC (permalink / raw)
To: Logan Gunthorpe
Cc: Jens Axboe, Song Liu, Yu Kuai, Keith Busch, Christoph Hellwig,
Sagi Grimberg, linux-block, linux-raid, linux-nvme, Li Nan,
Xiao Ni, Guoqing Jiang, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Bjorn Helgaas,
Shivaji Kant, Pranjal Shrivastava, Henrique Carvalho,
linux-kernel, linux-rdma, linux-pci
> Maybe drop my authorship from this patch.
I will do in v5. Any other changes needed?
> I'm not sure I follow the point of this new comment here.
I will drop the comment; it means the failure covers the whole bio and
won't change on retry, so we record whole range rather than per block.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 8/9] md/raid1,raid10: set IO_BLOCKED in case of BLK_STS_P2PDMA
2026-07-22 18:58 [PATCH v4 0/9] block,md,nvme: correct handling of unsupported P2PDMA transfers Mykola Marzhan
` (6 preceding siblings ...)
2026-07-22 18:58 ` [PATCH v4 7/9] md/raid1,raid10: skip futile retries on P2PDMA mapping failures Mykola Marzhan
@ 2026-07-22 18:58 ` Mykola Marzhan
2026-07-22 19:16 ` sashiko-bot
2026-07-22 19:19 ` Logan Gunthorpe
2026-07-22 18:58 ` [PATCH v4 9/9] nvme-rdma: return BLK_STS_P2PDMA for unsupported P2P transfers Mykola Marzhan
8 siblings, 2 replies; 23+ messages in thread
From: Mykola Marzhan @ 2026-07-22 18:58 UTC (permalink / raw)
To: Jens Axboe, Song Liu, Yu Kuai, Keith Busch, Christoph Hellwig,
Sagi Grimberg, linux-block, linux-raid, linux-nvme
Cc: Li Nan, Xiao Ni, Guoqing Jiang, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Logan Gunthorpe,
Bjorn Helgaas, Shivaji Kant, Pranjal Shrivastava,
Henrique Carvalho, linux-kernel, linux-rdma, linux-pci
From: Logan Gunthorpe <logang@deltatee.com>
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@deltatee.com>
Co-developed-by: Mykola Marzhan <mykola@meshstor.io>
Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
---
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;
bio_put(bio);
r10_bio->devs[slot].bio = NULL;
- if (mddev->ro)
+ if (mddev->ro || p2pdma_error)
r10_bio->devs[slot].bio = IO_BLOCKED;
else if (!test_bit(FailFast, &rdev->flags)) {
freeze_array(conf, 1);
--
2.52.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v4 8/9] md/raid1,raid10: set IO_BLOCKED in case of BLK_STS_P2PDMA
2026-07-22 18:58 ` [PATCH v4 8/9] md/raid1,raid10: set IO_BLOCKED in case of BLK_STS_P2PDMA Mykola Marzhan
@ 2026-07-22 19:16 ` sashiko-bot
2026-07-22 19:19 ` Logan Gunthorpe
1 sibling, 0 replies; 23+ messages in thread
From: sashiko-bot @ 2026-07-22 19:16 UTC (permalink / raw)
To: Mykola Marzhan; +Cc: linux-pci, yukuai, linux-raid
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [Critical] NULL pointer dereference when a read error occurs and the failing disk is concurrently removed.
- [High] Barrier count underflow leading to array deadlock for REQ_NOWAIT retries.
--
--- Patch [8]: [PATCH v4 8/9] md/raid1,raid10: set IO_BLOCKED in case of BLK_STS_P2PDMA ---
commit 64027366aa2bf452a9087c9fd113c84027594e8b
Author: Logan Gunthorpe <logang@deltatee.com>
md/raid1,raid10: set IO_BLOCKED in case of BLK_STS_P2PDMA
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. This skips the
read-error machinery, avoiding the eviction of healthy members under a P2P
read workload.
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 917d694ef401a..76a1426e64f02 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
[ ... ]
> @@ -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);
[Severity: Critical]
This isn't a bug introduced by this patch, but can we hit a NULL pointer
dereference if the failing disk is concurrently removed?
When a read error occurs, the bio is routed here to handle_read_error().
This function drops the pending count via rdev_dec_pending(rdev), which
could allow a concurrent mdadm --remove to succeed and set
conf->mirrors[r1_bio->read_disk].rdev to NULL.
The retry is then submitted via raid1_read_request(). If all healthy disks
are blocked or unavailable, read_balance() can return -1, triggering this
logging path in raid1_read_request():
raid1_read_request() {
...
if (rdisk < 0) {
...
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,
...);
...
}
Since rdev is now NULL, will this dereference rdev->bdev and panic the
kernel? For comparison, raid10_read_request() appears to correctly check
if rdev is NULL before logging in this scenario.
[Severity: High]
This is a pre-existing issue, but could this error path lead to a barrier
count underflow and array deadlock when retrying REQ_NOWAIT requests?
If a REQ_NOWAIT read fails and reaches handle_read_error(), it retries by
calling raid1_read_request() with the existing r1_bio.
Inside raid1_read_request(), wait_read_barrier() can return false due to
the nowait flag and the array being concurrently frozen, which leaves
nr_pending unchanged. However, raid1_read_request() then calls
raid_end_bio_io():
raid1_read_request() {
...
if (r1bio_existed) {
set_bit(R1BIO_Returned, &r1_bio->state);
raid_end_bio_io(r1_bio);
}
...
}
This calls allow_barrier(), decrementing the barrier count. Control then
returns to handle_read_error(), which ends with:
handle_read_error() {
...
raid1_read_request(mddev, bio, r1_bio->sectors, r1_bio);
allow_barrier(conf, sector);
}
Does this second allow_barrier() decrement the count again for a single I/O,
causing a net barrier underflow that blocks future raise_barrier() calls
and deadlocks the array?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260722185841.449934-1-mykola@meshstor.io?part=8
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v4 8/9] md/raid1,raid10: set IO_BLOCKED in case of BLK_STS_P2PDMA
2026-07-22 18:58 ` [PATCH v4 8/9] md/raid1,raid10: set IO_BLOCKED in case of BLK_STS_P2PDMA Mykola Marzhan
2026-07-22 19:16 ` sashiko-bot
@ 2026-07-22 19:19 ` Logan Gunthorpe
2026-07-22 20:02 ` Mykola Marzhan
1 sibling, 1 reply; 23+ messages in thread
From: Logan Gunthorpe @ 2026-07-22 19:19 UTC (permalink / raw)
To: Mykola Marzhan, Jens Axboe, Song Liu, Yu Kuai, Keith Busch,
Christoph Hellwig, Sagi Grimberg, linux-block, linux-raid,
linux-nvme
Cc: Li Nan, Xiao Ni, Guoqing Jiang, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Bjorn Helgaas,
Shivaji Kant, Pranjal Shrivastava, Henrique Carvalho,
linux-kernel, linux-rdma, linux-pci
On 2026-07-22 12:58, Mykola Marzhan wrote:
> From: Logan Gunthorpe <logang@deltatee.com>
>
> 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@deltatee.com>
> Co-developed-by: Mykola Marzhan <mykola@meshstor.io>
> Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
> ---
> 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
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v4 8/9] md/raid1,raid10: set IO_BLOCKED in case of BLK_STS_P2PDMA
2026-07-22 19:19 ` Logan Gunthorpe
@ 2026-07-22 20:02 ` Mykola Marzhan
0 siblings, 0 replies; 23+ messages in thread
From: Mykola Marzhan @ 2026-07-22 20:02 UTC (permalink / raw)
To: Logan Gunthorpe
Cc: Jens Axboe, Song Liu, Yu Kuai, Keith Busch, Christoph Hellwig,
Sagi Grimberg, linux-block, linux-raid, linux-nvme, Li Nan,
Xiao Ni, Guoqing Jiang, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Bjorn Helgaas,
Shivaji Kant, Pranjal Shrivastava, Henrique Carvalho,
linux-kernel, linux-rdma, linux-pci
> I think it would be better to
> move the code more towards similarity with raid1 instead of diverging it
> further.
I'll move bio = r10_bio->devs[slot].bio; up into the declaration so
raid10 matches raid1 in v5.
Thank you!
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 9/9] nvme-rdma: return BLK_STS_P2PDMA for unsupported P2P transfers
2026-07-22 18:58 [PATCH v4 0/9] block,md,nvme: correct handling of unsupported P2PDMA transfers Mykola Marzhan
` (7 preceding siblings ...)
2026-07-22 18:58 ` [PATCH v4 8/9] md/raid1,raid10: set IO_BLOCKED in case of BLK_STS_P2PDMA Mykola Marzhan
@ 2026-07-22 18:58 ` Mykola Marzhan
2026-07-22 19:20 ` sashiko-bot
8 siblings, 1 reply; 23+ messages in thread
From: Mykola Marzhan @ 2026-07-22 18:58 UTC (permalink / raw)
To: Jens Axboe, Song Liu, Yu Kuai, Keith Busch, Christoph Hellwig,
Sagi Grimberg, linux-block, linux-raid, linux-nvme
Cc: Li Nan, Xiao Ni, Guoqing Jiang, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Logan Gunthorpe,
Bjorn Helgaas, Shivaji Kant, Pranjal Shrivastava,
Henrique Carvalho, linux-kernel, linux-rdma, linux-pci
When the PCIe topology cannot route a P2P transfer between two devices,
the DMA layer fails the mapping with -EREMOTEIO, but ib_dma_map_sg()
returns 0 and the error is lost. nvme-rdma then fails the I/O with a
retryable status, so multipath requeues it forever and a single path
wastes its whole retry budget on an I/O that can never succeed.
Map the data and metadata scatterlists with ib_dma_map_sgtable_attrs(),
which preserves the error, and translate -EREMOTEIO to the new
BLK_STS_P2PDMA status so the request fails fast instead of being
retried. While at it, call nvme_start_request() only after mapping
succeeds 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..2f50509a7a61 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_P2PDMA;
else
ret = BLK_STS_IOERR;
nvme_cleanup_cmd(rq);
--
2.52.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v4 9/9] nvme-rdma: return BLK_STS_P2PDMA for unsupported P2P transfers
2026-07-22 18:58 ` [PATCH v4 9/9] nvme-rdma: return BLK_STS_P2PDMA for unsupported P2P transfers Mykola Marzhan
@ 2026-07-22 19:20 ` sashiko-bot
0 siblings, 0 replies; 23+ messages in thread
From: sashiko-bot @ 2026-07-22 19:20 UTC (permalink / raw)
To: Mykola Marzhan; +Cc: yukuai, linux-pci, linux-raid
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Calling nvme_start_request() after mapping the data for DMA causes stale metadata to be read by the RDMA device on non-coherent architectures.
Pre-existing issues:
- [Critical] Accessing `rq` and `req->sqe.dma` in the error path after `nvme_host_path_error(rq)` completes and frees the request.
- [High] Returning BLK_STS_IOERR or BLK_STS_P2PDMA directly to the block layer after nvme_start_request() has been called leaks the multipath nr_active counter and I/O stats.
--
--- Patch [9]: [PATCH v4 9/9] nvme-rdma: return BLK_STS_P2PDMA for unsupported P2P transfers ---
commit 53acbc82938f48638ba2fe9095582a69fb22439e
Author: Mykola Marzhan <mykola@meshstor.io>
nvme-rdma: return BLK_STS_P2PDMA for unsupported P2P transfers
When the PCIe topology cannot route a P2P transfer between two devices,
the DMA layer fails the mapping with -EREMOTEIO, but ib_dma_map_sg()
returns 0 and the error is lost. nvme-rdma then fails the I/O with a
retryable status, so multipath requeues it forever and a single path
wastes its whole retry budget on an I/O that can never succeed.
> diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
> index 6909e35427942..2f50509a7a616 100644
> --- a/drivers/nvme/host/rdma.c
> +++ b/drivers/nvme/host/rdma.c
[ ... ]
> @@ -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);
> +
[Severity: High]
Does moving nvme_start_request() here cause stale metadata to be read on
non-coherent architectures?
nvme_start_request() invokes blk_integrity_prepare(), which writes to the PI
metadata buffer via the CPU. Since this now happens after the metadata has
already been mapped for DMA in nvme_rdma_map_data(), these CPU writes won't
be flushed to main memory before the RDMA device accesses them.
> 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);
[Severity: Critical]
This is a pre-existing issue, but does this lead to a use-after-free?
nvme_host_path_error() synchronously completes the request via
__nvme_complete_rq(), freeing it back to the block layer where it can be
immediately reallocated.
The error path then continues to dereference rq and req->sqe.dma:
nvme_cleanup_cmd(rq);
unmap_qe:
ib_dma_unmap_single(dev, req->sqe.dma, sizeof(struct nvme_command),
DMA_TO_DEVICE);
Could this corrupt the newly reallocated request if it's reused by another CPU?
> 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_P2PDMA;
> else
> ret = BLK_STS_IOERR;
> nvme_cleanup_cmd(rq);
[Severity: High]
This isn't a bug introduced by this patch, but does returning an error status
directly to the block layer after nvme_start_request() leak multipath
counters?
When nvme_start_request() is executed, it increments the queue depth counter
(nr_active) and starts I/O accounting via nvme_mpath_start_request().
If the driver then returns an error status (like BLK_STS_P2PDMA or
BLK_STS_IOERR) to blk-mq, blk-mq completes the request directly, bypassing the
driver's nvme_mpath_end_request() hook. This permanently leaks the nr_active
counter.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260722185841.449934-1-mykola@meshstor.io?part=9
^ permalink raw reply [flat|nested] 23+ messages in thread