Linux RAID subsystem development
 help / color / mirror / Atom feed
* [PATCH v6 3/3] md/raid10: free r10bio before ending master_bio in raid_end_bio_io() and raid_end_discard_bio()
From: Chen Cheng @ 2026-06-23 12:38 UTC (permalink / raw)
  To: linux-raid, yukuai, yukuai; +Cc: chencheng, linux-kernel
In-Reply-To: <20260623123840.2521340-1-chencheng@fnnas.com>

From: Chen Cheng <chencheng@fnnas.com>

origin flow:

      bio_endio(master_bio);   /* may drop active_io to zero */
      allow_barrier(conf);
      free_r10bio(r10_bio);    /* reads conf->geo, returns to pool */

one scenario is:

  CPU A (softirq, raid_end_bio_io)         CPU B (action_store) --> reshape
  ================================         ===============================
  bio_endio(master_bio)
    md_end_clone_io
      percpu_ref_put -> 0
                                           wait_event wakeup, and,
                                           	mddev_suspend return
                                           raid10_start_reshape:
                                             setup_geo(&conf->geo, new)
                                             ...
                                             mempool_destroy(old_pool)
                                             conf->r10bio_pool = new_pool
  allow_barrier(conf)
  free_r10bio(r10_bio)
    put_all_bios:
      for (i=0; i<conf->geo.raid_disks; i++)
          ==> old obj, new geo, OOB
    mempool_free(r10_bio, conf->r10bio_pool)
          ==> old-geometry obj freed into new pool

so .. fix by reorder the flow:

	free_r10bio(r10_bio)
	allow_barrier(conf)
	bio_endio(master_io)

raid_end_discard_bio() is exactly the same.

Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
 drivers/md/raid10.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index d740744a9746..e44a9b6e95c7 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -330,24 +330,25 @@ static void reschedule_retry(struct r10bio *r10_bio)
  */
 static void raid_end_bio_io(struct r10bio *r10_bio)
 {
 	struct bio *bio = r10_bio->master_bio;
 	struct r10conf *conf = r10_bio->mddev->private;
+	unsigned long state = r10_bio->state;
 
-	if (!test_and_set_bit(R10BIO_Returned, &r10_bio->state)) {
-		if (!test_bit(R10BIO_Uptodate, &r10_bio->state))
-			bio->bi_status = BLK_STS_IOERR;
-		bio_endio(bio);
-	}
+	free_r10bio(r10_bio);
 
 	/*
 	 * Wake up any possible resync thread that waits for the device
 	 * to go idle.
 	 */
 	allow_barrier(conf);
 
-	free_r10bio(r10_bio);
+	if (!test_and_set_bit(R10BIO_Returned, &state)) {
+		if (!test_bit(R10BIO_Uptodate, &state))
+			bio->bi_status = BLK_STS_IOERR;
+		bio_endio(bio);
+	}
 }
 
 /*
  * Update disk head position estimator based on IRQ completion info.
  */
@@ -1580,13 +1581,15 @@ static void raid_end_discard_bio(struct r10bio *r10bio)
 		if (!test_bit(R10BIO_Discard, &r10bio->state)) {
 			first_r10bio = (struct r10bio *)r10bio->master_bio;
 			free_r10bio(r10bio);
 			r10bio = first_r10bio;
 		} else {
+			struct bio *master_bio = r10bio->master_bio;
+
 			md_write_end(r10bio->mddev);
-			bio_endio(r10bio->master_bio);
 			free_r10bio(r10bio);
+			bio_endio(master_bio);
 			break;
 		}
 	}
 }
 
-- 
2.54.0

^ permalink raw reply related

* [PATCH v6 0/3] md/raid10: fix r10bio width mismatches across reshape
From: Chen Cheng @ 2026-06-23 12:38 UTC (permalink / raw)
  To: linux-raid, yukuai, yukuai; +Cc: chencheng, linux-kernel

From: Chen Cheng <chencheng@fnnas.com>

Hi,

This series fixes slab out-of-bounds accesses in raid10 when reshape changes
the number of raid disks while regular I/O is still reusing r10bio objects
allocated under the previous geometry.

The bug is reproducible with a simple 4-disk to 5-disk reshape under write
load, for example:

  mdadm -C /dev/md777 -l10 -n4 /dev/sda /dev/sdb /dev/sdc /dev/sdd
  mkfs.ext4 /dev/md777
  mount /dev/md777 /mnt/test
  fsstress -d /mnt/test -n 24000 -p 8 -l 24 &
  mdadm /dev/md777 --add /dev/sde
  mdadm --grow /dev/md777 --raid-devices=5 \
    --backup-file=/tmp/md-reshape-backup


KASAN report:

  BUG: KASAN: slab-out-of-bounds in free_r10bio+0x1c4/0x260 [raid10]
  Read of size 8 at addr ffff00008c2dfac8 by task ksoftirqd/0/15
  free_r10bio
  raid_end_bio_io
  one_write_done
  raid10_end_write_request


This series addresses the problem in three steps:

  1. ensure the sync_action=reshape caller suspends and locks before start_reshape

  2. resize r10bio_pool when reshape grows raid_disks

  3. reorder the r10bio free flow before bio_endio in the regular and discard
     completion paths


Changes in v6:
   - suspend the array in action_store() after flush_work()
   - free r10bio before ending the discard master bio

Changes in v5 (suggested by Yu Kuai):
   - simplify patch 2
   - switch patch 3 from bounding reused r10bio devs[] walks by used_nr_devs
     to reordering the free/endio flow

Changes in v4:
   - make the sync_action=reshape path invoke mddev_suspend_and_lock() before
     calling start_reshape()
   - leave the md-cluster and dm-raid paths unchanged; they still reach
     start_reshape() with the mddev locked but without suspend

Changes in v3:
   - replace freeze_array()/unfreeze_array() in raid10_start_reshape() with
     mddev_suspend_and_lock_nointr()/mddev_unlock_and_resume(); freeze_array()
     can return while retry-list items still hold pool objects, while
     mddev_suspend() provides the correct upper-layer quiesce interface

Changes in v2:
  - add this cover letter
  - convert r10bio_pool to a fixed-size kmalloc mempool
  - rebuild r10bio_pool inside the freeze window before switching live reshape
    geometry
  - switch raid10_quiesce() to freeze_array()/unfreeze_array()


Testing:
  - reproduced the original KASAN slab-out-of-bounds on 4-disk -> 5-disk
    raid10 reshape with fsstress
  - verified that this series fixes that reproducer
  - exercised the 5-disk -> 4-disk reshape direction as well

Thanks,
Chen Cheng



Chen Cheng (3):
  md: suspend array when sync_action=reshape
  md/raid10: resize r10bio_pool for reshape
  md/raid10: free r10bio before ending master_bio in raid_end_bio_io()
    and raid_end_discard_bio()

 drivers/md/md.c     | 17 +++++++++----
 drivers/md/raid10.c | 61 ++++++++++++++++++++++++++++++++-------------
 drivers/md/raid10.h |  2 +-
 3 files changed, 56 insertions(+), 24 deletions(-)

-- 
2.54.0

^ permalink raw reply

* [PATCH v6 2/3] md/raid10: resize r10bio_pool for reshape
From: Chen Cheng @ 2026-06-23 12:38 UTC (permalink / raw)
  To: linux-raid, yukuai, yukuai; +Cc: chencheng, linux-kernel
In-Reply-To: <20260623123840.2521340-1-chencheng@fnnas.com>

From: Chen Cheng <chencheng@fnnas.com>

When reshape grows raid_disks, the pool must also switch to new geometry
object size , and allocate a new geometry size pool and replace the old.

But not for shrinking reshape, because regular I/O can still use the
prev geo for sectors that have not crossed reshape_progress yet.

Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
 drivers/md/raid10.c | 44 +++++++++++++++++++++++++++++++++-----------
 drivers/md/raid10.h |  2 +-
 2 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index cee5a253a281..d740744a9746 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -101,14 +101,25 @@ static void end_reshape(struct r10conf *conf);
 static inline struct r10bio *get_resync_r10bio(struct bio *bio)
 {
 	return get_resync_pages(bio)->raid_bio;
 }
 
-static void * r10bio_pool_alloc(gfp_t gfp_flags, void *data)
+static inline int calc_r10bio_size(unsigned int raid_disks)
 {
-	struct r10conf *conf = data;
-	int size = offsetof(struct r10bio, devs[conf->geo.raid_disks]);
+	return offsetof(struct r10bio, devs[raid_disks]);
+}
+
+static mempool_t *create_r10bio_pool(unsigned int raid_disks)
+{
+	int size = calc_r10bio_size(raid_disks);
+
+	return mempool_create_kmalloc_pool(NR_RAID_BIOS, size);
+}
+
+static struct r10bio *alloc_r10bio(unsigned int raid_disks, gfp_t gfp_flags)
+{
+	int size = calc_r10bio_size(raid_disks);
 
 	/* allocate a r10bio with room for raid_disks entries in the
 	 * bios array */
 	return kzalloc(size, gfp_flags);
 }
@@ -135,11 +146,11 @@ static void * r10buf_pool_alloc(gfp_t gfp_flags, void *data)
 	struct bio *bio;
 	int j;
 	int nalloc, nalloc_rp;
 	struct resync_pages *rps;
 
-	r10_bio = r10bio_pool_alloc(gfp_flags, conf);
+	r10_bio = alloc_r10bio(conf->geo.raid_disks, gfp_flags);
 	if (!r10_bio)
 		return NULL;
 
 	if (test_bit(MD_RECOVERY_SYNC, &conf->mddev->recovery) ||
 	    test_bit(MD_RECOVERY_RESHAPE, &conf->mddev->recovery))
@@ -275,11 +286,11 @@ static void put_all_bios(struct r10conf *conf, struct r10bio *r10_bio)
 static void free_r10bio(struct r10bio *r10_bio)
 {
 	struct r10conf *conf = r10_bio->mddev->private;
 
 	put_all_bios(conf, r10_bio);
-	mempool_free(r10_bio, &conf->r10bio_pool);
+	mempool_free(r10_bio, conf->r10bio_pool);
 }
 
 static void put_buf(struct r10bio *r10_bio)
 {
 	struct r10conf *conf = r10_bio->mddev->private;
@@ -1537,11 +1548,11 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
 static void __make_request(struct mddev *mddev, struct bio *bio, int sectors)
 {
 	struct r10conf *conf = mddev->private;
 	struct r10bio *r10_bio;
 
-	r10_bio = mempool_alloc(&conf->r10bio_pool, GFP_NOIO);
+	r10_bio = mempool_alloc(conf->r10bio_pool, GFP_NOIO);
 
 	r10_bio->master_bio = bio;
 	r10_bio->sectors = sectors;
 
 	r10_bio->mddev = mddev;
@@ -1729,11 +1740,11 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
 		last_stripe_index *= geo->far_copies;
 	end_disk_offset = (bio_end & geo->chunk_mask) +
 				(last_stripe_index << geo->chunk_shift);
 
 retry_discard:
-	r10_bio = mempool_alloc(&conf->r10bio_pool, GFP_NOIO);
+	r10_bio = mempool_alloc(conf->r10bio_pool, GFP_NOIO);
 	r10_bio->mddev = mddev;
 	r10_bio->state = 0;
 	r10_bio->sectors = 0;
 	r10_bio->read_slot = -1;
 	memset(r10_bio->devs, 0, sizeof(r10_bio->devs[0]) * geo->raid_disks);
@@ -3830,11 +3841,11 @@ static int setup_geo(struct geom *geo, struct mddev *mddev, enum geo_type new)
 static void raid10_free_conf(struct r10conf *conf)
 {
 	if (!conf)
 		return;
 
-	mempool_exit(&conf->r10bio_pool);
+	mempool_destroy(conf->r10bio_pool);
 	kfree(conf->mirrors);
 	kfree(conf->mirrors_old);
 	kfree(conf->mirrors_new);
 	safe_put_page(conf->tmppage);
 	bioset_exit(&conf->bio_split);
@@ -3877,13 +3888,12 @@ static struct r10conf *setup_conf(struct mddev *mddev)
 	if (!conf->tmppage)
 		goto out;
 
 	conf->geo = geo;
 	conf->copies = copies;
-	err = mempool_init(&conf->r10bio_pool, NR_RAID_BIOS, r10bio_pool_alloc,
-			   rbio_pool_free, conf);
-	if (err)
+	conf->r10bio_pool = create_r10bio_pool(conf->geo.raid_disks);
+	if (!conf->r10bio_pool)
 		goto out;
 
 	err = bioset_init(&conf->bio_split, BIO_POOL_SIZE, 0, 0);
 	if (err)
 		goto out;
@@ -4373,10 +4383,11 @@ static int raid10_start_reshape(struct mddev *mddev)
 	struct geom new;
 	struct r10conf *conf = mddev->private;
 	struct md_rdev *rdev;
 	int spares = 0;
 	int ret;
+	mempool_t *new_pool = NULL;
 
 	if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery))
 		return -EBUSY;
 
 	if (setup_geo(&new, mddev, geo_start) != conf->copies)
@@ -4409,10 +4420,15 @@ static int raid10_start_reshape(struct mddev *mddev)
 
 	if (spares < mddev->delta_disks)
 		return -EINVAL;
 
 	conf->offset_diff = min_offset_diff;
+	if (mddev->delta_disks > 0) {
+		new_pool = create_r10bio_pool(new.raid_disks);
+		if (!new_pool)
+			return -ENOMEM;
+	}
 	spin_lock_irq(&conf->device_lock);
 	if (conf->mirrors_new) {
 		memcpy(conf->mirrors_new, conf->mirrors,
 		       sizeof(struct raid10_info)*conf->prev.raid_disks);
 		smp_mb();
@@ -4509,10 +4525,14 @@ static int raid10_start_reshape(struct mddev *mddev)
 	mddev->degraded = calc_degraded(conf);
 	spin_unlock_irq(&conf->device_lock);
 	mddev->raid_disks = conf->geo.raid_disks;
 	mddev->reshape_position = conf->reshape_progress;
 	set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
+	if (new_pool) {
+		mempool_destroy(conf->r10bio_pool);
+		conf->r10bio_pool = new_pool;
+	}
 
 	clear_bit(MD_RECOVERY_SYNC, &mddev->recovery);
 	clear_bit(MD_RECOVERY_CHECK, &mddev->recovery);
 	clear_bit(MD_RECOVERY_DONE, &mddev->recovery);
 	set_bit(MD_RECOVERY_RESHAPE, &mddev->recovery);
@@ -4531,10 +4551,12 @@ static int raid10_start_reshape(struct mddev *mddev)
 	smp_wmb();
 	conf->reshape_progress = MaxSector;
 	conf->reshape_safe = MaxSector;
 	mddev->reshape_position = MaxSector;
 	spin_unlock_irq(&conf->device_lock);
+	if (new_pool)
+		mempool_destroy(new_pool);
 	return ret;
 }
 
 /* Calculate the last device-address that could contain
  * any block from the chunk that includes the array-address 's'
diff --git a/drivers/md/raid10.h b/drivers/md/raid10.h
index ec79d87fb92f..b711626a5db7 100644
--- a/drivers/md/raid10.h
+++ b/drivers/md/raid10.h
@@ -85,11 +85,11 @@ struct r10conf {
 	int			have_replacement; /* There is at least one
 						   * replacement device.
 						   */
 	wait_queue_head_t	wait_barrier;
 
-	mempool_t		r10bio_pool;
+	mempool_t		*r10bio_pool;
 	mempool_t		r10buf_pool;
 	struct page		*tmppage;
 	struct bio_set		bio_split;
 
 	/* When taking over an array from a different personality, we store
-- 
2.54.0

^ permalink raw reply related

* Re: [PATCH 2/7] md/raid1: handle atomic writes that require splitting
From: John Garry @ 2026-06-23 11:38 UTC (permalink / raw)
  To: Abd-Alrhman Masalkhi, song, yukuai, magiclinan, xiao, axboe,
	martin.petersen
  Cc: linux-raid, linux-kernel
In-Reply-To: <m2mrwl1sha.fsf@gmail.com>

On 23/06/2026 11:06, Abd-Alrhman Masalkhi wrote:
> On Tue, Jun 23, 2026 at 10:20 +0100, John Garry wrote:
>> On 23/06/2026 09:58, Abd-Alrhman Masalkhi wrote:
>>> On Tue, Jun 23, 2026 at 09:11 +0100, John Garry wrote:
>>>> On 23/06/2026 08:24, Abd-Alrhman Masalkhi wrote:
>>>>> If a request already requires splitting when entering
>>>>> raid1_write_request(), the current code allows it to proceed until it
>>>>> eventually reaches the split path.
>>>>
>>>> The block layer should catch invalid atomic writes in
>>>> submit_bio_noacct() -> blk_validate_atomic_write_op_size() before we
>>>> even get as far as the md atomic write handling. Having the check in
>>>> bio_submit_split_bioset() is really just a fail-safe for the block layer
>>>> not catching invalid atomic writes or the atomic writes queue limits not
>>>> being properly calculated.
>>> The request size itself satisfies the currently advertised atomic write
>>> limits, so blk_validate_atomic_write_op_size() allows it. The problem
>>> is that RAID1 may further restrict atomic writes to a single barrier
>>> unit via align_to_barrier_unit_end(). Therefore a request that crosses
>>> a barrier-unit boundary can still reach raid1_write_request() with
>>> max_sectors < bio_sectors(bio).
>>>
>>> If the barrier-unit restriction should instead be advertised through the
>>> atomic write queue limits,
>>
>> It should. Any restrictions should be advertised up front. For the user
>> to issue an atomic write which is valid according to limits, then it
>> should succeed.
>>
> 
> I'll take a look at how best to expose that through the queue limits and
> rework this part accordingly. If there is already an existing mechanism
> you had in mind, I'd appreciate any pointers.

Any write must fit within BARRIER_UNIT_SECTOR_SIZE, right?

Since an atomic write must be naturally aligned, then I would expect 
that the atomic write max unit is limited by BARRIER_UNIT_SECTOR_SIZE.

> 
>>> then I agree the block layer could reject
>>> such requests earlier and the RAID1 entry check would become
>>> unnecessary.
>>>
>>> However, there are also cases where max_sectors is reduced later within
>>> raid1_write_request(), for example when bad blocks are present on some
>>> mirrors (or due to other RAID1-specific constraints such as write-behind
>>> limits). Those reductions depend on RAID1 runtime state and mirror
>>> health, so they are not readily visible to the block layer during atomic
>>> write validation. In those cases RAID1 still needs to detect that the
>>> atomic write can no longer be serviced as requested and fail it
>>> appropriately.
>>
>> Sure, and we do this. As I remember, we should return -EIO in this case.
>>
> 
> Right, and that's the main motivation for this patch. The original
> atomic write support already returned -EIO for one bad-block path, but
> there are other cases where max_sectors can be reduced (e.g. the
> first_bad <= sector path and write-behind limits)
> 
> After a4c55c902670, those cases can end up completing with EINVAL or
> NOTSUPP instead. This patch is intended to restore consistent -EIO.
> 

ok, but I could not check this as I did not recognize the baseline code.

>>>
>>>>
>>>>> Along the way, the bio may instead
>>>>> fail due to other conditions and return a different status, even though
>>>>> the request was invalid as an atomic write from the beginning.
>>>>>
>>
> 


^ permalink raw reply

* Re: [PATCH v2] md: use READ_ONCE() for lockless reads of sb_flags
From: sashiko-bot @ 2026-06-23 11:25 UTC (permalink / raw)
  To: Chen Cheng; +Cc: yukuai, linux-raid
In-Reply-To: <20260623111617.2500313-1-chencheng@fnnas.com>

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

Pre-existing issues:
- [Low] Lockless plain reads of `mddev->recovery` will cause KCSAN data race warnings, similar to the ones fixed for `sb_flags`.
--

commit f286787fa23a14cb6ec022eb99574f7a85f9957d
Author: Chen Cheng <chencheng@fnnas.com>

md: use READ_ONCE() for lockless reads of sb_flags

This patch adds READ_ONCE() annotations to lockless reads of mddev->sb_flags
across md, raid1, raid5, and raid10. It aims to fix KCSAN data race warnings
caused by concurrent set_bit() operations on sb_flags.

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 096bb64e87bd5..c5c50640b6849 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -6832,7 +6832,7 @@ int md_run(struct mddev *mddev)
>  		set_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
>  	set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);

[Severity: Low]
This is a pre-existing issue, but since this patch targets KCSAN warnings for
atomic bitmasks, could plain lockless reads of mddev->recovery trigger similar
data races?

There are places where mddev->recovery is read without locks and without
READ_ONCE(), potentially causing KCSAN noise against set_bit() updates:

drivers/md/md.c:md_sync_action() {
    ...
    unsigned long recovery = mddev->recovery;
    ...
}

drivers/md/md.c:set_bitmap_file() {
    ...
    if (mddev->recovery || mddev->sync_thread)
    ...
}

drivers/md/md.c:update_array_info() {
    ...
    if (mddev->recovery || mddev->sync_thread) {
    ...
}

Should READ_ONCE() also be applied to these reads of mddev->recovery to
provide a more complete fix?

> -	if (mddev->sb_flags)
> +	if (READ_ONCE(mddev->sb_flags))
>  		md_update_sb(mddev, 0);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623111617.2500313-1-chencheng@fnnas.com?part=1

^ permalink raw reply

* [PATCH v2] md: use READ_ONCE() for lockless reads of sb_flags
From: Chen Cheng @ 2026-06-23 11:16 UTC (permalink / raw)
  To: linux-raid, yukuai, yukuai; +Cc: chencheng, linux-kernel

From: Chen Cheng <chencheng@fnnas.com>

sb_flags is checked without a lock in md, raid1, raid5, and raid10.
KCSAN reports these reads as data races.

The write side uses atomic bit ops.
The read side still has plain loads in a few places.

Use READ_ONCE() for the lockless reads of sb_flags.


v1 -> v2:
        - Add lock-free read paths for other array levels.

KCSAN reports #1:
======================================

BUG: KCSAN: data-race in md_check_recovery / md_write_start

write (marked) to 0xffff8e39f897f030 of 8 bytes by task 248146 on cpu 8:
  md_write_start+0x5dd/0x910
  raid10_make_request+0x9b/0x1080
  md_handle_request+0x4a2/0xa40
  [........]

read to 0xffff8e39f897f030 of 8 bytes by task 250445 on cpu 11:
  md_check_recovery+0x574/0x900
  raid10d+0xb7/0x2950
  [........]

KCSAN reports #2:
======================================
BUG: KCSAN: data-race in md_check_recovery / md_write_start

write (marked) to 0xffff8e39e953f030 of 8 bytes by task 540091 on cpu 11:
  md_write_start+0x5dd/0x910
  raid1_make_request+0x141/0x1990
  [........]

read to 0xffff8e39e953f030 of 8 bytes by task 580822 on cpu 0:
  md_check_recovery+0x574/0x900
  raid1d+0xcc/0x3840
  [........]

value changed: 0x0000000000000002 -> 0x0000000000000006

KCSAN reports #3:
======================================
BUG: KCSAN: data-race in md_check_recovery / md_do_sync.cold

write (marked) to 0xffff8e39e9404030 of 8 bytes by task 492473 on cpu 6:
  md_do_sync.cold+0x3f6/0x1686
  [........]

read to 0xffff8e39e9404030 of 8 bytes by task 492402 on cpu 3:
  md_check_recovery+0x16d/0x900
  raid1d+0xcc/0x3840
  [........]

value changed: 0x0000000000000000 -> 0x0000000000000002

KCSAN reports #4:
======================================
BUG: KCSAN: data-race in md_do_sync.cold / raid5d

write (marked) to 0xffff8e39c35cb030 of 8 bytes by task 192196 on cpu 10:
  md_do_sync.cold+0x3f6/0x1686
  md_thread+0x15a/0x2d0
  [........]

read to 0xffff8e39c35cb030 of 8 bytes by task 190759 on cpu 5:
  raid5d+0x7f9/0xba0
  md_thread+0x15a/0x2d0
  [........]

value changed: 0x0000000000000000 -> 0x0000000000000002

Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
 drivers/md/md.c     | 8 ++++----
 drivers/md/raid1.c  | 2 +-
 drivers/md/raid10.c | 4 ++--
 drivers/md/raid5.c  | 7 ++++---
 4 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 096bb64e87bd..c5c50640b684 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6830,11 +6830,11 @@ int md_run(struct mddev *mddev)
 		 * via sysfs - until a lack of spares is confirmed.
 		 */
 		set_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
 	set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
 
-	if (mddev->sb_flags)
+	if (READ_ONCE(mddev->sb_flags))
 		md_update_sb(mddev, 0);
 
 	if (IS_ENABLED(CONFIG_MD_BITMAP) && !mddev->bitmap_info.file &&
 	    !mddev->bitmap_info.offset)
 		md_bitmap_set_none(mddev);
@@ -7024,11 +7024,11 @@ static void __md_stop_writes(struct mddev *mddev)
 			mddev->bitmap_ops->flush(mddev);
 	}
 
 	if (md_is_rdwr(mddev) &&
 	    ((!mddev->in_sync && !mddev_is_clustered(mddev)) ||
-	     mddev->sb_flags)) {
+	     READ_ONCE(mddev->sb_flags))) {
 		/* mark array as shutdown cleanly */
 		if (!mddev_is_clustered(mddev))
 			mddev->in_sync = 1;
 		md_update_sb(mddev, 1);
 	}
@@ -10294,11 +10294,11 @@ static bool md_should_do_recovery(struct mddev *mddev)
 	/*
 	 * MD_SB_CHANGE_PENDING indicates that the array is switching from clean to
 	 * active, and no action is needed for now.
 	 * All other MD_SB_* flags require to update the superblock.
 	 */
-	if (mddev->sb_flags & ~ (1<<MD_SB_CHANGE_PENDING))
+	if (READ_ONCE(mddev->sb_flags) & ~ (1<<MD_SB_CHANGE_PENDING))
 		return true;
 
 	/*
 	 * If the array is not using external metadata and there has been no data
 	 * written for some time, then the array's status needs to be set to
@@ -10423,11 +10423,11 @@ void md_check_recovery(struct mddev *mddev)
 			spin_lock(&mddev->lock);
 			set_in_sync(mddev);
 			spin_unlock(&mddev->lock);
 		}
 
-		if (mddev->sb_flags)
+		if (READ_ONCE(mddev->sb_flags))
 			md_update_sb(mddev, 0);
 
 		/*
 		 * Never start a new sync thread if MD_RECOVERY_RUNNING is
 		 * still set.
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 29b58583e381..bd6808656edb 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2738,11 +2738,11 @@ static void raid1d(struct md_thread *thread)
 			handle_read_error(conf, r1_bio);
 		else
 			WARN_ON_ONCE(1);
 
 		cond_resched();
-		if (mddev->sb_flags & ~(1<<MD_SB_CHANGE_PENDING))
+		if (READ_ONCE(mddev->sb_flags) & ~(1 << MD_SB_CHANGE_PENDING))
 			md_check_recovery(mddev);
 	}
 	blk_finish_plug(&plug);
 }
 
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index adaf9e432e25..3ffa5a19964d 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3030,11 +3030,11 @@ static void raid10d(struct md_thread *thread)
 			handle_read_error(mddev, r10_bio);
 		else
 			WARN_ON_ONCE(1);
 
 		cond_resched();
-		if (mddev->sb_flags & ~(1<<MD_SB_CHANGE_PENDING))
+		if (READ_ONCE(mddev->sb_flags) & ~(1 << MD_SB_CHANGE_PENDING))
 			md_check_recovery(mddev);
 	}
 	blk_finish_plug(&plug);
 }
 
@@ -4698,11 +4698,11 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr,
 		else
 			mddev->curr_resync_completed = conf->reshape_progress;
 		conf->reshape_checkpoint = jiffies;
 		set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
 		md_wakeup_thread(mddev->thread);
-		wait_event(mddev->sb_wait, mddev->sb_flags == 0 ||
+		wait_event(mddev->sb_wait, READ_ONCE(mddev->sb_flags) == 0 ||
 			   test_bit(MD_RECOVERY_INTR, &mddev->recovery));
 		if (test_bit(MD_RECOVERY_INTR, &mddev->recovery)) {
 			allow_barrier(conf);
 			return sectors_done;
 		}
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index ded6a69f7795..cb58b4353995 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -1219,11 +1219,11 @@ static void ops_run_io(struct stripe_head *sh, struct stripe_head_state *s)
 				break;
 
 			if (bad < 0) {
 				set_bit(BlockedBadBlocks, &rdev->flags);
 				if (!conf->mddev->external &&
-				    conf->mddev->sb_flags) {
+				    READ_ONCE(conf->mddev->sb_flags)) {
 					/* It is very unlikely, but we might
 					 * still need to write out the
 					 * bad block log - better give it
 					 * a chance*/
 					md_check_recovery(conf->mddev);
@@ -6469,11 +6469,11 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk
 					rdev->recovery_offset = sector_nr;
 
 		conf->reshape_checkpoint = jiffies;
 		set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
 		md_wakeup_thread(mddev->thread);
-		wait_event(mddev->sb_wait, mddev->sb_flags == 0 ||
+		wait_event(mddev->sb_wait, READ_ONCE(mddev->sb_flags) == 0 ||
 			   test_bit(MD_RECOVERY_INTR, &mddev->recovery));
 		if (test_bit(MD_RECOVERY_INTR, &mddev->recovery))
 			return 0;
 		spin_lock_irq(&conf->device_lock);
 		conf->reshape_safe = mddev->reshape_position;
@@ -6913,11 +6913,12 @@ static void raid5d(struct md_thread *thread)
 						   conf->temp_inactive_list);
 		if (!batch_size && !released)
 			break;
 		handled += batch_size;
 
-		if (mddev->sb_flags & ~(1 << MD_SB_CHANGE_PENDING)) {
+		if (READ_ONCE(mddev->sb_flags) &
+		    ~(1 << MD_SB_CHANGE_PENDING)) {
 			spin_unlock_irq(&conf->device_lock);
 			md_check_recovery(mddev);
 			spin_lock_irq(&conf->device_lock);
 		}
 	}
-- 
2.54.0

^ permalink raw reply related

* Re: [PATCH v5 1/3] md: suspend array when sync_action=reshape
From: Chen Cheng @ 2026-06-23 10:20 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: yukuai, linux-raid
In-Reply-To: <20260622122554.E43991F000E9@smtp.kernel.org>

在 2026/6/22 20:25, sashiko-bot@kernel.org 写道:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> 
> New issues:
> - [High] Unmatched mddev_resume() on mddev_lock() failure causes mddev->suspended underflow.
> - [High] The retry loop bypasses array re-suspension but leaves the suspended flag set, causing unprotected reshape and double-resume.
> - [High] Suspending the array before calling flush_work() creates a circular memory reclaim deadlock.
> 
> Pre-existing issues:
> - [High] Cross-task corruption of PF_MEMALLOC_NOIO flags when mddev_suspend() and mddev_resume() are interleaved.
> --
> 
> commit ec573079e939aa92bfb3fd89247846ae57883ef2
> Author: Chen Cheng <chencheng@fnnas.com>
> 
> md: suspend array when sync_action=reshape
> 
> This patch suspends the md array before starting a reshape operation to
> prevent new requests from allocating objects from the old pool during
> the transition. The array is then resumed on exit.
> 
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index 096bb64e87bd5..e139f36e30b92 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -5258,26 +5258,37 @@ static ssize_t
>>   action_store(struct mddev *mddev, const char *page, size_t len)
>>   {
>>   	int ret;
>> +	bool suspended = false;
>>   	enum sync_action action;
>>   
>>   	if (!mddev->pers || !mddev->pers->sync_request)
>>   		return -EINVAL;
>>   
>> +	action = md_sync_action_by_name(page);
>> +	if (action == ACTION_RESHAPE) {
>> +		ret = mddev_suspend(mddev, true);
> 
> [Severity: High]
> This is a pre-existing issue, but does calling mddev_suspend() and
> mddev_resume() from concurrent contexts risk corrupting the PF_MEMALLOC_NOIO
> task flags?
> 
> When mddev_suspend() is called, it stores the task-specific NOIO flag into
> the global mddev structure:
> 
> drivers/md/md.c:mddev_suspend() {
>      ...
>      mddev->noio_flag = memalloc_noio_save();
>      ...
> }
> 
> If two tasks concurrently enter mddev_suspend(), the second task increments
> the suspend counter without overwriting mddev->noio_flag. If the first task
> then resumes (e.g., on a lock failure) it decrements the counter but returns
> early because the array is still suspended:
> 
> drivers/md/md.c:__mddev_resume() {
>      if (mddev->suspended) {
>          mutex_unlock(&mddev->suspend_mutex);
>          return;
>      }
>      ...
>      memalloc_noio_restore(mddev->noio_flag);
> }
> 
> This prevents the first task from restoring its NOIO flag. When the second
> task later resumes and drops the counter to 0, it incorrectly restores the
> first task's flags onto its own context. Could this leave the first task
> stuck in NOIO mode indefinitely?
> 
>> +		if (ret)
>> +			return ret;
>> +		suspended = true;
>> +	}
>>   retry:
>>   	if (work_busy(&mddev->sync_work))
>>   		flush_work(&mddev->sync_work);
> 

Looks like a independent problem, I need to sort out the flow.

> [Severity: High]
> Can holding the array suspended while synchronously waiting for the worker
> thread expose the worker's memory allocations to a circular deadlock?
> 
> If md_start_sync() allocates memory with GFP_KERNEL in the kworker thread
> (for example, when calling md_register_thread()), it can trigger direct
> memory reclaim. Because the worker thread did not transition the array to
> suspended itself, it does not inherit the PF_MEMALLOC_NOIO flag.
> 
> Consequently, memory reclaim can issue I/O to the md device. Since the array
> is now suspended, md_handle_request() will block this I/O indefinitely. The
> worker thread hangs waiting for I/O, and action_store() hangs here in
> flush_work() waiting for the worker thread, permanently deadlocking the
> array. Is it safe to suspend the array before flushing the work?
> 
>>   
>>   	ret = mddev_lock(mddev);
>> -	if (ret)
>> +	if (ret) {
>> +		mddev_resume(mddev);
>>   		return ret;
>> +	}
> 

Seems suspend_and_lock() after flush_work() more reasonable.

> [Severity: High]
> Does this unconditional mddev_resume() risk underflowing the mddev->suspended
> counter if the array was not actually suspended?
> 
> If action_store() is called with an action other than ACTION_RESHAPE, the
> array is not suspended. If the subsequent mddev_lock() fails (e.g., if it is
> interrupted by a signal), the error path unconditionally calls mddev_resume().
> 
> Since the array was not suspended, __mddev_resume() decrements
> mddev->suspended from 0 to -1. Because mddev->suspended is a signed integer
> and -1 evaluates to true, subsequent calls to mddev_suspend() will see
> mddev->suspended != 0, increment it back to 0, and return immediately without
> actually pausing I/O. Could this permanently break the suspension mechanism
> for the array?
> 
>>   
>>   	if (work_busy(&mddev->sync_work)) {
>> -		mddev_unlock(mddev);
>> +		if (suspended)
>> +			mddev_unlock_and_resume(mddev);
>> +		else
>> +			mddev_unlock(mddev);
>>   		goto retry;
>>   	}
> 

sure, my mistake.

> [Severity: High]
> Will the retry loop bypass array re-suspension while leaving the local
> suspended flag set to true?
> 
> If work_busy(&mddev->sync_work) is true, the code unlocks and resumes the
> array, then jumps back to the retry label. However, the retry label is placed
> after the mddev_suspend() block.
> 
> The code will not re-suspend the array, but the local 'suspended' boolean
> remains true. As a result, the reshape operation proceeds on an active,
> unsuspended array. Furthermore, when the function exits, it will call
> mddev_unlock_and_resume() again because 'suspended' is true, leading to a
> double-resume and another mddev->suspended underflow.
> 
> [ ... ]
>

^ permalink raw reply

* Re: [PATCH 2/7] md/raid1: handle atomic writes that require splitting
From: Abd-Alrhman Masalkhi @ 2026-06-23 10:06 UTC (permalink / raw)
  To: John Garry, song, yukuai, magiclinan, xiao, axboe,
	martin.petersen
  Cc: linux-raid, linux-kernel
In-Reply-To: <6130d0cb-4cf8-4042-843e-98a9d8aa00c5@oracle.com>

On Tue, Jun 23, 2026 at 10:20 +0100, John Garry wrote:
> On 23/06/2026 09:58, Abd-Alrhman Masalkhi wrote:
>> On Tue, Jun 23, 2026 at 09:11 +0100, John Garry wrote:
>>> On 23/06/2026 08:24, Abd-Alrhman Masalkhi wrote:
>>>> If a request already requires splitting when entering
>>>> raid1_write_request(), the current code allows it to proceed until it
>>>> eventually reaches the split path.
>>>
>>> The block layer should catch invalid atomic writes in
>>> submit_bio_noacct() -> blk_validate_atomic_write_op_size() before we
>>> even get as far as the md atomic write handling. Having the check in
>>> bio_submit_split_bioset() is really just a fail-safe for the block layer
>>> not catching invalid atomic writes or the atomic writes queue limits not
>>> being properly calculated.
>> The request size itself satisfies the currently advertised atomic write
>> limits, so blk_validate_atomic_write_op_size() allows it. The problem
>> is that RAID1 may further restrict atomic writes to a single barrier
>> unit via align_to_barrier_unit_end(). Therefore a request that crosses
>> a barrier-unit boundary can still reach raid1_write_request() with
>> max_sectors < bio_sectors(bio).
>> 
>> If the barrier-unit restriction should instead be advertised through the
>> atomic write queue limits, 
>
> It should. Any restrictions should be advertised up front. For the user 
> to issue an atomic write which is valid according to limits, then it 
> should succeed.
>

I'll take a look at how best to expose that through the queue limits and
rework this part accordingly. If there is already an existing mechanism 
you had in mind, I'd appreciate any pointers.

>> then I agree the block layer could reject
>> such requests earlier and the RAID1 entry check would become
>> unnecessary.
>> 
>> However, there are also cases where max_sectors is reduced later within
>> raid1_write_request(), for example when bad blocks are present on some
>> mirrors (or due to other RAID1-specific constraints such as write-behind
>> limits). Those reductions depend on RAID1 runtime state and mirror
>> health, so they are not readily visible to the block layer during atomic
>> write validation. In those cases RAID1 still needs to detect that the
>> atomic write can no longer be serviced as requested and fail it
>> appropriately.
>
> Sure, and we do this. As I remember, we should return -EIO in this case.
>

Right, and that's the main motivation for this patch. The original
atomic write support already returned -EIO for one bad-block path, but
there are other cases where max_sectors can be reduced (e.g. the
first_bad <= sector path and write-behind limits)

After a4c55c902670, those cases can end up completing with EINVAL or
NOTSUPP instead. This patch is intended to restore consistent -EIO.

>> 
>>>
>>>> Along the way, the bio may instead
>>>> fail due to other conditions and return a different status, even though
>>>> the request was invalid as an atomic write from the beginning.
>>>>
>

-- 
Best Regards,
Abd-Alrhman

^ permalink raw reply

* Re: [PATCH v5 3/3] md/raid10: free r10bio before ending master_bio in raid_end_bio_io()
From: Chen Cheng @ 2026-06-23 10:02 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: yukuai, linux-raid
In-Reply-To: <20260622122902.F25CD1F00A3A@smtp.kernel.org>

在 2026/6/22 20:29, sashiko-bot@kernel.org 写道:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Incomplete fix: identical Use-After-Free and mempool corruption in `raid_end_discard_bio`.
> - [High] Memory leak of `r10_bio` on `REQ_NOWAIT` write requests during cluster resync or reshape.
> --
> 
> commit 088539a2820315440079f31df4aa792c8a6bc547
> Author: Chen Cheng <chencheng@fnnas.com>
> 
> md/raid10: free r10bio before ending master_bio in raid_end_bio_io()
> 
> This commit reorders the teardown sequence in raid_end_bio_io() to free
> r10_bio before calling bio_endio(). This prevents a race condition where
> a concurrent array reshape can alter the geometry and destroy the mempool
> after the master bio completes but before the r10_bio is freed.
> 
>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>> index d740744a97461..a4642c903b202 100644
>> --- a/drivers/md/raid10.c
>> +++ b/drivers/md/raid10.c
>> @@ -332,12 +332,12 @@ static void raid_end_bio_io(struct r10bio *r10_bio)
>>   {
>>   	struct bio *bio = r10_bio->master_bio;
>>   	struct r10conf *conf = r10_bio->mddev->private;
>> +	bool returned = test_and_set_bit(R10BIO_Returned, &r10_bio->state);
>> +	blk_status_t status = test_bit(R10BIO_Uptodate, &r10_bio->state)
>> +				? BLK_STS_OK : BLK_STS_IOERR;
>>   
>> -	if (!test_and_set_bit(R10BIO_Returned, &r10_bio->state)) {
>> -		if (!test_bit(R10BIO_Uptodate, &r10_bio->state))
>> -			bio->bi_status = BLK_STS_IOERR;
>> -		bio_endio(bio);
>> -	}
>> +	put_all_bios(conf, r10_bio);
>> +	mempool_free(r10_bio, conf->r10bio_pool);
>>   
>>   	/*
>>   	 * Wake up any possible resync thread that waits for the device
>> @@ -345,7 +345,10 @@ static void raid_end_bio_io(struct r10bio *r10_bio)
>>   	 */
>>   	allow_barrier(conf);
>>   
>> -	free_r10bio(r10_bio);
>> +	if (!returned) {
>> +		bio->bi_status = status;
>> +		bio_endio(bio);
>> +	}
>>   }
>>   
> 
> [Severity: High]
> This is a pre-existing issue, but the exact same race condition appears to
> exist in the discard completion path.
> 
> In drivers/md/raid10.c:raid_end_discard_bio():
> 
> 	} else {
> 		md_write_end(r10bio->mddev);
> 		bio_endio(r10bio->master_bio);
> 		free_r10bio(r10bio);
> 		break;
> 	}
> 
> Since bio_endio(r10bio->master_bio) drops active_io to zero, couldn't this
> also wake up mddev_suspend() and allow a concurrent reshape to modify
> conf->geo and destroy conf->r10bio_pool before free_r10bio() is called?
> 

yes . raid_end_discard_bio have same issue .

> 
> [Severity: High]
> This is a pre-existing issue, but does raid10_write_request() leak the r10_bio
> on REQ_NOWAIT requests?
> 
> In drivers/md/raid10.c:__make_request():
> 
> 	r10_bio = mempool_alloc(conf->r10bio_pool, GFP_NOIO);
> ...
> 	if (bio_data_dir(bio) == READ)
> 		raid10_read_request(mddev, bio, r10_bio);
> 	else
> 		raid10_write_request(mddev, bio, r10_bio);
> 
> Then in raid10_write_request() during clustered resync:
> 
> 	if ((mddev_is_clustered(mddev) &&
> 	     mddev->cluster_ops->area_resyncing(mddev, WRITE,
> 						bio->bi_iter.bi_sector,
> 						bio_end_sector(bio)))) {
> 		DEFINE_WAIT(w);
> 		/* Bail out if REQ_NOWAIT is set for the bio */
> 		if (bio->bi_opf & REQ_NOWAIT) {
> 			bio_wouldblock_error(bio);
> 			return;
> 		}
> 
> And later during reshape:
> 
> 		if (bio->bi_opf & REQ_NOWAIT) {
> 			allow_barrier(conf);
> 			bio_wouldblock_error(bio);
> 			return;
> 		}
> 
> Since these paths return immediately, they leave the r10_bio allocated. Could
> repeated REQ_NOWAIT rejections eventually exhaust conf->r10bio_pool, causing
> subsequent allocations in __make_request() to hang indefinitely?
> 

seems fixed recently .

^ permalink raw reply

* Re: [PATCH 2/7] md/raid1: handle atomic writes that require splitting
From: John Garry @ 2026-06-23  9:20 UTC (permalink / raw)
  To: Abd-Alrhman Masalkhi, song, yukuai, magiclinan, xiao, axboe,
	martin.petersen
  Cc: linux-raid, linux-kernel
In-Reply-To: <m2se6d1vls.fsf@gmail.com>

On 23/06/2026 09:58, Abd-Alrhman Masalkhi wrote:
> On Tue, Jun 23, 2026 at 09:11 +0100, John Garry wrote:
>> On 23/06/2026 08:24, Abd-Alrhman Masalkhi wrote:
>>> If a request already requires splitting when entering
>>> raid1_write_request(), the current code allows it to proceed until it
>>> eventually reaches the split path.
>>
>> The block layer should catch invalid atomic writes in
>> submit_bio_noacct() -> blk_validate_atomic_write_op_size() before we
>> even get as far as the md atomic write handling. Having the check in
>> bio_submit_split_bioset() is really just a fail-safe for the block layer
>> not catching invalid atomic writes or the atomic writes queue limits not
>> being properly calculated.
> The request size itself satisfies the currently advertised atomic write
> limits, so blk_validate_atomic_write_op_size() allows it. The problem
> is that RAID1 may further restrict atomic writes to a single barrier
> unit via align_to_barrier_unit_end(). Therefore a request that crosses
> a barrier-unit boundary can still reach raid1_write_request() with
> max_sectors < bio_sectors(bio).
> 
> If the barrier-unit restriction should instead be advertised through the
> atomic write queue limits, 

It should. Any restrictions should be advertised up front. For the user 
to issue an atomic write which is valid according to limits, then it 
should succeed.

> then I agree the block layer could reject
> such requests earlier and the RAID1 entry check would become
> unnecessary.
> 
> However, there are also cases where max_sectors is reduced later within
> raid1_write_request(), for example when bad blocks are present on some
> mirrors (or due to other RAID1-specific constraints such as write-behind
> limits). Those reductions depend on RAID1 runtime state and mirror
> health, so they are not readily visible to the block layer during atomic
> write validation. In those cases RAID1 still needs to detect that the
> atomic write can no longer be serviced as requested and fail it
> appropriately.

Sure, and we do this. As I remember, we should return -EIO in this case.

> 
>>
>>> Along the way, the bio may instead
>>> fail due to other conditions and return a different status, even though
>>> the request was invalid as an atomic write from the beginning.
>>>


^ permalink raw reply

* [GIT PULL] md-7.2-20260623
From: Yu Kuai @ 2026-06-23  9:03 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-raid, linux-block, Abd-Alrhman Masalkhi, Chen Cheng

Hi Jens,

Please consider pulling the following changes into your block-7.2
branch.

This pull request contains:

Bug Fixes:
- Fix raid1 writes_pending and barrier reference leaks on write failures.
  (Abd-Alrhman Masalkhi)
- Fix raid10 writes_pending leak on write request failures.
  (Abd-Alrhman Masalkhi)
- Fix raid10 writes_pending and barrier reference leaks on discard failures.
  (Abd-Alrhman Masalkhi)
- Fix raid1 REQ_NOWAIT handling while waiting for behind writes.
  (Abd-Alrhman Masalkhi)
- Fix raid1 r1_bio leak when a REQ_NOWAIT retry would block.
  (Abd-Alrhman Masalkhi)
- Fix raid1 read-balance head_position data race. (Chen Cheng)
- Fix raid5 stripe batch bm_seq wraparound comparison. (Chen Cheng)
- Fix raid5 stripe batch state snapshot KCSAN noise. (Chen Cheng)
- Fix raid5 R5_Overlap races while breaking stripe batches. (Chen Cheng)

Improvements:
- Add raid5 discard IO accounting. (Yu Kuai)
- Always convert raid5 llbitmap bits for discard. (Yu Kuai)

Cleanups:
- Simplify raid1_write_request() error handling. (Abd-Alrhman Masalkhi)

Thanks,
Kuai

---

The following changes since commit d5b58fbb2fd7ac25fcd7e1c14730f998a90b0322:

  block: respect iov_iter::nofault flag in bio_iov_iter_bounce_write() (2026-06-16 14:48:35 -0600)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/mdraid/linux.git md-7.2

for you to fetch changes up to 55b77337bdd088c77461588e5ec094421b89911b:

  md/raid5: avoid R5_Overlap races while breaking stripe batches (2026-06-23 09:44:11 +0800)

----------------------------------------------------------------
Abd-Alrhman Masalkhi (6):
      md/raid1: fix writes_pending and barrier reference leaks on write failures
      md/raid10: fix writes_pending leak on write request failures
      md/raid10: fix writes_pending and barrier reference leaks on discard failures
      md/raid1: simplify raid1_write_request() error handling
      md/raid1: honor REQ_NOWAIT when waiting for behind writes
      md/raid1: free r1_bio when REQ_NOWAIT is set and read would block on retry

Chen Cheng (4):
      md/raid1: protect head_position for read balance
      md/raid5: let stripe batch bm_seq comparison wrap-safe
      md/raid5: use stripe state snapshot in break_stripe_batch_list()
      md/raid5: avoid R5_Overlap races while breaking stripe batches

Yu Kuai (3):
      md/raid5: account discard IO
      md/raid5: validate discard support at request time
      md/raid5: always convert llbitmap bits for discard

 drivers/md/md-bitmap.c   |   9 +++-
 drivers/md/md-bitmap.h   |   2 +-
 drivers/md/md-llbitmap.c |  13 +++--
 drivers/md/md.c          |   2 +-
 drivers/md/raid1.c       |  99 ++++++++++++++++++++---------------
 drivers/md/raid10.c      |  28 +++++++---
 drivers/md/raid5.c       | 131 +++++++++++++++++++++++++++++------------------
 drivers/md/raid5.h       |   1 +
 8 files changed, 178 insertions(+), 107 deletions(-)

^ permalink raw reply

* Re: [PATCH 2/7] md/raid1: handle atomic writes that require splitting
From: Abd-Alrhman Masalkhi @ 2026-06-23  8:58 UTC (permalink / raw)
  To: John Garry, song, yukuai, magiclinan, xiao, axboe,
	martin.petersen
  Cc: linux-raid, linux-kernel
In-Reply-To: <ba67f3ef-45cb-41c0-b4ea-fa0a22508cdc@oracle.com>

On Tue, Jun 23, 2026 at 09:11 +0100, John Garry wrote:
> On 23/06/2026 08:24, Abd-Alrhman Masalkhi wrote:
>> If a request already requires splitting when entering
>> raid1_write_request(), the current code allows it to proceed until it
>> eventually reaches the split path. 
>
> The block layer should catch invalid atomic writes in 
> submit_bio_noacct() -> blk_validate_atomic_write_op_size() before we 
> even get as far as the md atomic write handling. Having the check in 
> bio_submit_split_bioset() is really just a fail-safe for the block layer 
> not catching invalid atomic writes or the atomic writes queue limits not 
> being properly calculated.
The request size itself satisfies the currently advertised atomic write
limits, so blk_validate_atomic_write_op_size() allows it. The problem
is that RAID1 may further restrict atomic writes to a single barrier
unit via align_to_barrier_unit_end(). Therefore a request that crosses
a barrier-unit boundary can still reach raid1_write_request() with
max_sectors < bio_sectors(bio).

If the barrier-unit restriction should instead be advertised through the
atomic write queue limits, then I agree the block layer could reject
such requests earlier and the RAID1 entry check would become
unnecessary.

However, there are also cases where max_sectors is reduced later within
raid1_write_request(), for example when bad blocks are present on some
mirrors (or due to other RAID1-specific constraints such as write-behind
limits). Those reductions depend on RAID1 runtime state and mirror
health, so they are not readily visible to the block layer during atomic
write validation. In those cases RAID1 still needs to detect that the
atomic write can no longer be serviced as requested and fail it
appropriately.

>
>> Along the way, the bio may instead
>> fail due to other conditions and return a different status, even though
>> the request was invalid as an atomic write from the beginning.
>> 
>> Additionally, an otherwise valid atomic write may later require
>> splitting because bad blocks reduce the writable range or because
>> write-behind constraints reduce the maximum writable size. In these
>> cases, the bio currently completes with either EINVAL or ENOTSUPP,
>> whereas it should complete with EIO instead.
>> 
>> 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@gmail.com>
>> ---
>>   drivers/md/raid1.c | 25 +++++++++++--------------
>>   1 file changed, 11 insertions(+), 14 deletions(-)
>> 
>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>> index 86d4f224ffb1..8386d37343a4 100644
>> --- a/drivers/md/raid1.c
>> +++ b/drivers/md/raid1.c
>> @@ -1511,9 +1511,15 @@ 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;
>>   
>> +	if (atomic && max_sectors != bio_sectors(bio)) {
>> +		bio_endio_status(bio, BLK_STS_INVAL);
>> +		return false;
>> +	}
>> +
>>   	if (mddev_is_clustered(mddev) &&
>>   	    mddev->cluster_ops->area_resyncing(mddev, WRITE, sector,
>>   					       bio_end_sector(bio))) {
>> @@ -1592,20 +1598,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;
>
> what baseline are you using here? This looks different to linux-next 22 
> june and linus' master branch
>
I'm basing this series on Song's md tree, specifically the md-7.2
branch.

>> -					bio_endio(bio);
>> -					goto err_dec_pending;
>> -				}
>> -
>>   				good_sectors = first_bad - sector;
>>   				if (good_sectors < max_sectors)
>>   					max_sectors = good_sectors;
>> @@ -1626,6 +1618,11 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
>>   		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)
>

-- 
Best Regards,
Abd-Alrhman

^ permalink raw reply

* Re: [PATCH] md: use READ_ONCE() for lockless reads of sb_flags
From: Chen Cheng @ 2026-06-23  8:44 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: linux-raid, yukuai
In-Reply-To: <20260623082915.1B8A71F000E9@smtp.kernel.org>

在 2026/6/23 16:29, sashiko-bot@kernel.org 写道:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Incomplete fix: The patch claims to resolve KCSAN data races on `sb_flags` in md, raid1, raid5, and raid10, but completely omits the necessary `READ_ONCE()` updates to the `raid1`, `raid5`, and `raid10` subsystems.
> --
> 
> commit adcaa558925c24a70a377065ea735febac939f0b
> Author: Chen Cheng <chencheng@fnnas.com>
> 
> md: use READ_ONCE() for lockless reads of sb_flags
> 
> This commit adds READ_ONCE() wrappers around lockless reads of mddev->sb_flags
> to prevent KCSAN data race warnings. The commit message states it addresses
> issues across md, raid1, raid5, and raid10.
> 
>>      sb_flags is checked without a lock in md, raid1, raid5, and raid10.
>>      KCSAN reports these reads as data races.
>>      
>>      The write side uses atomic bit ops.
>>      The read side still has plain loads in a few places.
>>      
>>      Use READ_ONCE() for the lockless reads of sb_flags.
> 
> [Severity: Medium]
> Does this patch completely address the data races mentioned in the commit
> message?
> 

There isn't data races in strong consistency model arch..
raidXd() would be call periodically, so , steal value is ok..
But I see kcsan report, and I think use read_once to read would be better.

> The commit message indicates that sb_flags is checked without a lock in md,
> raid1, raid5, and raid10, and includes KCSAN traces for raid1d, raid5d, and
> raid10d. However, the provided diff only includes changes to drivers/md/md.c.
> 
> Are there additional READ_ONCE() updates missing for the raid1, raid5, and
> raid10 subsystems? For example:
> - drivers/md/raid1.c:raid1d()
> - drivers/md/raid5.c:raid5d(), ops_run_io(), and reshape_request()
> - drivers/md/raid10.c:raid10d() and reshape_request()
> 

raidXd() all call md_check_recovery().

>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index 096bb64e87bd5..f83cca895f85f 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
> [ ... ]
>> @@ -10425,7 +10425,7 @@ void md_check_recovery(struct mddev *mddev)
>>   			spin_unlock(&mddev->lock);
>>   		}
>>   
>> -		if (mddev->sb_flags)
>> +		if (READ_ONCE(mddev->sb_flags))
>>   			md_update_sb(mddev, 0);
>>   
>>   		/*
> 
> [Severity: Medium]
> Are there other plain reads of sb_flags remaining in this file?
> 
> Looking at drivers/md/md.c:__md_stop_writes(), there appears to be another
> plain lockless read of mddev->sb_flags. Should that also be wrapped with
> READ_ONCE() to fully eliminate the KCSAN warnings?
> 

Only add protect for lockless path. __md_stop_writes() already protected 
by mddev_lock.

^ permalink raw reply

* Re: [PATCH] md: use READ_ONCE() for lockless reads of sb_flags
From: sashiko-bot @ 2026-06-23  8:29 UTC (permalink / raw)
  To: Chen Cheng; +Cc: linux-raid, yukuai
In-Reply-To: <20260623081942.2488045-1-chencheng@fnnas.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete fix: The patch claims to resolve KCSAN data races on `sb_flags` in md, raid1, raid5, and raid10, but completely omits the necessary `READ_ONCE()` updates to the `raid1`, `raid5`, and `raid10` subsystems.
--

commit adcaa558925c24a70a377065ea735febac939f0b
Author: Chen Cheng <chencheng@fnnas.com>

md: use READ_ONCE() for lockless reads of sb_flags

This commit adds READ_ONCE() wrappers around lockless reads of mddev->sb_flags
to prevent KCSAN data race warnings. The commit message states it addresses
issues across md, raid1, raid5, and raid10.

>     sb_flags is checked without a lock in md, raid1, raid5, and raid10.
>     KCSAN reports these reads as data races.
>     
>     The write side uses atomic bit ops.
>     The read side still has plain loads in a few places.
>     
>     Use READ_ONCE() for the lockless reads of sb_flags.

[Severity: Medium]
Does this patch completely address the data races mentioned in the commit
message?

The commit message indicates that sb_flags is checked without a lock in md,
raid1, raid5, and raid10, and includes KCSAN traces for raid1d, raid5d, and
raid10d. However, the provided diff only includes changes to drivers/md/md.c.

Are there additional READ_ONCE() updates missing for the raid1, raid5, and
raid10 subsystems? For example:
- drivers/md/raid1.c:raid1d()
- drivers/md/raid5.c:raid5d(), ops_run_io(), and reshape_request()
- drivers/md/raid10.c:raid10d() and reshape_request()

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 096bb64e87bd5..f83cca895f85f 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
[ ... ]
> @@ -10425,7 +10425,7 @@ void md_check_recovery(struct mddev *mddev)
>  			spin_unlock(&mddev->lock);
>  		}
>  
> -		if (mddev->sb_flags)
> +		if (READ_ONCE(mddev->sb_flags))
>  			md_update_sb(mddev, 0);
>  
>  		/*

[Severity: Medium]
Are there other plain reads of sb_flags remaining in this file?

Looking at drivers/md/md.c:__md_stop_writes(), there appears to be another
plain lockless read of mddev->sb_flags. Should that also be wrapped with
READ_ONCE() to fully eliminate the KCSAN warnings?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623081942.2488045-1-chencheng@fnnas.com?part=1

^ permalink raw reply

* [PATCH] md: use READ_ONCE() for lockless reads of sb_flags
From: Chen Cheng @ 2026-06-23  8:19 UTC (permalink / raw)
  To: linux-raid, yukuai, yukuai; +Cc: chencheng, linux-kernel

From: Chen Cheng <chencheng@fnnas.com>

sb_flags is checked without a lock in md, raid1, raid5, and raid10.
KCSAN reports these reads as data races.

The write side uses atomic bit ops.
The read side still has plain loads in a few places.

Use READ_ONCE() for the lockless reads of sb_flags.

KCSAN reports #1:
======================================

BUG: KCSAN: data-race in md_check_recovery / md_write_start

write (marked) to 0xffff8e39f897f030 of 8 bytes by task 248146 on cpu 8:
  md_write_start+0x5dd/0x910
  raid10_make_request+0x9b/0x1080
  md_handle_request+0x4a2/0xa40
  [........]

read to 0xffff8e39f897f030 of 8 bytes by task 250445 on cpu 11:
  md_check_recovery+0x574/0x900
  raid10d+0xb7/0x2950
  [........]

KCSAN reports #2:
======================================
BUG: KCSAN: data-race in md_check_recovery / md_write_start

write (marked) to 0xffff8e39e953f030 of 8 bytes by task 540091 on cpu 11:
  md_write_start+0x5dd/0x910
  raid1_make_request+0x141/0x1990
  [........]

read to 0xffff8e39e953f030 of 8 bytes by task 580822 on cpu 0:
  md_check_recovery+0x574/0x900
  raid1d+0xcc/0x3840
  [........]

value changed: 0x0000000000000002 -> 0x0000000000000006

KCSAN reports #3:
======================================
BUG: KCSAN: data-race in md_check_recovery / md_do_sync.cold

write (marked) to 0xffff8e39e9404030 of 8 bytes by task 492473 on cpu 6:
  md_do_sync.cold+0x3f6/0x1686
  [........]

read to 0xffff8e39e9404030 of 8 bytes by task 492402 on cpu 3:
  md_check_recovery+0x16d/0x900
  raid1d+0xcc/0x3840
  [........]

value changed: 0x0000000000000000 -> 0x0000000000000002

KCSAN reports #4:
======================================
BUG: KCSAN: data-race in md_do_sync.cold / raid5d

write (marked) to 0xffff8e39c35cb030 of 8 bytes by task 192196 on cpu 10:
  md_do_sync.cold+0x3f6/0x1686
  md_thread+0x15a/0x2d0
  [........]

read to 0xffff8e39c35cb030 of 8 bytes by task 190759 on cpu 5:
  raid5d+0x7f9/0xba0
  md_thread+0x15a/0x2d0
  [........]

value changed: 0x0000000000000000 -> 0x0000000000000002

Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
 drivers/md/md.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 096bb64e87bd..f83cca895f85 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6830,11 +6830,11 @@ int md_run(struct mddev *mddev)
 		 * via sysfs - until a lack of spares is confirmed.
 		 */
 		set_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
 	set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
 
-	if (mddev->sb_flags)
+	if (READ_ONCE(mddev->sb_flags))
 		md_update_sb(mddev, 0);
 
 	if (IS_ENABLED(CONFIG_MD_BITMAP) && !mddev->bitmap_info.file &&
 	    !mddev->bitmap_info.offset)
 		md_bitmap_set_none(mddev);
@@ -10294,11 +10294,11 @@ static bool md_should_do_recovery(struct mddev *mddev)
 	/*
 	 * MD_SB_CHANGE_PENDING indicates that the array is switching from clean to
 	 * active, and no action is needed for now.
 	 * All other MD_SB_* flags require to update the superblock.
 	 */
-	if (mddev->sb_flags & ~ (1<<MD_SB_CHANGE_PENDING))
+	if (READ_ONCE(mddev->sb_flags) & ~ (1<<MD_SB_CHANGE_PENDING))
 		return true;
 
 	/*
 	 * If the array is not using external metadata and there has been no data
 	 * written for some time, then the array's status needs to be set to
@@ -10423,11 +10423,11 @@ void md_check_recovery(struct mddev *mddev)
 			spin_lock(&mddev->lock);
 			set_in_sync(mddev);
 			spin_unlock(&mddev->lock);
 		}
 
-		if (mddev->sb_flags)
+		if (READ_ONCE(mddev->sb_flags))
 			md_update_sb(mddev, 0);
 
 		/*
 		 * Never start a new sync thread if MD_RECOVERY_RUNNING is
 		 * still set.
-- 
2.54.0

^ permalink raw reply related

* Re: [PATCH 2/7] md/raid1: handle atomic writes that require splitting
From: John Garry @ 2026-06-23  8:11 UTC (permalink / raw)
  To: Abd-Alrhman Masalkhi, song, yukuai, magiclinan, xiao, axboe,
	martin.petersen
  Cc: linux-raid, linux-kernel
In-Reply-To: <20260623072456.333437-3-abd.masalkhi@gmail.com>

On 23/06/2026 08:24, Abd-Alrhman Masalkhi wrote:
> If a request already requires splitting when entering
> raid1_write_request(), the current code allows it to proceed until it
> eventually reaches the split path. 

The block layer should catch invalid atomic writes in 
submit_bio_noacct() -> blk_validate_atomic_write_op_size() before we 
even get as far as the md atomic write handling. Having the check in 
bio_submit_split_bioset() is really just a fail-safe for the block layer 
not catching invalid atomic writes or the atomic writes queue limits not 
being properly calculated.

> Along the way, the bio may instead
> fail due to other conditions and return a different status, even though
> the request was invalid as an atomic write from the beginning.
> 
> Additionally, an otherwise valid atomic write may later require
> splitting because bad blocks reduce the writable range or because
> write-behind constraints reduce the maximum writable size. In these
> cases, the bio currently completes with either EINVAL or ENOTSUPP,
> whereas it should complete with EIO instead.
> 
> 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@gmail.com>
> ---
>   drivers/md/raid1.c | 25 +++++++++++--------------
>   1 file changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 86d4f224ffb1..8386d37343a4 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -1511,9 +1511,15 @@ 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;
>   
> +	if (atomic && max_sectors != bio_sectors(bio)) {
> +		bio_endio_status(bio, BLK_STS_INVAL);
> +		return false;
> +	}
> +
>   	if (mddev_is_clustered(mddev) &&
>   	    mddev->cluster_ops->area_resyncing(mddev, WRITE, sector,
>   					       bio_end_sector(bio))) {
> @@ -1592,20 +1598,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;

what baseline are you using here? This looks different to linux-next 22 
june and linus' master branch

> -					bio_endio(bio);
> -					goto err_dec_pending;
> -				}
> -
>   				good_sectors = first_bad - sector;
>   				if (good_sectors < max_sectors)
>   					max_sectors = good_sectors;
> @@ -1626,6 +1618,11 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
>   		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)


^ permalink raw reply

* [PATCH] md/raid1: protect sequential read hints for read balance
From: Chen Cheng @ 2026-06-23  7:59 UTC (permalink / raw)
  To: linux-raid, yukuai, yukuai; +Cc: chencheng, linux-kernel

From: Chen Cheng <chencheng@fnnas.com>

The patch just suppress KCSAN noise. No functional change.

KCSAN reports a race, point to update_read_sectors() update next_seq_sect vs.
read next_seq_sect.

Protect next_seq_sect and seq_start with READ_ONCE/WRITE_ONCE, otherwise,
read balance see stale sequential-read hints.

KCSAN report:
==============
 BUG: KCSAN: data-race in raid1_read_request / raid1_read_request

 write to 0xffff8e3a2d6736d0 of 8 bytes by task 593784 on cpu 10:
  raid1_read_request+0xe5a/0x19f0
  raid1_make_request+0xdf/0x1990
  md_handle_request+0x4a2/0xa40
  [...]

 read to 0xffff8e3a2d6736d0 of 8 bytes by task 593776 on cpu 11:
  raid1_read_request+0xe3f/0x19f0
  raid1_make_request+0xdf/0x1990
  md_handle_request+0x4a2/0xa40
  [...]

 value changed: 0x0000000000356368 -> 0x0000000000356370

Signed-off-by: Chen Cheng <chencheng@fnnas.com>
---
 drivers/md/raid1.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 4cdf4484cab6..29b58583e381 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -602,13 +602,13 @@ static void update_read_sectors(struct r1conf *conf, int disk,
 				sector_t this_sector, int len)
 {
 	struct raid1_info *info = &conf->mirrors[disk];
 
 	atomic_inc(&info->rdev->nr_pending);
-	if (info->next_seq_sect != this_sector)
-		info->seq_start = this_sector;
-	info->next_seq_sect = this_sector + len;
+	if (READ_ONCE(info->next_seq_sect) != this_sector)
+		WRITE_ONCE(info->seq_start, this_sector);
+	WRITE_ONCE(info->next_seq_sect, this_sector + len);
 }
 
 static int choose_first_rdev(struct r1conf *conf, struct r1bio *r1_bio,
 			     int *max_sectors)
 {
@@ -733,31 +733,33 @@ static int choose_slow_rdev(struct r1conf *conf, struct r1bio *r1_bio,
 	return bb_disk;
 }
 
 static bool is_sequential(struct r1conf *conf, int disk, struct r1bio *r1_bio)
 {
-	/* TODO: address issues with this check and concurrency. */
-	return conf->mirrors[disk].next_seq_sect == r1_bio->sector ||
+	return READ_ONCE(conf->mirrors[disk].next_seq_sect) == r1_bio->sector ||
 	       READ_ONCE(conf->mirrors[disk].head_position) == r1_bio->sector;
 }
 
 /*
  * If buffered sequential IO size exceeds optimal iosize, check if there is idle
  * disk. If yes, choose the idle disk.
  */
 static bool should_choose_next(struct r1conf *conf, int disk)
 {
 	struct raid1_info *mirror = &conf->mirrors[disk];
+	sector_t seq_start, next_seq_sect;
 	int opt_iosize;
 
 	if (!test_bit(Nonrot, &mirror->rdev->flags))
 		return false;
 
 	opt_iosize = bdev_io_opt(mirror->rdev->bdev) >> 9;
-	return opt_iosize > 0 && mirror->seq_start != MaxSector &&
-	       mirror->next_seq_sect > opt_iosize &&
-	       mirror->next_seq_sect - opt_iosize >= mirror->seq_start;
+	seq_start = READ_ONCE(mirror->seq_start);
+	next_seq_sect = READ_ONCE(mirror->next_seq_sect);
+	return opt_iosize > 0 && seq_start != MaxSector &&
+	       next_seq_sect > opt_iosize &&
+	       next_seq_sect - opt_iosize >= seq_start;
 }
 
 static bool rdev_readable(struct md_rdev *rdev, struct r1bio *r1_bio)
 {
 	if (!rdev || test_bit(Faulty, &rdev->flags))
-- 
2.54.0

^ permalink raw reply related

* [PATCH 7/7] md/raid10: simplify read request error handling
From: Abd-Alrhman Masalkhi @ 2026-06-23  7:24 UTC (permalink / raw)
  To: song, yukuai, magiclinan, xiao, axboe, john.g.garry,
	martin.petersen, abd.masalkhi
  Cc: linux-raid, linux-kernel
In-Reply-To: <20260623072456.333437-1-abd.masalkhi@gmail.com>

raid10_read_request() currently handles bio completion, barrier
handling, and r10_bio lifetime management in several different error
paths. This results in duplicated cleanup logic and increases the risk
of introducing bugs in future modifications.

Make raid10_read_request() return a status to its callers, consolidate
the read error paths, and free r10_bio from a single location in the
callers. Since the callers allocate r10_bio, they should also be
responsible for freeing it when the request fails.

This makes the read path follow the same ownership model as the write
path and simplifies the error handling flow.

Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
 drivers/md/raid10.c | 45 +++++++++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 2de898733337..830c0fe30b96 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1143,7 +1143,7 @@ static bool regular_request_wait(struct mddev *mddev, struct r10conf *conf,
 	return true;
 }
 
-static void raid10_read_request(struct mddev *mddev, struct bio *bio,
+static bool raid10_read_request(struct mddev *mddev, struct bio *bio,
 				struct r10bio *r10_bio)
 {
 	struct r10conf *conf = mddev->private;
@@ -1191,8 +1191,7 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
 
 	if (!regular_request_wait(mddev, conf, bio, r10_bio->sectors)) {
 		bio_wouldblock_error(bio);
-		free_r10bio(r10_bio);
-		return;
+		return false;
 	}
 
 	rdev = read_balance(conf, r10_bio, &max_sectors);
@@ -1202,8 +1201,8 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
 					    mdname(mddev), b,
 					    (unsigned long long)r10_bio->sector);
 		}
-		raid_end_bio_io(r10_bio);
-		return;
+		bio_io_error(bio);
+		goto err_allow_barrier;
 	}
 	if (err_rdev)
 		pr_err_ratelimited("md/raid10:%s: %pg: redirecting sector %llu to another mirror\n",
@@ -1215,10 +1214,8 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
 		bio = bio_submit_split_bioset(bio, max_sectors,
 					      &conf->bio_split);
 		wait_barrier(conf, false);
-		if (!bio) {
-			set_bit(R10BIO_Returned, &r10_bio->state);
-			goto err_handle;
-		}
+		if (!bio)
+			goto err_dec_pending;
 
 		r10_bio->master_bio = bio;
 		r10_bio->sectors = max_sectors;
@@ -1244,10 +1241,16 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
 	read_bio->bi_private = r10_bio;
 	mddev_trace_remap(mddev, read_bio, r10_bio->sector);
 	submit_bio_noacct(read_bio);
-	return;
-err_handle:
+
+	return true;
+
+err_dec_pending:
 	atomic_dec(&rdev->nr_pending);
-	raid_end_bio_io(r10_bio);
+
+err_allow_barrier:
+	allow_barrier(conf);
+
+	return false;
 }
 
 static void raid10_write_one_disk(struct mddev *mddev, struct r10bio *r10_bio,
@@ -1543,14 +1546,13 @@ static bool __make_request(struct mddev *mddev, struct bio *bio, int sectors)
 	memset(r10_bio->devs, 0, sizeof(r10_bio->devs[0]) *
 			conf->geo.raid_disks);
 
-	ret = true;
 	if (bio_data_dir(bio) == READ)
-		raid10_read_request(mddev, bio, r10_bio);
-	else {
+		ret = raid10_read_request(mddev, bio, r10_bio);
+	else
 		ret = raid10_write_request(mddev, bio, r10_bio);
-		if (!ret)
-			free_r10bio(r10_bio);
-	}
+
+	if (!ret)
+		free_r10bio(r10_bio);
 
 	return ret;
 }
@@ -1880,6 +1882,7 @@ static bool raid10_make_request(struct mddev *mddev, struct bio *bio)
 	sector_t chunk_mask = (conf->geo.chunk_mask & conf->prev.chunk_mask);
 	int chunk_sects = chunk_mask + 1;
 	int sectors = bio_sectors(bio);
+	bool write = bio_data_dir(bi) == WRITE;
 
 	if (unlikely(bio->bi_opf & REQ_PREFLUSH)
 	    && md_flush_request(mddev, bio))
@@ -1903,7 +1906,7 @@ static bool raid10_make_request(struct mddev *mddev, struct bio *bio)
 		sectors = chunk_sects -
 			(bio->bi_iter.bi_sector &
 			 (chunk_sects - 1));
-	if (!__make_request(mddev, bio, sectors))
+	if (!__make_request(mddev, bio, sectors) && write)
 		md_write_end(mddev);
 
 	/* In case raid10d snuck in to freeze_array */
@@ -2871,7 +2874,9 @@ static void handle_read_error(struct mddev *mddev, struct r10bio *r10_bio)
 
 	rdev_dec_pending(rdev, mddev);
 	r10_bio->state = 0;
-	raid10_read_request(mddev, r10_bio->master_bio, r10_bio);
+	if (!raid10_read_request(mddev, r10_bio->master_bio, r10_bio))
+		free_r10bio(r10_bio);
+
 	/*
 	 * allow_barrier after re-submit to ensure no sync io
 	 * can be issued while regular io pending.
-- 
2.43.0


^ permalink raw reply related

* [PATCH 6/7] md/raid10: simplify write request error handling
From: Abd-Alrhman Masalkhi @ 2026-06-23  7:24 UTC (permalink / raw)
  To: song, yukuai, magiclinan, xiao, axboe, john.g.garry,
	martin.petersen, abd.masalkhi
  Cc: linux-raid, linux-kernel
In-Reply-To: <20260623072456.333437-1-abd.masalkhi@gmail.com>

raid10_write_request() currently handles bio completion, barrier
handling, and r10_bio lifetime management in several different error
paths. This results in duplicated cleanup logic and increases the risk
of introducing bugs in future modifications.

Move bio_wouldblock_error() handling to the callers of
regular_request_wait(), consolidate the write error paths, and free
r10_bio from a single location in __make_request() when
raid10_write_request() fails.

It remove redundant local copies of r10_bio->sectors and use a single
max_sectors variable throughout the function.

Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
 drivers/md/raid10.c | 61 +++++++++++++++++++++------------------------
 1 file changed, 29 insertions(+), 32 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 7085fd97b98a..2de898733337 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1123,18 +1123,16 @@ static bool regular_request_wait(struct mddev *mddev, struct r10conf *conf,
 				 struct bio *bio, sector_t sectors)
 {
 	/* Bail out if REQ_NOWAIT is set for the bio */
-	if (!wait_barrier(conf, bio->bi_opf & REQ_NOWAIT)) {
-		bio_wouldblock_error(bio);
+	if (!wait_barrier(conf, bio->bi_opf & REQ_NOWAIT))
 		return false;
-	}
+
 	while (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) &&
 	    bio->bi_iter.bi_sector < conf->reshape_progress &&
 	    bio->bi_iter.bi_sector + sectors > conf->reshape_progress) {
 		allow_barrier(conf);
-		if (bio->bi_opf & REQ_NOWAIT) {
-			bio_wouldblock_error(bio);
+		if (bio->bi_opf & REQ_NOWAIT)
 			return false;
-		}
+
 		mddev_add_trace_msg(conf->mddev, "raid10 wait reshape");
 		wait_event(conf->wait_barrier,
 			   conf->reshape_progress <= bio->bi_iter.bi_sector ||
@@ -1192,6 +1190,7 @@ static void raid10_read_request(struct mddev *mddev, struct bio *bio,
 	}
 
 	if (!regular_request_wait(mddev, conf, bio, r10_bio->sectors)) {
+		bio_wouldblock_error(bio);
 		free_r10bio(r10_bio);
 		return;
 	}
@@ -1354,13 +1353,12 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
 {
 	struct r10conf *conf = mddev->private;
 	int i, k;
-	sector_t sectors;
-	int max_sectors;
+	int max_sectors = r10_bio->sectors;
+	bool nowait = bio->bi_opf & REQ_NOWAIT;
 	bool atomic = bio->bi_opf & REQ_ATOMIC;
 
-	if (atomic && r10_bio->sectors != bio_sectors(bio)) {
+	if (atomic && max_sectors != bio_sectors(bio)) {
 		bio_endio_status(bio, BLK_STS_INVAL);
-		free_r10bio(r10_bio);
 		return false;
 	}
 
@@ -1369,9 +1367,8 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
 						bio->bi_iter.bi_sector,
 						bio_end_sector(bio)))) {
 		/* Bail out if REQ_NOWAIT is set for the bio */
-		if (bio->bi_opf & REQ_NOWAIT) {
+		if (nowait) {
 			bio_wouldblock_error(bio);
-			free_r10bio(r10_bio);
 			return false;
 		}
 
@@ -1381,28 +1378,25 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
 								    bio_end_sector(bio)));
 	}
 
-	sectors = r10_bio->sectors;
-	if (!regular_request_wait(mddev, conf, bio, sectors)) {
-		free_r10bio(r10_bio);
+	if (!regular_request_wait(mddev, conf, bio, max_sectors)) {
+		bio_wouldblock_error(bio);
 		return false;
 	}
 
 	if (test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) &&
 	    (mddev->reshape_backwards
 	     ? (bio->bi_iter.bi_sector < conf->reshape_safe &&
-		bio->bi_iter.bi_sector + sectors > conf->reshape_progress)
-	     : (bio->bi_iter.bi_sector + sectors > conf->reshape_safe &&
+		bio->bi_iter.bi_sector + max_sectors > conf->reshape_progress)
+	     : (bio->bi_iter.bi_sector + max_sectors > conf->reshape_safe &&
 		bio->bi_iter.bi_sector < conf->reshape_progress))) {
 		/* Need to update reshape_position in metadata */
 		mddev->reshape_position = conf->reshape_progress;
 		set_mask_bits(&mddev->sb_flags, 0,
 			      BIT(MD_SB_CHANGE_DEVS) | BIT(MD_SB_CHANGE_PENDING));
 		md_wakeup_thread(mddev->thread);
-		if (bio->bi_opf & REQ_NOWAIT) {
-			allow_barrier(conf);
+		if (nowait) {
 			bio_wouldblock_error(bio);
-			free_r10bio(r10_bio);
-			return false;
+			goto err_allow_barrier;
 		}
 		mddev_add_trace_msg(conf->mddev,
 			"raid10 wait reshape metadata");
@@ -1427,8 +1421,6 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
 
 	wait_blocked_dev(mddev, r10_bio);
 
-	max_sectors = r10_bio->sectors;
-
 	for (i = 0;  i < conf->copies; i++) {
 		int d = r10_bio->devs[i].devnum;
 		struct md_rdev *rdev, *rrdev;
@@ -1485,15 +1477,15 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
 		r10_bio->sectors = max_sectors;
 
 	if (r10_bio->sectors < bio_sectors(bio)) {
-		if (atomic)
-			goto err_handle;
+		if (atomic) {
+			bio_io_error(bio);
+			goto err_dec_pending;
+		}
 
 		bio = bio_submit_split_bioset(bio, r10_bio->sectors,
 					      &conf->bio_split);
-		if (!bio) {
-			set_bit(R10BIO_Returned, &r10_bio->state);
-			goto err_handle;
-		}
+		if (!bio)
+			goto err_dec_pending;
 
 		r10_bio->master_bio = bio;
 	}
@@ -1511,7 +1503,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
 	one_write_done(r10_bio);
 	return true;
 
-err_handle:
+err_dec_pending:
 	for (k = 0;  k < i; k++) {
 		int d = r10_bio->devs[k].devnum;
 		struct md_rdev *rdev = conf->mirrors[d].rdev;
@@ -1527,7 +1519,9 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
 		}
 	}
 
-	raid_end_bio_io(r10_bio);
+err_allow_barrier:
+	allow_barrier(conf);
+
 	return false;
 }
 
@@ -1552,8 +1546,11 @@ static bool __make_request(struct mddev *mddev, struct bio *bio, int sectors)
 	ret = true;
 	if (bio_data_dir(bio) == READ)
 		raid10_read_request(mddev, bio, r10_bio);
-	else
+	else {
 		ret = raid10_write_request(mddev, bio, r10_bio);
+		if (!ret)
+			free_r10bio(r10_bio);
+	}
 
 	return ret;
 }
-- 
2.43.0


^ permalink raw reply related

* [PATCH 5/7] md/raid10: replace wait loop with wait_event_idle()
From: Abd-Alrhman Masalkhi @ 2026-06-23  7:24 UTC (permalink / raw)
  To: song, yukuai, magiclinan, xiao, axboe, john.g.garry,
	martin.petersen, abd.masalkhi
  Cc: linux-raid, linux-kernel
In-Reply-To: <20260623072456.333437-1-abd.masalkhi@gmail.com>

The wait loop is equivalent to wait_event_idle() and can be simplified
by usaing it for improving readability.

Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
 drivers/md/raid10.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 4bc1d5553ec7..7085fd97b98a 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1368,22 +1368,17 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
 	     mddev->cluster_ops->area_resyncing(mddev, WRITE,
 						bio->bi_iter.bi_sector,
 						bio_end_sector(bio)))) {
-		DEFINE_WAIT(w);
 		/* Bail out if REQ_NOWAIT is set for the bio */
 		if (bio->bi_opf & REQ_NOWAIT) {
 			bio_wouldblock_error(bio);
 			free_r10bio(r10_bio);
 			return false;
 		}
-		for (;;) {
-			prepare_to_wait(&conf->wait_barrier,
-					&w, TASK_IDLE);
-			if (!mddev->cluster_ops->area_resyncing(mddev, WRITE,
-				 bio->bi_iter.bi_sector, bio_end_sector(bio)))
-				break;
-			schedule();
-		}
-		finish_wait(&conf->wait_barrier, &w);
+
+		wait_event_idle(conf->wait_barrier,
+				!mddev->cluster_ops->area_resyncing(mddev, WRITE,
+								    bio->bi_iter.bi_sector,
+								    bio_end_sector(bio)));
 	}
 
 	sectors = r10_bio->sectors;
-- 
2.43.0


^ permalink raw reply related

* [PATCH 4/7] md/raid10: raid10_write_request() drops the barrier before calling
From: Abd-Alrhman Masalkhi @ 2026-06-23  7:24 UTC (permalink / raw)
  To: song, yukuai, magiclinan, xiao, axboe, john.g.garry,
	martin.petersen, abd.masalkhi
  Cc: linux-raid, linux-kernel
In-Reply-To: <20260623072456.333437-1-abd.masalkhi@gmail.com>

bio_submit_split_bioset() and reacquires it afterwards. This is
unnecessary because bio_submit_split_bioset() does not require
releasing the barrier protection.

Remove the redundant allow_barrier()/wait_barrier() pair around
bio_submit_split_bioset().

Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
 drivers/md/raid10.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 840f0446c231..4bc1d5553ec7 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1493,10 +1493,8 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
 		if (atomic)
 			goto err_handle;
 
-		allow_barrier(conf);
 		bio = bio_submit_split_bioset(bio, r10_bio->sectors,
 					      &conf->bio_split);
-		wait_barrier(conf, false);
 		if (!bio) {
 			set_bit(R10BIO_Returned, &r10_bio->state);
 			goto err_handle;
-- 
2.43.0


^ permalink raw reply related

* [PATCH 3/7] md/raid10: handle atomic writes that require splitting
From: Abd-Alrhman Masalkhi @ 2026-06-23  7:24 UTC (permalink / raw)
  To: song, yukuai, magiclinan, xiao, axboe, john.g.garry,
	martin.petersen, abd.masalkhi
  Cc: linux-raid, linux-kernel
In-Reply-To: <20260623072456.333437-1-abd.masalkhi@gmail.com>

If a request already requires splitting when entering
raid10_write_request(), the current code allows it to proceed until it
eventually reaches the split path. Along the way, the bio may instead
fail due to other conditions and return a different status, even though
the request was invalid as an atomic write from the beginning.

Additionally, an otherwise valid atomic write may later require
splitting because bad blocks reduce the writable range. In this case,
the bio currently completes with either EINVAL or EIO, whereas it should
complete with EIO consistently.

Fixes: a1d9b4fd42d9 ("md/raid10: Atomic write support")
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
 drivers/md/raid10.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index bd322eccdc3f..840f0446c231 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1356,6 +1356,13 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
 	int i, k;
 	sector_t sectors;
 	int max_sectors;
+	bool atomic = bio->bi_opf & REQ_ATOMIC;
+
+	if (atomic && r10_bio->sectors != bio_sectors(bio)) {
+		bio_endio_status(bio, BLK_STS_INVAL);
+		free_r10bio(r10_bio);
+		return false;
+	}
 
 	if ((mddev_is_clustered(mddev) &&
 	     mddev->cluster_ops->area_resyncing(mddev, WRITE,
@@ -1464,16 +1471,6 @@ static bool raid10_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)
-					goto err_handle;
-
 				good_sectors = first_bad - dev_sector;
 				if (good_sectors < max_sectors)
 					max_sectors = good_sectors;
@@ -1493,6 +1490,9 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
 		r10_bio->sectors = max_sectors;
 
 	if (r10_bio->sectors < bio_sectors(bio)) {
+		if (atomic)
+			goto err_handle;
+
 		allow_barrier(conf);
 		bio = bio_submit_split_bioset(bio, r10_bio->sectors,
 					      &conf->bio_split);
-- 
2.43.0


^ permalink raw reply related

* [PATCH 2/7] md/raid1: handle atomic writes that require splitting
From: Abd-Alrhman Masalkhi @ 2026-06-23  7:24 UTC (permalink / raw)
  To: song, yukuai, magiclinan, xiao, axboe, john.g.garry,
	martin.petersen, abd.masalkhi
  Cc: linux-raid, linux-kernel
In-Reply-To: <20260623072456.333437-1-abd.masalkhi@gmail.com>

If a request already requires splitting when entering
raid1_write_request(), the current code allows it to proceed until it
eventually reaches the split path. Along the way, the bio may instead
fail due to other conditions and return a different status, even though
the request was invalid as an atomic write from the beginning.

Additionally, an otherwise valid atomic write may later require
splitting because bad blocks reduce the writable range or because
write-behind constraints reduce the maximum writable size. In these
cases, the bio currently completes with either EINVAL or ENOTSUPP,
whereas it should complete with EIO instead.

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@gmail.com>
---
 drivers/md/raid1.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 86d4f224ffb1..8386d37343a4 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1511,9 +1511,15 @@ 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;
 
+	if (atomic && max_sectors != bio_sectors(bio)) {
+		bio_endio_status(bio, BLK_STS_INVAL);
+		return false;
+	}
+
 	if (mddev_is_clustered(mddev) &&
 	    mddev->cluster_ops->area_resyncing(mddev, WRITE, sector,
 					       bio_end_sector(bio))) {
@@ -1592,20 +1598,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;
@@ -1626,6 +1618,11 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
 		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)
-- 
2.43.0


^ permalink raw reply related

* [PATCH 1/7] md/raid10: fix r10bio leak in raid10_write_request() error paths
From: Abd-Alrhman Masalkhi @ 2026-06-23  7:24 UTC (permalink / raw)
  To: song, yukuai, magiclinan, xiao, axboe, john.g.garry,
	martin.petersen, abd.masalkhi
  Cc: linux-raid, linux-kernel, sashiko-bot
In-Reply-To: <20260623072456.333437-1-abd.masalkhi@gmail.com>

When raid10_write_request() fails because REQ_NOWAIT is set, the
allocated r10_bio is not freed before returning, resulting in a memory
leak. Free r10_bio before returning from the REQ_NOWAIT error paths.

Fixes: c9aa889b035f ("md: raid10 add nowait support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/linux-raid/20260613184042.BCEC01F000E9@smtp.kernel.org/
Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
---
 drivers/md/raid10.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 0a3cfdd3f5df..bd322eccdc3f 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1365,6 +1365,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
 		/* Bail out if REQ_NOWAIT is set for the bio */
 		if (bio->bi_opf & REQ_NOWAIT) {
 			bio_wouldblock_error(bio);
+			free_r10bio(r10_bio);
 			return false;
 		}
 		for (;;) {
@@ -1398,6 +1399,7 @@ static bool raid10_write_request(struct mddev *mddev, struct bio *bio,
 		if (bio->bi_opf & REQ_NOWAIT) {
 			allow_barrier(conf);
 			bio_wouldblock_error(bio);
+			free_r10bio(r10_bio);
 			return false;
 		}
 		mddev_add_trace_msg(conf->mddev,
-- 
2.43.0


^ permalink raw reply related

* [PATCH 0/7] md/raid10: fixes, atomic write handling, and error-path cleanup
From: Abd-Alrhman Masalkhi @ 2026-06-23  7:24 UTC (permalink / raw)
  To: song, yukuai, magiclinan, xiao, axboe, john.g.garry,
	martin.petersen, abd.masalkhi
  Cc: linux-raid, linux-kernel

Hi,

This series contains a mix of bug fixes and cleanups for RAID10, along
with a related atomic write fix for RAID1.

Thanks,
Abd-alrhman,

Abd-Alrhman Masalkhi (7):
  md/raid10: fix r10bio leak in raid10_write_request() error paths
  md/raid1: handle atomic writes that require splitting
  md/raid10: handle atomic writes that require splitting
  md/raid10: raid10_write_request() drops the barrier before calling
  md/raid10: replace wait loop with wait_event_idle()
  md/raid10: simplify write request error handling
  md/raid10: simplify read request error handling

 drivers/md/raid1.c  |  25 ++++-----
 drivers/md/raid10.c | 123 +++++++++++++++++++++-----------------------
 2 files changed, 71 insertions(+), 77 deletions(-)

-- 
2.43.0


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox