* [PATCH v3 4/8] md/raid1: don't use write-behind for P2PDMA bios
From: Mykola Marzhan @ 2026-07-21 17:44 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
In-Reply-To: <20260721174502.111503-1-mykola@meshstor.io>
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
* [PATCH v3 3/8] md/raid1: serialize non-write-behind writes on CollisionCheck rdevs
From: Mykola Marzhan @ 2026-07-21 17:44 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
In-Reply-To: <20260721174502.111503-1-mykola@meshstor.io>
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
* [PATCH v3 2/8] md: ensure REQ_NOMERGE is set on P2PDMA bios
From: Mykola Marzhan @ 2026-07-21 17:44 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
In-Reply-To: <20260721174502.111503-1-mykola@meshstor.io>
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
* [PATCH v3 1/8] blk-mq-dma: restore BLK_STS_TARGET for unsupported P2P transfers
From: Mykola Marzhan @ 2026-07-21 17:44 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
In-Reply-To: <20260721174502.111503-1-mykola@meshstor.io>
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
^ permalink raw reply related
* [PATCH v3 0/8] block,md,nvme: correct handling of unsupported P2PDMA transfers
From: Mykola Marzhan @ 2026-07-21 17:44 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 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
^ permalink raw reply
* Re: [PATCH 0/6] block,md,nvme: correct handling of unsupported P2PDMA transfers
From: Leon Romanovsky @ 2026-07-21 12:49 UTC (permalink / raw)
To: Keith Busch
Cc: Mykola Marzhan, Jens Axboe, Song Liu, Yu Kuai, Christoph Hellwig,
Sagi Grimberg, linux-block, linux-raid, linux-nvme, Li Nan,
Xiao Ni, Jason Gunthorpe, Kiran Kumar Modukuri,
Chaitanya Kulkarni, Logan Gunthorpe, Bjorn Helgaas, Shivaji Kant,
Pranjal Shrivastava, Henrique Carvalho, linux-kernel, linux-rdma,
linux-pci
In-Reply-To: <al5IJb-L0FpztnYz@kbusch-mbp>
On Mon, Jul 20, 2026 at 10:09:09AM -0600, Keith Busch wrote:
> On Mon, Jul 20, 2026 at 03:43:04PM +0300, Leon Romanovsky wrote:
> > It would be great if developers supervised their AI tools and kept
> > commit messages and cover letters concise and readable.
> >
> > Everything above can be reduced to a simple sentence: "MD treats an
> > unsupported P2P operation as success rather than failure; update the
> > code accordingly."
>
> Thank you for TL;DR summary! The verbosity from these things is a bit
> annoying.
Yeah, too verbose and conveys almost no useful information.
Lately, I've started ignoring such submissions to prioritize commits
from people who put real effort into explaining what they are doing.
Thanks
^ permalink raw reply
* Re: [PATCH] md/raid5: complete discard bios while reshape is active
From: genjian zhang @ 2026-07-21 6:48 UTC (permalink / raw)
To: yukuai; +Cc: song, magiclinan, xiao, linux-raid, linux-kernel, Genjian Zhang
In-Reply-To: <8ff91547-4841-4cea-87bc-9d6979df61ab@fygo.io>
At 2026-07-19 20:54:42, "yu kuai" <yukuai@fygo.io> wrote:
>Hi,
>
>在 2026/7/12 0:13, Genjian 写道:
>> From: Genjian Zhang <zhanggenjian@kylinos.cn>
>>
>> make_discard_request() returns without completing the bio when reshape
>> is in progress. Discard callers block in submit_bio_wait()
>> waiting for a completion that never arrives. The caller hangs in
>> uninterruptible sleep, and this does not resolve when reshape finishes.
>>
>> Complete the bio with BLK_STS_AGAIN so userspace can retry after reshape,
>> consistent with the existing policy of not processing discard during
>> reshape.
>>
>> Tested on a loop-backed RAID5 array during mdadm --grow: without this
>> patch, blkdiscard hangs in bio_await() and remains in uninterruptible
>> sleep after md reports "reshape done"; with this patch it returns
>> -EAGAIN instead.
>>
>> Signed-off-by: Genjian Zhang <zhanggenjian@kylinos.cn>
>> ---
>> drivers/md/raid5.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> index ffb5fcde54a9..4c3b42b2af85 100644
>> --- a/drivers/md/raid5.c
>> +++ b/drivers/md/raid5.c
>> @@ -5722,9 +5722,11 @@ static void make_discard_request(struct mddev *mddev, struct bio *bi)
>> if (WARN_ON_ONCE(bi->bi_opf & REQ_NOWAIT))
>> return;
>
>As nowait will be removed soon, I'm fine leaving it here.
>
>Reviewed-by: Yu Kuai <yukuai@fygo.io>
>
>>
>> - if (mddev->reshape_position != MaxSector)
>> + if (mddev->reshape_position != MaxSector) {
>> /* Skip discard while reshape is happening */
>> + bio_endio_status(bi, BLK_STS_AGAIN);
>> return;
>> + }
>>
>> if (!raid5_discard_limits(mddev, bi))
>> return;
>
>--
>Thanks,
>Kuai
Thanks for the review.
I'll leave the REQ_NOWAIT path unchanged and won't send a v2 for it.
Thanks,
Genjian
^ permalink raw reply
* Re: [PATCH v2 1/7] blk-mq-dma: restore BLK_STS_TARGET for unsupported P2P transfers
From: Logan Gunthorpe @ 2026-07-20 18:49 UTC (permalink / raw)
To: Mykola Marzhan, Christoph Hellwig
Cc: Jens Axboe, Song Liu, Yu Kuai, Keith Busch, 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
In-Reply-To: <b2c37f4f-79dd-428f-915a-2a862d20924c@deltatee.com>
On 2026-07-20 12:42, Logan Gunthorpe wrote:
>
>
> On 2026-07-20 11:30, Mykola Marzhan wrote:
>> On Mon, Jul 20, 2026 at 4:49 PM Christoph Hellwig <hch@lst.de
>>> This is a really weird writing style, and suggested to me you neither
>>> understand the code nor the problem. Please actually think yourself,
>>> write the patches and commit logs yourself instdad of this garbage.
>>>
>>> And please also explain how you even generate the I/O that fails this
>>> way.
>> - md mirrors the write: the copy to member 1 succeeds, the copy to far
>> member fails as BLK_STS_INVAL.
>> - md deliberately ignores BLK_STS_INVAL, so the write is counted as
>> written and the mirror silently diverges.
>
> Seeing I just reviewed this and answered it for myself I'll expand on
> what's going on here:
>
> The switch statement will take the default branch when
> pci_p2pdma_state() returns PCI_P2PDMA_MAP_NOT_SUPPORTED. This indicates
> the memory that's trying to be mapped in this way will not succeed
> because it's going through an unsupported host bridge (or if the device
> isn't a PCI device, etc, but this is rarer or perhaps not possible).
> Returning BLK_STS_INVAL doesn't seem correct here, to me. (Arguably it
> might have been clearer if the switch case explicitly stated
> PCI_P2PDMA_MAP_NOT_SUPPORTED instead of relying on the default).
>
> Returning -EREMOTEIO/BLK_STS_TARGET was the convention for this I had
> originally set when I wrote some of this and differentiating the error
> does seem important in Patch 6 in this series.
Sorry, one other note I almost forgot about which is probably important:
BLK_STS_INVAL will attempt a retry, where BLK_STS_TARGET will not (I
think that's part of why I chose that specific error in the first
place). A retry here doesn't make any sense because it will always hit
the same case and fail. So it's probably worth applying this patch even
without the specific error being used in Patch 6.
Logan
^ permalink raw reply
* Re: [PATCH v2 1/7] blk-mq-dma: restore BLK_STS_TARGET for unsupported P2P transfers
From: Logan Gunthorpe @ 2026-07-20 18:42 UTC (permalink / raw)
To: Mykola Marzhan, Christoph Hellwig
Cc: Jens Axboe, Song Liu, Yu Kuai, Keith Busch, 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
In-Reply-To: <CAPzsNDv6hUV3gHdaxuwjRcr4couUKq_M5DKObFNaYqCHnm7ZMw@mail.gmail.com>
On 2026-07-20 11:30, Mykola Marzhan wrote:
> On Mon, Jul 20, 2026 at 4:49 PM Christoph Hellwig <hch@lst.de
>> This is a really weird writing style, and suggested to me you neither
>> understand the code nor the problem. Please actually think yourself,
>> write the patches and commit logs yourself instdad of this garbage.
>>
>> And please also explain how you even generate the I/O that fails this
>> way.
> - md mirrors the write: the copy to member 1 succeeds, the copy to far
> member fails as BLK_STS_INVAL.
> - md deliberately ignores BLK_STS_INVAL, so the write is counted as
> written and the mirror silently diverges.
Seeing I just reviewed this and answered it for myself I'll expand on
what's going on here:
The switch statement will take the default branch when
pci_p2pdma_state() returns PCI_P2PDMA_MAP_NOT_SUPPORTED. This indicates
the memory that's trying to be mapped in this way will not succeed
because it's going through an unsupported host bridge (or if the device
isn't a PCI device, etc, but this is rarer or perhaps not possible).
Returning BLK_STS_INVAL doesn't seem correct here, to me. (Arguably it
might have been clearer if the switch case explicitly stated
PCI_P2PDMA_MAP_NOT_SUPPORTED instead of relying on the default).
Returning -EREMOTEIO/BLK_STS_TARGET was the convention for this I had
originally set when I wrote some of this and differentiating the error
does seem important in Patch 6 in this series.
Logan
^ permalink raw reply
* Re: [PATCH v2 6/7] md/raid1,raid10: skip futile retries on P2PDMA mapping failures
From: Logan Gunthorpe @ 2026-07-20 16:49 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
In-Reply-To: <20260719105327.864949-7-mykola@meshstor.io>
On 2026-07-19 04:53, Mykola Marzhan wrote:
> drivers/md/raid1.c | 56 +++++++++++++++++++++++++++++++-----
> drivers/md/raid1.h | 2 ++
> drivers/md/raid10.c | 69 ++++++++++++++++++++++++++++++++++++++-------
> drivers/md/raid10.h | 2 ++
This is an intimidating and massive patch to review. I gotta second the
verbosity of the AI commit messages comment from Keith.
My eyes glazed over a bit. But one point is that maybe this could be
split into multiple patches: one patch introduces R1BIO_P2PDMA, one
patch for raid1 and another patch for raid10? That might make it a bit
more manageable.
Also, I'm not sure if it's possible without digging deeper but, instead
of adding a !p2pdma_unmappable check to every if() statement in the
raid*_end_write_request() functions, might it be possible to factor out
a helper function and return early from it? Maybe the helper could be
shared between raid10 and raid1 as they look very similar. The
differences between the two are minimal -- there might be a bug fix that
was applied to one that should be applied to both?
Thanks!
Logan
^ permalink raw reply
* Re: [PATCH v2 5/7] md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones
From: Logan Gunthorpe @ 2026-07-20 16:32 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
In-Reply-To: <20260719105327.864949-6-mykola@meshstor.io>
On 2026-07-19 04:53, Mykola Marzhan wrote:
> 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>
Looks correct, thanks.
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
^ permalink raw reply
* Re: [PATCH v2 4/7] md/raid1: don't use write-behind for P2PDMA bios
From: Logan Gunthorpe @ 2026-07-20 16:31 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
In-Reply-To: <20260719105327.864949-5-mykola@meshstor.io>
On 2026-07-19 04:53, Mykola Marzhan wrote:
> 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>
Looks correct to me.
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
^ permalink raw reply
* Re: [PATCH 0/6] block,md,nvme: correct handling of unsupported P2PDMA transfers
From: Keith Busch @ 2026-07-20 16:09 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Mykola Marzhan, Jens Axboe, Song Liu, Yu Kuai, Christoph Hellwig,
Sagi Grimberg, linux-block, linux-raid, linux-nvme, Li Nan,
Xiao Ni, Jason Gunthorpe, Kiran Kumar Modukuri,
Chaitanya Kulkarni, Logan Gunthorpe, Bjorn Helgaas, Shivaji Kant,
Pranjal Shrivastava, Henrique Carvalho, linux-kernel, linux-rdma,
linux-pci
In-Reply-To: <20260720124304.GE110966@unreal>
On Mon, Jul 20, 2026 at 03:43:04PM +0300, Leon Romanovsky wrote:
> It would be great if developers supervised their AI tools and kept
> commit messages and cover letters concise and readable.
>
> Everything above can be reduced to a simple sentence: "MD treats an
> unsupported P2P operation as success rather than failure; update the
> code accordingly."
Thank you for TL;DR summary! The verbosity from these things is a bit
annoying.
^ permalink raw reply
* Re: [PATCH 2/6] md: ensure REQ_NOMERGE is set on P2PDMA bios
From: Logan Gunthorpe @ 2026-07-20 16:03 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, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Bjorn Helgaas,
Shivaji Kant, Pranjal Shrivastava, Henrique Carvalho,
linux-kernel, linux-rdma, linux-pci
In-Reply-To: <20260718162547.448892-3-mykola@meshstor.io>
On 2026-07-18 10:25, Mykola Marzhan wrote:
> md_submit_bio() unconditionally strips REQ_NOMERGE before passing the
> bio to the personality, an optimization from commit 9c573de3283a ("MD:
> make bio mergeable"): a bio that md has split may become mergeable
> again below md.
>
> For PCI P2PDMA bios the flag is load-bearing, not a hint. The block
> layer sets REQ_NOMERGE on P2PDMA bios (__bio_add_page(), and the
> extraction path of bio_iov_iter_get_pages()) because the DMA mapping
> type of a request is resolved once, from its first segment
> (blk_dma_map_iter_start()), and request-level merging is prevented
> only by REQ_NOMERGE. Stripping it allows the member queue to merge a
> P2PDMA bio with a bio carrying pages of a different pgmap, or host
> memory, mapping the merged segments with the wrong bus address:
> silent data corruption on the member.
>
> Set the flag for P2PDMA bios instead of merely preserving it. No
> in-tree path currently submits P2PDMA pages through the bvec-iter
> path (bio_iov_bvec_set()), which skips the flagging -- but nothing
> structural prevents one, so setting rather than preserving hardens
> md against that gap at the cost of one branch. Everything else keeps
> the original optimization of clearing the flag. This covers every
> personality that advertises BLK_FEAT_PCI_P2PDMA (raid0, raid1,
> raid10), which is why the fix lives in the shared md_submit_bio()
> path.
>
> 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>
Makes sense to me. Matches the similar check in nvmet_bdev_execute_rw().
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
^ permalink raw reply
* Re: [PATCH 1/6] blk-mq-dma: restore BLK_STS_TARGET for unsupported P2P transfers
From: Logan Gunthorpe @ 2026-07-20 15:58 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, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Bjorn Helgaas,
Shivaji Kant, Pranjal Shrivastava, Henrique Carvalho,
linux-kernel, linux-rdma, linux-pci
In-Reply-To: <20260718162547.448892-2-mykola@meshstor.io>
On 2026-07-18 10:25, Mykola Marzhan wrote:
> 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
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
Looks correct to me, thanks!
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
^ permalink raw reply
* Re: [PATCH v2 1/7] blk-mq-dma: restore BLK_STS_TARGET for unsupported P2P transfers
From: Christoph Hellwig @ 2026-07-20 14:49 UTC (permalink / raw)
To: Mykola Marzhan
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, Logan Gunthorpe,
Bjorn Helgaas, Shivaji Kant, Pranjal Shrivastava,
Henrique Carvalho, linux-kernel, linux-rdma, linux-pci
In-Reply-To: <20260719105327.864949-2-mykola@meshstor.io>
On Sun, Jul 19, 2026 at 10:53:21AM +0000, Mykola Marzhan wrote:
> matching dma_map_sgtable()'s -EREMOTEIO: "When this happens, return
> BLK_STS_TARGET so the request isn't retried." The conversion to
> blk_rq_dma_map silently changed the status to BLK_STS_INVAL, which
> regresses two consumers:
This is a really weird writing style, and suggested to me you neither
understand the code nor the problem. Please actually think yourself,
write the patches and commit logs yourself instdad of this garbage.
And please also explain how you even generate the I/O that fails this
way.
^ permalink raw reply
* Re: [PATCH] md: do overflow check for sb->bblog_shift in super_1_load()
From: Coly Li @ 2026-07-20 14:38 UTC (permalink / raw)
To: sashiko-reviews; +Cc: yukuai, linux-raid
In-Reply-To: <20260720113553.9353D1F000E9@smtp.kernel.org>
> 2026年7月20日 19:35，sashiko-bot@kernel.org 写道：
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] The newly added bounds check for `sb->bblog_shift` incorrectly allows values up to 63, failing to prevent 32-bit integer overflows and undefined behavior which can lead to a divide-by-zero kernel panic.
> --
>
> --- Patch [1]: [PATCH] md: do overflow check for sb->bblog_shift in super_1_load() ---
> commit 36f9915dbd10f1c3803610b32e46390e9cb7ec7c
> Author: Coly Li <colyli@fygo.io>
>
> md: do overflow check for sb->bblog_shift in super_1_load()
>
> This commit adds a bounds check for sb->bblog_shift in super_1_load() to
> prevent possible overflows in the badblocks API. It limits the shift value
> to be less than BITS_PER_TYPE(sector_t).
>
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index d1465bcd86c81..67f8edf4db368 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -1920,6 +1920,13 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
>> rdev->bb_page, REQ_OP_READ, true))
>> return -EIO;
>> bbp = (__le64 *)page_address(rdev->bb_page);
>> +
>> + /* check for badblocks api. */
>> + if (sb->bblog_shift >= BITS_PER_TYPE(sector_t)) {
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> [Severity: High]
> This is a pre-existing issue, but does this bounds check still allow values
> that can cause undefined behavior and integer overflows?
>
> Since BITS_PER_TYPE(sector_t) is 64, this allows sb->bblog_shift to be up
> to 63.
>
> However, count is declared as a 32-bit signed int in super_1_load(), so the
> following operation:
>
> count <<= sb->bblog_shift;
>
> will cause undefined behavior if the shift is >= 31.
>
> Additionally, downstream in block/badblocks.c (such as in badblocks_check()
> and _badblocks_set()), the shift is used with a 32-bit literal:
>
> rounddown(s, 1 << bb->shift);
>
> If sb->bblog_shift is >= 32, this can result in undefined behavior and
> yield 0, leading to a divide-by-zero panic in the rounddown() macro during
> array IO.
>
> Should the maximum shift be restricted to a smaller value?
With an overflow check patch from Ramesh Adhikari <adhikari.resume@gmail.com>,
only checking sb->bblog_shift >= BITS_PER_TYPE(sector_t) is enough. The rested overflow
check will be performed inside badblocks API.
And if variables are overflowed before calling badblocks API, this is not the badblocks API
issue, should not belong to the badblocks API check fixes.
Also I don’t feel it is necessary to do restricted overflow checking in super block loading stage,
if the invalid values are rejected, this super block will fail to load and won’t introduce any run time
problem.
Therefore, current checking with (sb->bblog_shift >= BITS_PER_TYPE(sector_t) is sufficient.
>
>> + pr_err("md: %pg: bogus bblog_shift %u for badblocks.\n",
>> + rdev->bdev, sb->bblog_shift);
>> + return -EINVAL;
>> + }
>> rdev->badblocks.shift = sb->bblog_shift;
>> for (i = 0 ; i < (sectors << (9-3)) ; i++, bbp++) {
>> u64 bb = le64_to_cpu(*bbp);
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260720111400.2120834-1-colyli@fygo.io?part=1
^ permalink raw reply
* Re: [PATCH 0/6] block,md,nvme: correct handling of unsupported P2PDMA transfers
From: Leon Romanovsky @ 2026-07-20 12:43 UTC (permalink / raw)
To: Mykola Marzhan
Cc: Jens Axboe, Song Liu, Yu Kuai, Keith Busch, Christoph Hellwig,
Sagi Grimberg, linux-block, linux-raid, linux-nvme, Li Nan,
Xiao Ni, Jason Gunthorpe, Kiran Kumar Modukuri,
Chaitanya Kulkarni, Logan Gunthorpe, Bjorn Helgaas, Shivaji Kant,
Pranjal Shrivastava, Henrique Carvalho, linux-kernel, linux-rdma,
linux-pci
In-Reply-To: <20260718162547.448892-1-mykola@meshstor.io>
On Sat, Jul 18, 2026 at 04:25:41PM +0000, Mykola Marzhan wrote:
> Driving peer-to-peer I/O (NVMe CMB source) through md arrays and
> nvme-rdma legs on asymmetric PCIe topologies -- where the peer device
> reaches some members/paths but not others -- turns up two failures on
> v7.2-rc, both rooted in how an unsupported-P2PDMA mapping failure is
> reported. On md: silent data loss -- an unreachable-leg P2PDMA write
> counts as written, mirrors silently diverge, and where no member is
> reachable the write reports success with zero copies on stable
> storage; reads picked to an unreachable leg fail with EIO and are
> never retried on the mirror that holds the data. On nvme-rdma: the
> transport swallows the DMA layer's mapping errno into a path error,
> which under the default multipath configuration livelocks the I/O (a
> stacked md mirror hangs) and with nvme_core.multipath=N surfaces as
> retryable BLK_STS_TRANSPORT.
>
> The root cause is a block-layer status-code regression, not an md or
> nvme bug. Commit 91fb2b6052f7 ("nvme-pci: convert to using
> dma_map_sgtable()") deliberately mapped unsupported P2PDMA transfers
> to BLK_STS_TARGET ("... return BLK_STS_TARGET so the request isn't
> retried"). Commit 858299dc6160 ("block: add scatterlist-less DMA
> mapping helpers") silently changed that to BLK_STS_INVAL, and since
> commit 7ce3c1dd78fc ("nvme-pci: convert the data mapping to
> blk_rq_dma_map") in v6.17 the failure surfaces to direct NVMe
> consumers as EINVAL instead of the documented -EREMOTEIO, with
> blk_path_error() now classifying it as retryable. md/raid1,raid10
> ignore BLK_STS_INVAL leg failures per commit f7b24c7b41f2
> ("md/raid1,raid10: don't fail devices for invalid IO errors"), where
> it means a request-shaped error that fails identically on every
> member -- hence the silent divergence. The md leg of the regression
> became reachable when md started advertising BLK_FEAT_PCI_P2PDMA in
> v7.2-rc1 (commit 02666132403a ("md: propagate BLK_FEAT_PCI_P2PDMA
> from member devices to RAID device")).
>
> Patch 1 restores BLK_STS_TARGET at the source. md then handles an
> unreachable leg like any other per-device error: badblocks on the
> affected member, the master bio succeeds while an In_sync leg holds
> the data, and reads are redirected to the other mirror.
>
> Patches 2-4 fix independent md-side P2PDMA bugs of the same vintage:
> md_submit_bio() strips REQ_NOMERGE (the only request-level protection
> against merging P2PDMA segments across pgmaps); raid1 write-behind
> CPU-copies device BAR memory via bio_copy_data(); and
> narrow_write_error()'s retry clones reset bi_opf, dropping the
> REQ_NOMERGE protection exactly on the error-retry path.
>
> Patch 5 stops the restored device-error machinery from misfiring
> where its medium-error assumptions don't hold. A mapping failure is a
> property of the peer/member pairing: retrying the same peer pages
> against the same member cannot succeed, and there is nothing on the
> medium to repair. Without it, narrow_write_error() serializes dozens
> of guaranteed-to-fail chunk retries per failed write; on reads each
> unreachable-leg pick costs a full freeze_array() quiesce plus a tick
> of the read-error budget -- measured below, a mixed host/P2P read
> workload on an asymmetric topology kicks the perfectly healthy far
> leg after ~20 failed picks, in under a second; and on FailFast
> members a single unroutable I/O evicts a healthy mirror outright.
> Because BLK_STS_TARGET is also produced for transient device
> conditions md cannot distinguish from bi_status alone, writes probe
> the whole range once instead of predicting futility: a real mapping
> failure is recorded in one call, a cleared transient recovers with
> nothing recorded. Patches 1 and 5 belong in the same release: with
> patch 1 alone, restoring the error surfaces the retry storms and
> read-path evictions above on 7.2's newly reachable md path.
>
> Patch 6 closes the same hole in the rdma transport. nvme-rdma maps
> the data scatterlist with ib_dma_map_sg(), which returns 0 on a
> peer-unreachable failure, discarding the -EREMOTEIO that
> dma_map_sgtable() documents for exactly this case; the driver reports
> it as a path error. Because the multipath head node advertises
> BLK_FEAT_PCI_P2PDMA (commit fb0eeeed91f3 ("nvme-multipath: enable PCI
> P2PDMA for multipath devices"), v7.2-rc1) and the mapping failure is
> deterministic, nvme_failover_req() requeues the bios with a fresh
> retry budget every cycle and the I/O never completes -- a hot requeue
> livelock that hangs a stacked md mirror instead of failing over; with
> nvme_core.multipath=N it burns nvme_max_retries requeues and
> completes as retryable BLK_STS_TRANSPORT. Patch 6 maps with
> ib_dma_map_sgtable_attrs() to preserve the errno and returns
> BLK_STS_TARGET, matching nvme-pci so patch 5's handling covers rdma
> legs too, and starts the request only after the map succeeds so
> nvme_mpath_start_request() accounting cannot leak on the direct
> blk-mq completion. nvme-rdma stays on the scatterlist DMA API: its
> fast-reg MR path (ib_map_mr_sg()) consumes scatterlists, so a
> blk_rq_dma_map conversion is separate modernization, out of scope for
> a fix.
>
> Behavior on v7.2-rc2, QEMU rig (one CMB provider, two NVMe members,
> q35 PCIe topologies), 90 8KiB peer-memory reads under concurrent
> host reads for the read rows:
>
> topology op v7.2-rc2 patched (1-5)
> symmetric write+read ok ok
> asymmetric write "ok", silent diverge ok + badblocks on
> far leg (1 probe)
> asymmetric read 41/90 EIO, no mirror 90/90 ok, no
> under load retry, no eviction eviction
> unreachable write "ok", NO data copied EIO + badblocks
> unreachable read EIO EIO, no members
> kicked
>
> Also verified on the same rig: a transient injected TARGET recovers
> through the single probe with nothing recorded; P2P MEDIUM keeps the
> chunked path; host-page TARGET/INVAL handling is unchanged; FailFast
> survives mapping failures but still evicts on genuine errors;
> write-mostly legs are written directly (no write-behind CPU copy); a
> degraded array whose only leg is unreachable gets plain EIO -- no
> budget charge, no eviction; an exhausted badblocks table fails the
> member as described below. The write/read/injection rows repeat
> identically on raid10.
>
> Validation of patch 6: the mechanism -- ib_dma_map_sg() returning 0
> where ib_dma_map_sgtable_attrs() preserves -EREMOTEIO -- was
> confirmed on a real mlx5 HCA with a map-only probe. The
> start-after-map reorder ran over an rxe nvmet-rdma loopback under fio
> with crc32c verification; multipath inflight and nr_active accounting
> drain to zero, matching the pre-reorder kernel. rxe cannot produce
> -EREMOTEIO, so the failure paths were driven by injecting that exact
> mechanism at the map call site: unpatched, one 4KiB write livelocked
> (132 failover requeues in 8s, a stacked raid1 hung; multipath=N:
> retryable BLK_STS_TRANSPORT); patched, it failed immediately as
> BLK_STS_TARGET with zero requeues, and the stacked raid1 badblocked
> the leg without evicting it.
>
> Known trade-offs of routing through the stock md write machinery:
> an unroutable P2P write records badblocks and sets WriteErrorSeen
> and WantReplacement, so later writes overlapping those ranges skip
> the member -- host writes do not clear the entries and repair skips
> known-bad ranges -- leaving them single-copy until re-add or
> replacement recovery (host memory, so it succeeds) rewrites and
> clears them; a member whose badblocks table is exhausted or disabled
> is failed on the first unroutable P2P write, as with any write error
> md cannot record. FailFast members are exempted from md_error() for
> mapping failures only (nothing reached the wire, so the error says
> nothing about device health); genuine I/O errors keep their
> immediate-eviction semantics. raid10 additionally fails a
> replacement device outright on a P2P write mapping failure (its
> stock replacement-write policy), and an IO_BLOCKED slot excludes
> that slot's replacement from the read retry -- both pre-existing
> raid10 behaviors with a new trigger. On reads, read_balance() keeps
> no memory of unreachability, so each far-leg pick still costs one
> failed submission before the redirect; a sticky per-rdev
> reachability hint would be md-next follow-up material, as would a
> distinct block status naming the mapping failure exactly.
>
> Routing: patch 1 is block-tree material and fixes a v6.17-rc1 status
> regression (hence its Cc: stable); patches 2-5 are md and address
> exposure that only became reachable with v7.2-rc1; patch 6 is
> nvme-rdma and also carries Cc: stable -- the transport misreport
> predates the head-node change (Fixes: 23528aa3320a, v7.1) and is
> reachable on v7.1 with multipath=N. Patch 6 sources the errno from
> dma_map_sgtable() in rdma.c and does not depend on patch 1, which
> fixes the separate blk_rq_dma_map path. Patches 1 (block) and 6 (nvme) are
> independently applicable bug fixes: each fixes a user-visible defect
> on its own and neither depends on the md patches, while 2-5 are
> md-side hardening built on patch 1 -- a maintainer can take 1 and 6
> even if the md discussion runs longer. Single-cycle routing through
> the relevant trees with acks works for us: patch 1 block, 2-5 md,
> 6 nvme.
>
> Per Documentation/process/coding-assistants.rst: the patches were
> developed with AI assistance (see the Assisted-by trailers); all code
> was human-reviewed and tested as described above.
It would be great if developers supervised their AI tools and kept
commit messages and cover letters concise and readable.
Everything above can be reduced to a simple sentence: "MD treats an
unsupported P2P operation as success rather than failure; update the
code accordingly."
Thanks
>
> Series is against v7.2-rc2.
>
> Mykola Marzhan (6):
> blk-mq-dma: restore BLK_STS_TARGET for unsupported P2P transfers
> md: ensure REQ_NOMERGE is set on P2PDMA bios
> md/raid1: don't use write-behind for P2PDMA bios
> md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones
> md/raid1,raid10: skip futile retries on P2PDMA mapping failures
> nvme-rdma: return BLK_STS_TARGET for unsupported P2P transfers
>
> block/blk-mq-dma.c | 10 ++++++-
> drivers/md/md.c | 14 ++++++++--
> drivers/md/md.h | 31 +++++++++++++++++++++
> drivers/md/raid1.c | 56 +++++++++++++++++++++++++++++++------
> drivers/md/raid10.c | 60 ++++++++++++++++++++++++++++++++++------
> drivers/nvme/host/rdma.c | 26 +++++++++++------
> 6 files changed, 168 insertions(+), 29 deletions(-)
>
>
> base-commit: 8cdeaa50eae8dad34885515f62559ee83e7e8dda
> --
> 2.43.0
>
>
^ permalink raw reply
* Re: [PATCH] md: do overflow check for sb->bblog_shift in super_1_load()
From: sashiko-bot @ 2026-07-20 11:35 UTC (permalink / raw)
To: colyli; +Cc: yukuai, linux-raid
In-Reply-To: <20260720111400.2120834-1-colyli@fygo.io>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The newly added bounds check for `sb->bblog_shift` incorrectly allows values up to 63, failing to prevent 32-bit integer overflows and undefined behavior which can lead to a divide-by-zero kernel panic.
--
--- Patch [1]: [PATCH] md: do overflow check for sb->bblog_shift in super_1_load() ---
commit 36f9915dbd10f1c3803610b32e46390e9cb7ec7c
Author: Coly Li <colyli@fygo.io>
md: do overflow check for sb->bblog_shift in super_1_load()
This commit adds a bounds check for sb->bblog_shift in super_1_load() to
prevent possible overflows in the badblocks API. It limits the shift value
to be less than BITS_PER_TYPE(sector_t).
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index d1465bcd86c81..67f8edf4db368 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -1920,6 +1920,13 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
> rdev->bb_page, REQ_OP_READ, true))
> return -EIO;
> bbp = (__le64 *)page_address(rdev->bb_page);
> +
> + /* check for badblocks api. */
> + if (sb->bblog_shift >= BITS_PER_TYPE(sector_t)) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[Severity: High]
This is a pre-existing issue, but does this bounds check still allow values
that can cause undefined behavior and integer overflows?
Since BITS_PER_TYPE(sector_t) is 64, this allows sb->bblog_shift to be up
to 63.
However, count is declared as a 32-bit signed int in super_1_load(), so the
following operation:
count <<= sb->bblog_shift;
will cause undefined behavior if the shift is >= 31.
Additionally, downstream in block/badblocks.c (such as in badblocks_check()
and _badblocks_set()), the shift is used with a 32-bit literal:
rounddown(s, 1 << bb->shift);
If sb->bblog_shift is >= 32, this can result in undefined behavior and
yield 0, leading to a divide-by-zero panic in the rounddown() macro during
array IO.
Should the maximum shift be restricted to a smaller value?
> + pr_err("md: %pg: bogus bblog_shift %u for badblocks.\n",
> + rdev->bdev, sb->bblog_shift);
> + return -EINVAL;
> + }
> rdev->badblocks.shift = sb->bblog_shift;
> for (i = 0 ; i < (sectors << (9-3)) ; i++, bbp++) {
> u64 bb = le64_to_cpu(*bbp);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720111400.2120834-1-colyli@fygo.io?part=1
^ permalink raw reply
* [PATCH] md: do overflow check for sb->bblog_shift in super_1_load()
From: colyli @ 2026-07-20 11:14 UTC (permalink / raw)
To: yukuai; +Cc: linux-raid, linux-block, Coly Li, stable, Ramesh Adhikari
From: Coly Li <colyli@fygo.io>
In super_1_load(), sb->bblog_shift is an __u8 type value loaded from on-
disk superblock. It is used for badblocks API badblocks_set() by the
following sequence,
1930 rdev->badblocks.shift = sb->bblog_shift;
1931 for (i = 0 ; i < (sectors << (9-3)) ; i++, bbp++) {
1932 u64 bb = le64_to_cpu(*bbp);
1933 int count = bb & (0x3ff);
1934 u64 sector = bb >> 10;
1935 sector <<= sb->bblog_shift;
1936 count <<= sb->bblog_shift;
1937 if (bb + 1 == 0)
1938 break;
1939 if (!badblocks_set(&rdev->badblocks, sector, count, 1))
1940 return -EINVAL;
1941 }
bb->bblog_shit is in range of 0-255, variable sector is 64bit width, for
an invalid bb->bblog_shit, it is possible to make sector be overflowed
by the following calculation,
1935 sector <<= sb->bblog_shift;
Then in turn when call badblocks_set() at line 1939 with the invalid
rdev->badblocks.shift set at line 1930, may result an overflow inside
_badblocks_clear() in block/badblocks.c.
Although there are many places to call badblocks APIs, the non-zero
shift value is only used in super_1_load(), other places always use 0 as
the shift value. Therefore it is unnecessary to do a general shift value
overflow check inside badblock API, and just check here as the caller.
This may avoid unnecessary check, make the badblocks API code more simple
and elegant.
Fixes: 2699b67223aca ("md: load/store badblock list from v1.x metadata")
Fixes: 1726c77467833 ("badblocks: improve badblocks_set() for multiple ranges handling")
Cc: stable@vger.kernel.org
Cc: Ramesh Adhikari <adhikari.resume@gmail.com>
Signed-off-by: Coly Li <colyli@fygo.io>
---
drivers/md/md.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index d1465bcd86c8..67f8edf4db36 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -1918,10 +1918,17 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
bb_sector = (long long)offset;
if (!sync_page_io(rdev, bb_sector, sectors << 9,
rdev->bb_page, REQ_OP_READ, true))
return -EIO;
bbp = (__le64 *)page_address(rdev->bb_page);
+
+ /* check for badblocks api. */
+ if (sb->bblog_shift >= BITS_PER_TYPE(sector_t)) {
+ pr_err("md: %pg: bogus bblog_shift %u for badblocks.\n",
+ rdev->bdev, sb->bblog_shift);
+ return -EINVAL;
+ }
rdev->badblocks.shift = sb->bblog_shift;
for (i = 0 ; i < (sectors << (9-3)) ; i++, bbp++) {
u64 bb = le64_to_cpu(*bbp);
int count = bb & (0x3ff);
u64 sector = bb >> 10;
--
2.47.3
^ permalink raw reply related
* Re: [PATCH] md: fix soft lockup during resync when sync is repeatedly skipped
From: Yunye Zhao @ 2026-07-20 6:20 UTC (permalink / raw)
To: Yu Kuai, Song Liu
Cc: Yunye Zhao, Li Nan, Xiao Ni, Joseph Qi, linux-raid, linux-kernel
In-Reply-To: <7abeaff6-1387-4046-bea3-f1b04465addd@fygo.io>
Hi Kuai,
On 2026/7/17 14:27, Yu Kuai wrote:
>> md_do_sync()'s main loop advances io_sectors only when I/O is actually
>> issued (skipped == 0). When sync_request() keeps returning skipped == 1,
>> io_sectors never increases, [...]
>
> That's not expected, io_sectors should always increase in the skip case.
Sorry, my description was not accurate. The problem is not that io_sectors
stays 0. md_do_sync()'s loop exit condition is j == max_sectors, and in
raid10_sync_request()'s recovery path sectors is always 128, so for a very
large max_sectors the loop iterates a huge number of times.
raid10_sync_request(), recovery branch:
max_sync = RESYNC_PAGES << (PAGE_SHIFT-9); /* 128 */
must_sync = md_bitmap_start_sync(mddev, sect, &sync_blocks, true);
if (sync_blocks < max_sync) /* sync_blocks huge, never true */
max_sync = sync_blocks; /* so max_sync stays 128 */
...
if (biolist == NULL) {
*skipped = 1;
return max_sync; /* 128; the large span is discarded */
}
Back in md_do_sync()'s main loop, j then crawls forward 128 sectors per
call until it reaches max_sectors:
while (j < max_sectors) {
sectors = mddev->pers->sync_request(mddev, j, max_sectors, &skipped);
if (!skipped) /* skipped == 1 -> io_sectors stays 0 */
io_sectors += sectors;
j += sectors; /* j += 128 only */
if (last_check + window > io_sectors || j == max_sectors)
continue;
}
From the vmcore:
sync_blocks returned = 0x5ED03680 (~1.59e8 sectors), clamped to 128
recovery max_sectors = dev_sectors = 2^40
iterations = 2^40 / 128 = 2^33 (~8.6e9)
> What kernel version you're testing?
6.6.102. I also tested mainline and hit the same problem.
> If this is latest kernel, bitmap_start_sync() need to be fixed. It can't
> return skip while setting skipping sectors to 0.
bitmap_start_sync() is actually fine -- it returns a large clean span
(0x5ED03680 above). The recovery path just discards it: max_sync is only
ever clamped down, so it returns 128 regardless.
> And since this is dead loop, a cond_resched() will not fix anything.
Agreed -- cond_resched() only stops the watchdog; the thread still spins
~8.6e9 no-op iterations and pins a CPU for ~313s.
For v2 I'd fix this at the source: make the recovery path honour the clean
span reported by the bitmap instead of capping the skip at 128. Does that
direction look right to you?
Thanks,
Yunye
^ permalink raw reply
* Re: [PATCH 1/3] md/raid10: restore still_degraded detection in recovery
From: Paul Menzel @ 2026-07-20 4:32 UTC (permalink / raw)
To: Mykola Marzhan
Cc: Song Liu, Yu Kuai, Li Nan, Xiao Ni, Hannes Reinecke, linux-raid,
linux-kernel
In-Reply-To: <20260719144227.940444-2-mykola@meshstor.io>
Dear Mykola,
Thank you for your patch.
Am 19.07.26 um 16:42 schrieb Mykola Marzhan:
> raid10_sync_request()'s recovery arm passes still_degraded -- "will
> the array still be degraded after this recovery completes?" -- to
> md_bitmap_start_sync(), whose !degraded branch consumes the chunk's
> NEEDED marker, the write-intent record. That is only safe when the
> recovery restores full redundancy; otherwise the record still
> describes writes an absent member missed.
>
> Commit fe6a19d40ceb ("md/md-bitmap: merge md_bitmap_start_sync()
> into bitmap_operations") turned the detection loop's only assignment
> from "still_degraded = 1" into "still_degraded = false" in its
> int-to-bool styling pass, so the flag has been constant false ever
> since. raid1 and raid5 were converted to "= true"; only raid10 was
> inverted.
>
> The result is silent data loss on a classic-bitmap raid10 that stays
> degraded across a recovery. With mirror pairs {A,B} and {C,D}: lose
> D, keep writing, lose B, add a spare. The rebuild into B's slot
> strips NEEDED from every chunk -- including chunks whose only
> unsatisfied intent belonged to D -- so a later --re-add of D sees an
> empty intent record, completes instantly, and serves stale data.
>
> Restore the pre-conversion assignment. Only the classic-bitmap
> recovery arm is affected: llbitmap ignores the degraded parameter,
> and the resync arm passes mddev->degraded directly.
>
> Fixes: fe6a19d40ceb ("md/md-bitmap: merge md_bitmap_start_sync() into bitmap_operations")
> Cc: stable@vger.kernel.org # v6.12+
> Assisted-by: Claude-Code:claude-opus-4-8
> Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
> ---
> drivers/md/raid10.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 0a3cfdd3f5df..54cddb3a98cd 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -3365,7 +3365,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
> struct md_rdev *rdev = conf->mirrors[j].rdev;
>
> if (rdev == NULL || test_bit(Faulty, &rdev->flags)) {
> - still_degraded = false;
> + still_degraded = true;
> break;
> }
> }
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Kind regards,
Paul
^ permalink raw reply
* Re: [PATCH 2/3] md: only consult skip_sync_blocks when 'j' is in the bitmap's domain
From: yu kuai @ 2026-07-20 2:25 UTC (permalink / raw)
To: Mykola Marzhan, Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, Hannes Reinecke, linux-raid, linux-kernel
In-Reply-To: <20260719144227.940444-3-mykola@meshstor.io>
Hi,
在 2026/7/19 22:42, Mykola Marzhan 写道:
> Rebuilding a raid10 member with llbitmap can complete in a few
> hundred milliseconds having copied nothing: md_do_sync() offers
> every position 'j' to bitmap_ops->skip_sync_blocks(), the bitmap
> answers for the wrong chunks, the whole rebuild is skipped as
> "unwritten", and the array reports optimal with a stale member. The
> staleness is typically noticed only when the mirror partner fails;
> writes made while the array was degraded vanish.
>
> The bitmap spans [0, resync_max_sectors) in the personality's
> sync-cursor space. Resync walks exactly that range for every
> personality and may always consult the bitmap. Recovery walks
> per-device offsets, which match the bitmap's space only for raid1
> (device space is array space) and raid456 (the bitmap is indexed by
> the per-device sync cursor; bitmap_sector() translates array IO
> into it). raid10's bitmap is in array space -- near-2 on four
> disks spans twice a member's size; far-2 on two disks puts array
> chunk 2c+d at device chunk c of disk d -- so
> llbitmap_skip_sync_blocks() was consulted with offsets it cannot
> interpret.
>
> Only call the hook when the offsets match what the bitmap expects.
> Reshape must visit every stripe regardless of bitmap state --
> skipping would desync reshape_position from 'j' -- so never skip
> there. For recovery, add an md_personality capability,
> recovery_in_bitmap_space, set by raid1 and raid456. raid10 leaves
> it unset (raid10_find_virt() needs the disk number, which
> md_do_sync() does not have); raid0 and linear too, but no recovery
> path of theirs reaches the hook. No performance is lost against any
> released kernel: the hook is llbitmap-only and new in v6.18, so
> raid10 recovery returns to its earlier path -- walk every stripe,
> skip clean chunks inside raid10_sync_request().
>
> A capability rather than a geometry test: for raid10 layouts with
> raid_disks == near_copies * far_copies, resync_max_sectors equals
> dev_sectors while the spaces can still differ (far and offset
> variants), so an arithmetic proxy reopens the same hole on those
> layouts. Disabling the hook for all recovery instead would
> forfeit llbitmap's fast-forward over unwritten chunks for raid1 and
> raid456 rebuilds. Opt-in is also the safe default: a new
> personality gets no bitmap consultation during recovery until it
> declares otherwise.
>
> Verified for near-2 and far-2 layouts: a rebuild onto an added spare
> copies the writes made while the array was degraded.
>
> Fixes: f196d7288864 ("md/md-bitmap: add a new method skip_sync_blocks() in bitmap_operations")
> Cc: stable@vger.kernel.org # v6.18+
> Assisted-by: Claude-Code:claude-opus-4-8
> Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
> ---
> drivers/md/md.c | 13 ++++++++++++-
> drivers/md/md.h | 11 +++++++++++
> drivers/md/raid1.c | 1 +
> drivers/md/raid5.c | 3 +++
> 4 files changed, 27 insertions(+), 1 deletion(-)
There is already a patchset to support llbitmap reshape for raid10 and raid5:
[PATCH v2 00/20] md/md-llbitmap: support reshape for RAID10 and RAID5 -
Yu Kuai <https://lore.kernel.org/linux-raid/cover.1782282042.git.yukuai@kernel.org/>
Before this set, reshape for raid10 or raid5 is not safe. Kernel panic or data lost
is observed. Please check if your problems still exist with this set.
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index d1465bcd86c8..d60ea7aaca3a 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -9846,7 +9846,18 @@ void md_do_sync(struct md_thread *thread)
> if (test_bit(MD_RECOVERY_INTR, &mddev->recovery))
> break;
>
> - if (mddev->bitmap_ops && mddev->bitmap_ops->skip_sync_blocks) {
> + /*
> + * The bitmap may be consulted with 'j' for a plain resync,
> + * but for recovery only when the personality declares that
> + * its recovery cursor addresses the bitmap's space; raid10
> + * recovery walks per-device offsets the bitmap cannot
> + * interpret. Reshape must relocate every stripe regardless
> + * of bitmap state, so never skip there either.
> + */
> + if (mddev->bitmap_ops && mddev->bitmap_ops->skip_sync_blocks &&
> + !test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) &&
> + (!test_bit(MD_RECOVERY_RECOVER, &mddev->recovery) ||
> + mddev->pers->recovery_in_bitmap_space)) {
> sectors = mddev->bitmap_ops->skip_sync_blocks(mddev, j);
> if (sectors)
> goto update;
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index d8daf0f75cbb..a490a47736b0 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -798,6 +798,17 @@ struct md_personality
> /* convert io ranges from array to bitmap */
> void (*bitmap_sector)(struct mddev *mddev, sector_t *offset,
> unsigned long *sectors);
> + /*
> + * The position md_do_sync() walks during MD_RECOVERY_RECOVER
> + * addresses the space the write-intent bitmap is indexed by, so
> + * recovery may consult bitmap_ops->skip_sync_blocks() directly.
> + * True for raid1 (device space is array space) and raid456 (the
> + * bitmap is indexed by the per-device sync cursor). raid10 must
> + * leave this unset: its recovery walks per-device offsets while
> + * its bitmap is indexed by array sectors, and translating needs
> + * the disk number, which md_do_sync() does not have.
> + */
> + bool recovery_in_bitmap_space;
> };
>
> struct md_sysfs_entry {
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index afe2ca96ad8c..1222c0470ab6 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -3518,6 +3518,7 @@ static struct md_personality raid1_personality =
> .check_reshape = raid1_reshape,
> .quiesce = raid1_quiesce,
> .takeover = raid1_takeover,
> + .recovery_in_bitmap_space = true,
> };
>
> static int __init raid1_init(void)
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index ffb5fcde54a9..25f8d829a986 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -9078,6 +9078,7 @@ static struct md_personality raid6_personality =
> .change_consistency_policy = raid5_change_consistency_policy,
> .prepare_suspend = raid5_prepare_suspend,
> .bitmap_sector = raid5_bitmap_sector,
> + .recovery_in_bitmap_space = true,
> };
> static struct md_personality raid5_personality =
> {
> @@ -9108,6 +9109,7 @@ static struct md_personality raid5_personality =
> .change_consistency_policy = raid5_change_consistency_policy,
> .prepare_suspend = raid5_prepare_suspend,
> .bitmap_sector = raid5_bitmap_sector,
> + .recovery_in_bitmap_space = true,
> };
>
> static struct md_personality raid4_personality =
> @@ -9139,6 +9141,7 @@ static struct md_personality raid4_personality =
> .change_consistency_policy = raid5_change_consistency_policy,
> .prepare_suspend = raid5_prepare_suspend,
> .bitmap_sector = raid5_bitmap_sector,
> + .recovery_in_bitmap_space = true,
> };
>
> static int __init raid5_init(void)
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH 1/3] md/raid10: restore still_degraded detection in recovery
From: yu kuai @ 2026-07-20 2:17 UTC (permalink / raw)
To: Mykola Marzhan, Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, Hannes Reinecke, linux-raid, linux-kernel
In-Reply-To: <20260719144227.940444-2-mykola@meshstor.io>
在 2026/7/19 22:42, Mykola Marzhan 写道:
> raid10_sync_request()'s recovery arm passes still_degraded -- "will
> the array still be degraded after this recovery completes?" -- to
> md_bitmap_start_sync(), whose !degraded branch consumes the chunk's
> NEEDED marker, the write-intent record. That is only safe when the
> recovery restores full redundancy; otherwise the record still
> describes writes an absent member missed.
>
> Commit fe6a19d40ceb ("md/md-bitmap: merge md_bitmap_start_sync()
> into bitmap_operations") turned the detection loop's only assignment
> from "still_degraded = 1" into "still_degraded = false" in its
> int-to-bool styling pass, so the flag has been constant false ever
> since. raid1 and raid5 were converted to "= true"; only raid10 was
> inverted.
>
> The result is silent data loss on a classic-bitmap raid10 that stays
> degraded across a recovery. With mirror pairs {A,B} and {C,D}: lose
> D, keep writing, lose B, add a spare. The rebuild into B's slot
> strips NEEDED from every chunk -- including chunks whose only
> unsatisfied intent belonged to D -- so a later --re-add of D sees an
> empty intent record, completes instantly, and serves stale data.
>
> Restore the pre-conversion assignment. Only the classic-bitmap
> recovery arm is affected: llbitmap ignores the degraded parameter,
> and the resync arm passes mddev->degraded directly.
>
> Fixes: fe6a19d40ceb ("md/md-bitmap: merge md_bitmap_start_sync() into bitmap_operations")
> Cc:stable@vger.kernel.org # v6.12+
> Assisted-by: Claude-Code:claude-opus-4-8
> Signed-off-by: Mykola Marzhan<mykola@meshstor.io>
> ---
> drivers/md/raid10.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Yu Kuai <yukuai@fygo.io>
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH 4/4] md: include events counters when kicking non-fresh device
From: yu kuai @ 2026-07-20 2:14 UTC (permalink / raw)
To: Mykola Marzhan, Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <20260719144409.940492-5-mykola@meshstor.io>
Hi,
在 2026/7/19 22:44, Mykola Marzhan 写道:
> When analyze_sbs() kicks a member with a stale events counter, the
> warning names the device but not the divergence that caused the
> kick, so judging whether the kicked member is salvageable means
> running mdadm --examine on every member. Print the member's on-disk
> events counter and the freshest events counter in the warning
> itself.
If user space tools can do this already, then user space tools should be
considered preferred.
>
> rdev->sb_events cannot be used here: it is only assigned when a
> superblock is written and still reads 0 during assemble. Read the
> counter from the superblock page instead -- md_event() for 0.90
> metadata, the little-endian events field for 1.x.
>
> The legacy substring "kicking non-fresh %pg from array!" is
> unchanged, so existing log parsers continue to match.
>
> Assisted-by: Claude-Code:claude-opus-4-8
> Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
> ---
> drivers/md/md.c | 25 +++++++++++++++++++++++--
> 1 file changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index d60ea7aaca3a..9ec87375a717 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -3934,8 +3934,29 @@ static int analyze_sbs(struct mddev *mddev)
> if (rdev != freshest) {
> if (super_types[mddev->major_version].
> validate_super(mddev, freshest, rdev)) {
> - pr_warn("md: kicking non-fresh %pg from array!\n",
> - rdev->bdev);
> + u64 rdev_events = 0;
> +
> + /*
> + * rdev->sb_events is only assigned when
> + * writing a superblock and reads 0 during
> + * assemble; read events from the on-disk
> + * superblock instead.
> + */
> + if (rdev->sb_page) {
> + if (mddev->major_version == 1) {
> + struct mdp_superblock_1 *sb =
> + page_address(rdev->sb_page);
> + rdev_events = le64_to_cpu(sb->events);
> + } else if (mddev->major_version == 0) {
> + mdp_super_t *sb =
> + page_address(rdev->sb_page);
> + rdev_events = md_event(sb);
> + }
> + }
> + pr_warn("md: kicking non-fresh %pg from array! (events=%llu, freshest=%llu)\n",
> + rdev->bdev,
> + (unsigned long long)rdev_events,
> + (unsigned long long)mddev->events);
And I don't like the above changes. If you really want such log, please add
it inside validate_super() methods.
> md_kick_rdev_from_array(rdev);
> continue;
> }
--
Thanks,
Kuai
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox