Re: [PATCH v2 2/7] md/raid1: advertise atomic write limits and handle runtime constraints
From: John Garry
Date: Fri Jul 03 2026 - 04:29:31 EST
On 30/06/2026 09:39, Abd-Alrhman Masalkhi wrote:
Hi John,
On Mon, Jun 29, 2026 at 15:48 +0100, John Garry wrote:
On 28/06/2026 15:24, Abd-Alrhman Masalkhi wrote:
Atomic writes in RAID1 must fit within a single barrier unit. Advertise
this restriction through the queue limits by setting
atomic_write_hw_unit_max to BARRIER_UNIT_SECTOR_SIZE so that bios which
would cross a barrier-unit boundary are rejected by the block layer
before reaching MD.
A bio that passes block-layer validation may still become unserviceable
within RAID1 due to bad blocks or write-behind constraints. In the former
case, complete the bio with EIO. In the latter case, disable
write-behind rather than failing the bio with EIO.
Fixes: f2a38abf5f1c ("md/raid1: Atomic write support")
Fixes: a4c55c902670 ("md/raid1: simplify raid1_write_request() error handling")
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@xxxxxxxxx>
---
Changes in v2:
- Drop the early atomic write split check from raid1_write_request().
- Advertise the atomic write size limit via queue limits.
- Disable write-behind instead of failing atomic writes when the
BIO_MAX_VECS limit is encountered.
- Link to v1: https://urldefense.com/v3/__https://lore.kernel.org/linux-raid/20260623072456.333437-3-abd.masalkhi@xxxxxxxxx/__;!!ACWV5N9M2RV99hQ!LbMSGSClRi0PNBqQti5ZNWGDVjDd34-7saYEAwNyBNjpNTjEA7veqM5RHG8KB1QiscarW4UaIefjm19ywSImtIgh$
---
drivers/md/raid1.c | 36 +++++++++++++++++++-----------------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index afe2ca96ad8c..f322048ab3c2 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1522,6 +1522,7 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
int first_clone;
bool write_behind = false;
bool nowait = bio->bi_opf & REQ_NOWAIT;
+ bool atomic = bio->bi_opf & REQ_ATOMIC;
bool is_discard = op_is_discard(bio->bi_opf);
sector_t sector = bio->bi_iter.bi_sector;
@@ -1603,20 +1604,6 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
}
if (is_bad) {
int good_sectors;
-
- /*
- * We cannot atomically write this, so just
- * error in that case. It could be possible to
- * atomically write other mirrors, but the
- * complexity of supporting that is not worth
- * the benefit.
- */
- if (bio->bi_opf & REQ_ATOMIC) {
- bio->bi_status = BLK_STS_NOTSUPP;
- bio_endio(bio);
- goto err_dec_pending;
- }
-
good_sectors = first_bad - sector;
if (good_sectors < max_sectors)
max_sectors = good_sectors;
@@ -1633,10 +1620,24 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
* at a time and thus needs a new bio that can fit the whole payload
* this bio in page sized chunks.
*/
- if (write_behind && mddev->bitmap)
- max_sectors = min_t(int, max_sectors,
- BIO_MAX_VECS * (PAGE_SIZE >> 9));
+ if (write_behind && mddev->bitmap) {
+ if (atomic && max_sectors > BIO_MAX_VECS * (PAGE_SIZE >> 9))
where does BIO_MAX_VECS * (PAGE_SIZE >> 9) even come from?
BIO_MAX_VECS * (PAGE_SIZE >> 9) defines the maximum size supported by
write-behind. The write-behind copy (alloc_behind_master_bio) uses a
single bio, which can hold at most BIO_MAX_VECS pages, making this the
largest payload it can carry. With a 4 KiB PAGE_SIZE, that corresponds
to 256 pages, or 1 MiB (2048 sectors).
This patch changes the behavior for atomic writes that exceed this
limit. Instead of failing the write with -EIO when the number of sectors
must be reduced, it disables write-behind and proceeds with the atomic
write.
+ /*
+ * Atomic writes cannot be split, so disable
+ * write-behind.
+ */
+ write_behind = false;
It's a bit poor to have write_behind initially = false, then allow it to be set = true, and then later be set = false. Can this be improved?
+ else
+ max_sectors = min_t(int, max_sectors,
+ BIO_MAX_VECS * (PAGE_SIZE >> 9));
+ }
+
if (max_sectors < bio_sectors(bio)) {
+ if (atomic) {
+ bio_io_error(bio);
+ goto err_dec_pending;
+ }
+
bio = bio_submit_split_bioset(bio, max_sectors,
&conf->bio_split);
if (!bio)
@@ -3229,6 +3230,7 @@ static int raid1_set_limits(struct mddev *mddev)
lim.max_write_zeroes_sectors = 0;
lim.max_hw_wzeroes_unmap_sectors = 0;
lim.logical_block_size = mddev->logical_block_size;
+ lim.atomic_write_hw_unit_max = BARRIER_UNIT_SECTOR_SIZE;
This BARRIER_UNIT_SECTOR_SIZE is a bit like chunk sectors, no? I am just
wondering if we just should set it to chunk sectors =
BARRIER_UNIT_SECTOR_SIZE
I assume that it affects more than Reads and writes, e.g. discard also.
BARRIER_UNIT_SECTOR_SIZE is the resync barrier-bucket, not the layout
chunk size. Unless I'm missing something, using
atomic_write_hw_unit_max seems more appropriate than using the chunk
size. That way, the limit only applies to atomic writes instead of
affecting other operations such.
well it seems to be that everything in the driver is split over BARRIER_UNIT_SECTOR_SIZE, so is in effect a chunk size.
Note that atomic_write_hw_unit_max is going to be small always compared to BARRIER_UNIT_SECTOR_SIZE.
However, can you check the blk stacking code to make sure that this does as you want? As I remember, for stacking we take the atomic write limits of the first bottom device and then stack the other bottom devices and I don't think that setting atomic_write_hw_unit_max in this way has an impact - see blk_stack_atomic_writes_limits()
lim.features |= BLK_FEAT_ATOMIC_WRITES;
lim.features |= BLK_FEAT_PCI_P2PDMA;
err = mddev_stack_rdev_limits(mddev, &lim, MDDEV_STACK_INTEGRITY);