Re: [PATCH v2 2/7] md/raid1: advertise atomic write limits and handle runtime constraints

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 <[email protected]>
> ---
> 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/[email protected]/__;!!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?

> +			/*
> +			 * Atomic writes cannot be split, so disable
> +			 * write-behind.
> +			 */
> +			write_behind = false;
> +		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.

>   	lim.features |= BLK_FEAT_ATOMIC_WRITES;
>   	lim.features |= BLK_FEAT_PCI_P2PDMA;
>   	err = mddev_stack_rdev_limits(mddev, &lim, MDDEV_STACK_INTEGRITY);