* Re: [PATCH v4 1/3] md: suspend array before raid10 reshape via sync_action
From: yu kuai @ 2026-06-20 19:43 UTC (permalink / raw)
To: Chen Cheng, linux-raid, yukuai; +Cc: linux-kernel
In-Reply-To: <20260603035925.217847-2-chencheng@fnnas.com>
Hi,
在 2026/6/3 11:59, Chen Cheng 写道:
> From: Chen Cheng <chencheng@fnnas.com>
>
> The sync_action=reshape path currently enters mddev_start_reshape() with
> reconfig_mutex held but without suspending the array first. For raid10,
> that means raid10_start_reshape() has to drop reconfig_mutex and reacquire
> the array through mddev_suspend_and_lock_nointr() before it can safely
> switch geometry-dependent state.
>
> Use mddev_suspend_and_lock() for ACTION_RESHAPE in action_store(), so
> the sysfs reshape path reaches mddev_start_reshape() with the array
> already suspended and locked.
>
> Other sync_action operations keep using mddev_lock() unchanged.
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
> drivers/md/md.c | 22 +++++++++++++++++-----
> 1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 096bb64e87bd..5bc937e149ac 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -5256,30 +5256,39 @@ static int mddev_start_reshape(struct mddev *mddev)
>
> 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);
> retry:
> if (work_busy(&mddev->sync_work))
> flush_work(&mddev->sync_work);
>
> - ret = mddev_lock(mddev);
> + if (action == ACTION_RESHAPE) {
> + ret = mddev_suspend_and_lock(mddev);
suspend during retry is too heavy. Please move suspend above retry.
> + suspended = true;
> + } else {
> + ret = mddev_lock(mddev);
> + suspended = false;
> + }
> if (ret)
> return ret;
>
> if (work_busy(&mddev->sync_work)) {
> - mddev_unlock(mddev);
> + if (suspended)
> + mddev_unlock_and_resume(mddev);
> + else
> + mddev_unlock(mddev);
> goto retry;
> }
>
> - action = md_sync_action_by_name(page);
> -
> /* TODO: mdadm rely on "idle" to start sync_thread. */
> if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)) {
> switch (action) {
> case ACTION_FROZEN:
> md_frozen_sync_thread(mddev);
> @@ -5344,11 +5353,14 @@ action_store(struct mddev *mddev, const char *page, size_t len)
> md_wakeup_thread(mddev->thread);
> sysfs_notify_dirent_safe(mddev->sysfs_action);
> ret = len;
>
> out:
> - mddev_unlock(mddev);
> + if (suspended)
> + mddev_unlock_and_resume(mddev);
> + else
> + mddev_unlock(mddev);
> return ret;
> }
>
> static struct md_sysfs_entry md_scan_mode =
> __ATTR_PREALLOC(sync_action, S_IRUGO|S_IWUSR, action_show, action_store);
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH v4 3/3] md/raid10: bound reused r10bio devs[] walks by used_nr_devs
From: yu kuai @ 2026-06-20 20:02 UTC (permalink / raw)
To: Chen Cheng, linux-raid, yukuai; +Cc: linux-kernel
In-Reply-To: <20260603035925.217847-4-chencheng@fnnas.com>
Hi,
在 2026/6/3 11:59, Chen Cheng 写道:
> From: Chen Cheng <chencheng@fnnas.com>
>
> After reshape changes raid_disks, an in-flight r10bio from the old geometry
> can still be completed or freed later. In that case, using the current
> geometry to walk r10_bio->devs[] is unsafe. A failure was reproduced with a
> simple write workload while reshaping a raid10 array from 4 disks to 5 disks.
> e.g.:
>
> 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
>
> the sequence above can trigger:
>
> 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
>
> The buggy object was 200 bytes long, which matches an r10bio with space for
> only four devs[] entries. However, put_all_bios() and find_bio_disk() walk
> r10_bio->devs[] using the current conf->geo.raid_disks value. Once reshape
> switches conf->geo.raid_disks from 4 to 5, an old 4-slot r10bio can be
> completed or freed as if it had 5 slots, and the walk overruns devs[4]. The
I don't understand, is this still possible after patch 1.
> same stale-width mismatch can also surface during a 5-disk to 4-disk reshape.
>
> Track the number of valid devs[] entries in each reused r10bio with
> used_nr_devs. Initialize it whenever an r10bio is prepared for regular I/O,
> discard, or resync/recovery/reshape work, and use it to bound devs[] walks
> in put_all_bios() and find_bio_disk().
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
> drivers/md/raid10.c | 8 ++++++--
> drivers/md/raid10.h | 2 ++
> 2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 5eca34432e63..f134b93fd593 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -273,11 +273,11 @@ static void r10buf_pool_free(void *__r10_bio, void *data)
>
> static void put_all_bios(struct r10conf *conf, struct r10bio *r10_bio)
> {
> int i;
>
> - for (i = 0; i < conf->geo.raid_disks; i++) {
> + for (i = 0; i < r10_bio->used_nr_devs; i++) {
> struct bio **bio = & r10_bio->devs[i].bio;
> if (!BIO_SPECIAL(*bio))
> bio_put(*bio);
> *bio = NULL;
> bio = &r10_bio->devs[i].repl_bio;
> @@ -370,11 +370,11 @@ static int find_bio_disk(struct r10conf *conf, struct r10bio *r10_bio,
> struct bio *bio, int *slotp, int *replp)
> {
> int slot;
> int repl = 0;
>
> - for (slot = 0; slot < conf->geo.raid_disks; slot++) {
> + for (slot = 0; slot < r10_bio->used_nr_devs; slot++) {
> if (r10_bio->devs[slot].bio == bio)
> break;
> if (r10_bio->devs[slot].repl_bio == bio) {
> repl = 1;
> break;
> @@ -1561,10 +1561,11 @@ static void __make_request(struct mddev *mddev, struct bio *bio, int sectors)
>
> r10_bio->mddev = mddev;
> r10_bio->sector = bio->bi_iter.bi_sector;
> r10_bio->state = 0;
> r10_bio->read_slot = -1;
> + r10_bio->used_nr_devs = conf->geo.raid_disks;
> memset(r10_bio->devs, 0, sizeof(r10_bio->devs[0]) *
> conf->geo.raid_disks);
>
> if (bio_data_dir(bio) == READ)
> raid10_read_request(mddev, bio, r10_bio);
> @@ -1749,10 +1750,11 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
> 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;
> + r10_bio->used_nr_devs = geo->raid_disks;
> memset(r10_bio->devs, 0, sizeof(r10_bio->devs[0]) * geo->raid_disks);
> wait_blocked_dev(mddev, r10_bio);
>
> /*
> * For far layout it needs more than one r10bio to cover all regions.
> @@ -3083,10 +3085,12 @@ static struct r10bio *raid10_alloc_init_r10buf(struct r10conf *conf)
> test_bit(MD_RECOVERY_RESHAPE, &conf->mddev->recovery))
> nalloc = conf->copies; /* resync */
> else
> nalloc = 2; /* recovery */
>
> + r10bio->used_nr_devs = nalloc;
> +
> for (i = 0; i < nalloc; i++) {
> bio = r10bio->devs[i].bio;
> rp = bio->bi_private;
> bio_reset(bio, NULL, 0);
> bio->bi_private = rp;
> diff --git a/drivers/md/raid10.h b/drivers/md/raid10.h
> index b711626a5db7..4751119f9770 100644
> --- a/drivers/md/raid10.h
> +++ b/drivers/md/raid10.h
> @@ -125,10 +125,12 @@ struct r10bio {
> struct bio *master_bio;
> /*
> * if the IO is in READ direction, then this is where we read
> */
> int read_slot;
> + /* Used to bound devs[] walks when the object is reused. */
> + unsigned int used_nr_devs;
>
> struct list_head retry_list;
> /*
> * if the IO is in WRITE direction, then multiple bios are used,
> * one for each copy.
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH v4 2/3] md/raid10: make r10bio_pool use fixed-size objects
From: yu kuai @ 2026-06-20 20:05 UTC (permalink / raw)
To: Chen Cheng, linux-raid; +Cc: linux-kernel, yukuai
In-Reply-To: <20260603035925.217847-3-chencheng@fnnas.com>
Hi,
在 2026/6/3 11:59, Chen Cheng 写道:
> From: Chen Cheng <chencheng@fnnas.com>
>
> raid10 currently sizes regular r10bio_pool objects from
> conf->geo.raid_disks, which makes the mempool element width depend on
> the current geometry.
>
> That breaks across reshape. Regular r10bio objects are preallocated and
> reused, so after a geometry change the pool may still contain objects
> allocated for the old width. A later request under the new geometry can
> then reuse an r10bio whose devs[] array is still sized for the previous
> raid_disks value.
>
> Fix this by backing r10bio_pool with a fixed-size kmalloc mempool sized
> for the maximum width needed across the current reshape transition.
> Apply the same sizing rule to standalone r10bio objects allocated from
> r10buf_pool_alloc().
>
> This removes the geometry-dependent allocation width from regular
> r10bio_pool objects and prevents reshape from reusing pool entries that
> are too small for the new layout.
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
> drivers/md/raid10.c | 48 +++++++++++++++++++++++++++++++++------------
> drivers/md/raid10.h | 2 +-
> 2 files changed, 36 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index cee5a253a281..5eca34432e63 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -101,17 +101,32 @@ 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 unsigned int calc_r10bio_pool_disks(struct mddev *mddev)
> {
> - struct r10conf *conf = data;
> - int size = offsetof(struct r10bio, devs[conf->geo.raid_disks]);
> + /* If delta_disks < 0, use bigger r10bio->devs[] is ok. */
> + return mddev->raid_disks + max(0, mddev->delta_disks);
I don't understand this change first, I get what you want to do after taking
a look at patch 3. However, I think geo.raid_disks is safe here. A r10_bio
that is allocated from old pool should never be reused under new geometry.
> +}
> +
> +static inline int calc_r10bio_size(struct mddev *mddev)
> +{
> + return offsetof(struct r10bio, devs[calc_r10bio_pool_disks(mddev)]);
> +}
> +
> +static mempool_t *create_r10bio_pool(struct mddev *mddev)
> +{
> + int size = calc_r10bio_size(mddev);
> +
> + return mempool_create_kmalloc_pool(NR_RAID_BIOS, size);
> +}
> +
> +static struct r10bio *alloc_r10bio(struct mddev *mddev, gfp_t gfp_flags)
> +{
> + int size = calc_r10bio_size(mddev);
>
> - /* allocate a r10bio with room for raid_disks entries in the
> - * bios array */
> return kzalloc(size, gfp_flags);
> }
>
> #define RESYNC_SECTORS (RESYNC_BLOCK_SIZE >> 9)
> /* amount of memory to reserve for resync requests */
> @@ -135,11 +150,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->mddev, 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 +290,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 +1552,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 +1744,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 +3845,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 +3892,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(mddev);
> + if (!conf->r10bio_pool)
> goto out;
>
> err = bioset_init(&conf->bio_split, BIO_POOL_SIZE, 0, 0);
> if (err)
> goto out;
> @@ -4373,10 +4387,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;
>
> if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery))
> return -EBUSY;
>
> if (setup_geo(&new, mddev, geo_start) != conf->copies)
> @@ -4409,10 +4424,17 @@ 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(mddev);
> + if (!new_pool)
> + return -ENOMEM;
> + mempool_destroy(conf->r10bio_pool);
> + conf->r10bio_pool = new_pool;
> + }
> 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();
> 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
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid1: honor REQ_NOWAIT when waiting for behind writes
From: Yu Kuai @ 2026-06-20 20:18 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song, yukuai, magiclinan, xiao, vverma,
axboe
Cc: linux-raid, linux-kernel, yukuai
In-Reply-To: <20260611083514.754922-1-abd.masalkhi@gmail.com>
Hi,
在 2026/6/11 16:35, Abd-Alrhman Masalkhi 写道:
> raid1 supports REQ_NOWAIT reads by avoiding waits in the barrier path
> through wait_read_barrier(). However, a read can still block on a
> WriteMostly device when the array uses a bitmap and there are
> outstanding behind writes.
>
> In that case raid1 unconditionally calls wait_behind_writes(), which
> may sleep until all behind writes complete. As a result, a REQ_NOWAIT
> read can block despite the caller explicitly requesting non-blocking
> behavior.
>
> This ensures that raid1 consistently honors REQ_NOWAIT reads across all
> paths that may otherwise wait for behind writes.
>
> Fixes: 5aa705039c4f ("md: raid1 add nowait support")
> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
> ---
> 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 | 10 +++++++---
> 5 files changed, 24 insertions(+), 12 deletions(-)
Applied to md-7.2.
One note, remove nowait support is already in my to-do lists. There is a case
that can't be handled for now, for example:
2 disk raid1, issue no wait write IO, rdev1 succeed while rdev2 failed. In this
case, we don't know if rdev2 failed issue because nowait or disk really have
badblocks. So we can't either record badblocks or issue again without nowait,
for consequence, rdev1 and rdev2 will end up with different data.
I think the best solution is to remove nowait support for mdraid. Feel free
to cook a patch or if you have something else in mind.
>
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 028b9ca8ce52..1206e31f323a 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -2063,18 +2063,23 @@ static void bitmap_end_behind_write(struct mddev *mddev)
> bitmap->mddev->bitmap_info.max_write_behind);
> }
>
> -static void bitmap_wait_behind_writes(struct mddev *mddev)
> +static bool bitmap_wait_behind_writes(struct mddev *mddev, bool nowait)
> {
> struct bitmap *bitmap = mddev->bitmap;
>
> /* wait for behind writes to complete */
> if (bitmap && atomic_read(&bitmap->behind_writes) > 0) {
> + if (nowait)
> + return false;
> +
> pr_debug("md:%s: behind writes in progress - waiting to stop.\n",
> mdname(mddev));
> /* need to kick something here to make sure I/O goes? */
> wait_event(bitmap->behind_wait,
> atomic_read(&bitmap->behind_writes) == 0);
> }
> +
> + return true;
> }
>
> static void bitmap_destroy(struct mddev *mddev)
> @@ -2084,7 +2089,7 @@ static void bitmap_destroy(struct mddev *mddev)
> if (!bitmap) /* there was no bitmap */
> return;
>
> - bitmap_wait_behind_writes(mddev);
> + bitmap_wait_behind_writes(mddev, false);
> if (!test_bit(MD_SERIALIZE_POLICY, &mddev->flags))
> mddev_destroy_serial_pool(mddev, NULL);
>
> diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
> index 214f623c7e79..f46674bdfeb9 100644
> --- a/drivers/md/md-bitmap.h
> +++ b/drivers/md/md-bitmap.h
> @@ -98,7 +98,7 @@ struct bitmap_operations {
>
> void (*start_behind_write)(struct mddev *mddev);
> void (*end_behind_write)(struct mddev *mddev);
> - void (*wait_behind_writes)(struct mddev *mddev);
> + bool (*wait_behind_writes)(struct mddev *mddev, bool nowait);
>
> md_bitmap_fn *start_write;
> md_bitmap_fn *end_write;
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index 1adc5b117821..5a4e2abaa757 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -1574,16 +1574,19 @@ static void llbitmap_end_behind_write(struct mddev *mddev)
> wake_up(&llbitmap->behind_wait);
> }
>
> -static void llbitmap_wait_behind_writes(struct mddev *mddev)
> +static bool llbitmap_wait_behind_writes(struct mddev *mddev, bool nowait)
> {
> struct llbitmap *llbitmap = mddev->bitmap;
>
> - if (!llbitmap)
> - return;
> + if (llbitmap && atomic_read(&llbitmap->behind_writes) > 0) {
> + if (nowait)
> + return false;
>
> - wait_event(llbitmap->behind_wait,
> - atomic_read(&llbitmap->behind_writes) == 0);
> + wait_event(llbitmap->behind_wait,
> + atomic_read(&llbitmap->behind_writes) == 0);
> + }
>
> + return true;
> }
>
> static ssize_t bits_show(struct mddev *mddev, char *page)
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 096bb64e87bd..d1465bcd86c8 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -7050,7 +7050,7 @@ EXPORT_SYMBOL_GPL(md_stop_writes);
> static void mddev_detach(struct mddev *mddev)
> {
> if (md_bitmap_enabled(mddev, false))
> - mddev->bitmap_ops->wait_behind_writes(mddev);
> + mddev->bitmap_ops->wait_behind_writes(mddev, false);
> if (mddev->pers && mddev->pers->quiesce && !is_md_suspended(mddev)) {
> mddev->pers->quiesce(mddev, 1);
> mddev->pers->quiesce(mddev, 0);
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index b1ed4cc6ade4..b2d7c13b64bd 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -1341,6 +1341,7 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
> int max_sectors;
> int rdisk;
> bool r1bio_existed = !!r1_bio;
> + bool nowait = bio->bi_opf & REQ_NOWAIT;
>
> /*
> * An md cloned bio indicates we are in the error path.
> @@ -1360,8 +1361,7 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
> * Still need barrier for READ in case that whole
> * array is frozen.
> */
> - if (!wait_read_barrier(conf, bio->bi_iter.bi_sector,
> - bio->bi_opf & REQ_NOWAIT)) {
> + if (!wait_read_barrier(conf, bio->bi_iter.bi_sector, nowait)) {
> bio_wouldblock_error(bio);
> return;
> }
> @@ -1402,7 +1402,11 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
> * over-take any writes that are 'behind'
> */
> mddev_add_trace_msg(mddev, "raid1 wait behind writes");
> - mddev->bitmap_ops->wait_behind_writes(mddev);
> + if (!mddev->bitmap_ops->wait_behind_writes(mddev, nowait)) {
> + bio_wouldblock_error(bio);
> + set_bit(R1BIO_Returned, &r1_bio->state);
> + goto err_handle;
> + }
> }
>
> if (max_sectors < bio_sectors(bio)) {
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid1: free r1_bio when REQ_NOWAIT is set and read would block on retry
From: yu kuai @ 2026-06-20 20:30 UTC (permalink / raw)
To: Abd-Alrhman Masalkhi, song, yukuai, magiclinan, xiao, vverma,
axboe
Cc: linux-raid, linux-kernel, sashiko-bot, yukuai
In-Reply-To: <20260611101350.759154-1-abd.masalkhi@gmail.com>
在 2026/6/11 18:13, Abd-Alrhman Masalkhi 写道:
> When a read is retried, raid1_read_request() may be called with a
> pre-allocated r1_bio. If wait_read_barrier() fails for a REQ_NOWAIT
> read, the bio is completed and the function returns immediately. In this
> case the existing r1_bio is leaked.
>
> This fixes a leak of pre-allocated r1_bio structures for retried reads.
>
> Fixes: 5aa705039c4f ("md: raid1 add nowait support")
> Reported-by: sashiko-bot<sashiko-bot@kernel.org>
> Closes:https://sashiko.dev/#/patchset/20260611083514.754922-1-abd.masalkhi@gmail.com?part=1
> Signed-off-by: Abd-Alrhman Masalkhi<abd.masalkhi@gmail.com>
> ---
> drivers/md/raid1.c | 6 ++++++
> 1 file changed, 6 insertions(+)
Applied to md-7.2
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid1: protect head_position for read balance
From: yu kuai @ 2026-06-20 20:44 UTC (permalink / raw)
To: Chen Cheng, linux-raid, yukuai; +Cc: linux-kernel
In-Reply-To: <20260619044114.1208456-1-chencheng@fnnas.com>
在 2026/6/19 12:41, Chen Cheng 写道:
> From: Chen Cheng<chencheng@fnnas.com>
>
> KCSAN reports a data race between raid1_end_read_request() and
> raid1_read_request().
>
> The completion path updates conf->mirrors[disk].head_position in
> update_head_pos() without a lock, while the read-balance heuristic reads
> the same field locklessly in is_sequential() and choose_best_rdev().
>
> KCSAN report:
> =========================
> BUG: KCSAN: data-race in raid1_end_read_request / raid1_read_request
>
> write to 0xffff8f0306ba7868 of 8 bytes by interrupt on cpu 9:
> raid1_end_read_request+0xb5/0x440
> bio_endio+0x3c9/0x3e0
> blk_update_request+0x257/0x770
> scsi_end_request+0x4d/0x520
> scsi_io_completion+0x6f/0x990
> scsi_finish_command+0x188/0x280
> scsi_complete+0xac/0x160
> blk_complete_reqs+0x8e/0xb0
> blk_done_softirq+0x1d/0x30
> [...]
>
> read to 0xffff8f0306ba7868 of 8 bytes by task 667002 on cpu 11:
> raid1_read_request+0x497/0x1a10
> raid1_make_request+0xdf/0x1950
> md_handle_request+0x2c5/0x700
> md_submit_bio+0x126/0x320
> __submit_bio+0x2ec/0x3a0
> submit_bio_noacct_nocheck+0x572/0x890
> [...]
>
> value changed: 0x0000000000000078 -> 0x00000000005fe448
>
> Signed-off-by: Chen Cheng<chencheng@fnnas.com>
> ---
> drivers/md/raid1.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
Applied to md-7.2
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH v2] md/raid5: protect bitmap batch counters aka seq_flush/seq_write consistency
From: yu kuai @ 2026-06-20 21:13 UTC (permalink / raw)
To: Chen Cheng, linux-raid, yukuai; +Cc: chenchneg33, linux-kernel
In-Reply-To: <20260617142839.882378-1-chencheng@fnnas.com>
Hi,
在 2026/6/17 22:28, Chen Cheng 写道:
> From: Chen Cheng <chencheng@fnnas.com>
>
> kcsan detect race :
> - raid5d() closes the current bitmap batch by updating
> conf->seq_flush under conf->device_lock.
> - __add_stripe_bio() read conf->seq_flush without that
> lock when assigning sh->bm_seq.
>
> so, protect seq_flush/seq_write consistency for multiple CPUs by
> READ_ONCE()/WRITE_ONCE() under the path without held device_lock.
>
> re-explain the stripe batch sequence number update flow:
> 1. sh->bm_seq declare which batch number the stripe belongs to
> when perform bitmap-related write.
> ==> bm_seq = seq_flush+1
>
> 2. stripe be handled,
> * if sh->bm_seq - conf->seq_write > 0, means the
> batch stripes **newer than** the last written
> batch, it cannot proceed yet, queued on bitmap_list.
> * otherwise , has already proceed.
>
> 3. raid5d() `++seq_flush` to closes the current batch, means
> * no more stripes join that old batch
> * just-closed batch ready to write-out to disk
>
> 4. raid5d() calls bitmap hooks unplug() or writeout, then,
> `++seq_write` to the same as bm_seq.
>
> - seq_flush - for producer, to close batches.
> - seq_write - for consumer, the checkpoint number.
>
> the report:
> ====================================
> BUG: KCSAN: data-race in __add_stripe_bio / raid5d
>
> write to 0xffff88ba5625d470 of 4 bytes by task 82401 on cpu 0:
> raid5d+0x1d9/0xba0
> [.....]
>
> read to 0xffff88ba5625d470 of 4 bytes by task 82421 on cpu 8:
> __add_stripe_bio+0x332/0x400
> raid5_make_request+0x6ac/0x2930
> md_handle_request+0x4a2/0xa40
> md_submit_bio+0x109/0x1a0
> __submit_bio+0x2ec/0x390
> [.....]
>
> v1 -> v2:
> - remove WRITE_ONCE(conf->seq_write) in held device_lock path.
> - remove READ_ONCE(conf->seq_flush) in held device_lock path.
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
> drivers/md/raid5.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index a320b71d7117..b2c5a1930841 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -3536,11 +3536,11 @@ static void __add_stripe_bio(struct stripe_head *sh, struct bio *bi,
> pr_debug("added bi b#%llu to stripe s#%llu, disk %d, logical %llu\n",
> (*bip)->bi_iter.bi_sector, sh->sector, dd_idx,
> sh->dev[dd_idx].sector);
>
> if (conf->mddev->bitmap && firstwrite && !sh->batch_head) {
> - sh->bm_seq = conf->seq_flush+1;
> + sh->bm_seq = READ_ONCE(conf->seq_flush) + 1;
> set_bit(STRIPE_BIT_DELAY, &sh->state);
> }
> }
>
> /*
> @@ -5827,11 +5827,11 @@ static void make_discard_request(struct mddev *mddev, struct bio *bi)
> md_write_inc(mddev, bi);
> sh->overwrite_disks++;
> }
> spin_unlock_irq(&sh->stripe_lock);
> if (conf->mddev->bitmap) {
> - sh->bm_seq = conf->seq_flush + 1;
> + sh->bm_seq = READ_ONCE(conf->seq_flush) + 1;
> set_bit(STRIPE_BIT_DELAY, &sh->state);
> }
>
> set_bit(STRIPE_HANDLE, &sh->state);
> clear_bit(STRIPE_DELAYED, &sh->state);
> @@ -6877,16 +6877,17 @@ static void raid5d(struct md_thread *thread)
> clear_bit(R5_DID_ALLOC, &conf->cache_state);
>
> if (
> !list_empty(&conf->bitmap_list)) {
> /* Now is a good time to flush some bitmap updates */
> - conf->seq_flush++;
> + int seq = conf->seq_flush + 1;
checkpatch should warn this about missing blank line after declaration.
> + WRITE_ONCE(conf->seq_flush, seq);
WRITE_ONCE(a, a + 1) is safe, you can see lots of examples.
> spin_unlock_irq(&conf->device_lock);
> if (md_bitmap_enabled(mddev, true))
> mddev->bitmap_ops->unplug(mddev, true);
> spin_lock_irq(&conf->device_lock);
> - conf->seq_write = conf->seq_flush;
> + conf->seq_write = seq;
> activate_bit_delay(conf, conf->temp_inactive_list);
> }
> raid5_activate_delayed(conf);
>
> while ((bio = remove_bio_from_retry(conf, &offset))) {
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid5: let stripe batch bm_seq comparison wrap-safe
From: yu kuai @ 2026-06-20 21:27 UTC (permalink / raw)
To: Chen Cheng, linux-raid, yukuai; +Cc: chenchneg33, linux-kernel
In-Reply-To: <20260618025735.915113-1-chencheng@fnnas.com>
在 2026/6/18 10:57, Chen Cheng 写道:
> From: Chen Cheng<chencheng@fnnas.com>
>
> Once the 32-bit seq wraps, a newer bm_seq can look smaller
> than old, so .. covert to wrap-safe calculate way.
>
> Signed-off-by: Chen Cheng<chencheng@fnnas.com>
> ---
> drivers/md/raid5.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Applied to md-7.2
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid5: use stripe state snapshot in break_stripe_batch_list()
From: yu kuai @ 2026-06-20 21:37 UTC (permalink / raw)
To: Chen Cheng, linux-raid, yukuai; +Cc: linux-kernel
In-Reply-To: <20260618134748.1168360-1-chencheng@fnnas.com>
Hi,
在 2026/6/18 21:47, Chen Cheng 写道:
> From: Chen Cheng <chencheng@fnnas.com>
>
> The patch just suppress KCSAN noise. No functional change.
>
> RAID-5 can group multi full-stripe-write aka stripe_head into a
> batch aka batch_list, with one head_sh leading them. Call
> break_stripe_batch_list() when the batch is finished, or,
> a stripe has to be dropped out of the batch.
>
> break_stripe_batch_list() reads stripe state several times while
> request paths can update thost state words concurrently with
> lockless bitops, which reported by KCSAN.
>
> Use a snapshot to guarantees that the value used for
> warning, copying, and handle checks is internally consistent
> at current read moment.
>
> KCSAN report:
> ==============================================
> BUG: KCSAN: data-race in __add_stripe_bio / break_stripe_batch_list
>
> write (marked) to 0xffff8e89d4f0b988 of 8 bytes by task 4323 on cpu 3:
> __add_stripe_bio+0x35e/0x400
> raid5_make_request+0x6ac/0x2930
> md_handle_request+0x4a2/0xa40
> md_submit_bio+0x109/0x1a0
> __submit_bio+0x2ec/0x390
> submit_bio_noacct_nocheck+0x457/0x710
> submit_bio_noacct+0x2a7/0xc20
> submit_bio+0x56/0x250
> blkdev_direct_IO+0x54c/0xda0
> blkdev_write_iter+0x38f/0x570
> aio_write+0x22b/0x490
> io_submit_one+0xa51/0xf70
>
> read to 0xffff8e89d4f0b988 of 8 bytes by task 4290 on cpu 4:
> break_stripe_batch_list+0x3ce/0x480
> handle_stripe_clean_event+0x720/0x9b0
> handle_stripe+0x32fb/0x4500
> handle_active_stripes.isra.0+0x6e0/0xa50
> raid5d+0x7e0/0xba0
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
> drivers/md/raid5.c | 45 ++++++++++++++++++++++++++-------------------
> 1 file changed, 26 insertions(+), 19 deletions(-)
You have another patch that is relied on this patch, they should be in one
patchset. What's worse, they are sent in the reverse order :(
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 26c24986e01c..a376560be92e 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -4906,35 +4906,39 @@ static int clear_batch_ready(struct stripe_head *sh)
> static void break_stripe_batch_list(struct stripe_head *head_sh,
> unsigned long handle_flags)
> {
> struct stripe_head *sh, *next;
> int i;
> + unsigned long state;
>
> list_for_each_entry_safe(sh, next, &head_sh->batch_list, batch_list) {
>
> list_del_init(&sh->batch_list);
>
> - WARN_ONCE(sh->state & ((1 << STRIPE_ACTIVE) |
> - (1 << STRIPE_SYNCING) |
> - (1 << STRIPE_REPLACED) |
> - (1 << STRIPE_DELAYED) |
> - (1 << STRIPE_BIT_DELAY) |
> - (1 << STRIPE_FULL_WRITE) |
> - (1 << STRIPE_BIOFILL_RUN) |
> - (1 << STRIPE_COMPUTE_RUN) |
> - (1 << STRIPE_DISCARD) |
> - (1 << STRIPE_BATCH_READY) |
> - (1 << STRIPE_BATCH_ERR)),
> - "stripe state: %lx\n", sh->state);
> - WARN_ONCE(head_sh->state & ((1 << STRIPE_DISCARD) |
> - (1 << STRIPE_REPLACED)),
> - "head stripe state: %lx\n", head_sh->state);
> + state = READ_ONCE(sh->state);
> + WARN_ONCE(state & ((1 << STRIPE_ACTIVE) |
> + (1 << STRIPE_SYNCING) |
> + (1 << STRIPE_REPLACED) |
> + (1 << STRIPE_DELAYED) |
> + (1 << STRIPE_BIT_DELAY) |
> + (1 << STRIPE_FULL_WRITE) |
> + (1 << STRIPE_BIOFILL_RUN) |
> + (1 << STRIPE_COMPUTE_RUN) |
> + (1 << STRIPE_DISCARD) |
> + (1 << STRIPE_BATCH_READY) |
> + (1 << STRIPE_BATCH_ERR)),
> + "stripe state: %lx\n", state);
> +
> + state = READ_ONCE(head_sh->state);
> + WARN_ONCE(state & ((1 << STRIPE_DISCARD) |
> + (1 << STRIPE_REPLACED)),
> + "head stripe state: %lx\n", state);
>
> set_mask_bits(&sh->state, ~(STRIPE_EXPAND_SYNC_FLAGS |
> (1 << STRIPE_PREREAD_ACTIVE) |
> (1 << STRIPE_ON_UNPLUG_LIST)),
> - head_sh->state & (1 << STRIPE_INSYNC));
> + state & (1 << STRIPE_INSYNC));
>
> sh->check_state = head_sh->check_state;
> sh->reconstruct_state = head_sh->reconstruct_state;
> spin_lock_irq(&sh->stripe_lock);
> sh->batch_head = NULL;
> @@ -4943,22 +4947,25 @@ static void break_stripe_batch_list(struct stripe_head *head_sh,
> if (test_and_clear_bit(R5_Overlap, &sh->dev[i].flags))
> wake_up_bit(&sh->dev[i].flags, R5_Overlap);
> sh->dev[i].flags = head_sh->dev[i].flags &
> (~((1 << R5_WriteError) | (1 << R5_Overlap)));
> }
> - if (handle_flags == 0 ||
> - sh->state & handle_flags)
> +
> + state = READ_ONCE(sh->state);
> + if (handle_flags == 0 || (state & handle_flags))
> set_bit(STRIPE_HANDLE, &sh->state);
> raid5_release_stripe(sh);
> }
> spin_lock_irq(&head_sh->stripe_lock);
> head_sh->batch_head = NULL;
> spin_unlock_irq(&head_sh->stripe_lock);
> for (i = 0; i < head_sh->disks; i++)
> if (test_and_clear_bit(R5_Overlap, &head_sh->dev[i].flags))
> wake_up_bit(&head_sh->dev[i].flags, R5_Overlap);
> - if (head_sh->state & handle_flags)
> +
> + state = READ_ONCE(head_sh->state);
> + if (state & handle_flags)
> set_bit(STRIPE_HANDLE, &head_sh->state);
> }
>
> /*
> * handle_stripe - do things to a stripe.
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid5: use stripe state snapshot in break_stripe_batch_list()
From: yu kuai @ 2026-06-20 21:38 UTC (permalink / raw)
To: Chen Cheng, linux-raid, yukuai; +Cc: linux-kernel
In-Reply-To: <20260618134748.1168360-1-chencheng@fnnas.com>
在 2026/6/18 21:47, Chen Cheng 写道:
> From: Chen Cheng<chencheng@fnnas.com>
>
> The patch just suppress KCSAN noise. No functional change.
>
> RAID-5 can group multi full-stripe-write aka stripe_head into a
> batch aka batch_list, with one head_sh leading them. Call
> break_stripe_batch_list() when the batch is finished, or,
> a stripe has to be dropped out of the batch.
>
> break_stripe_batch_list() reads stripe state several times while
> request paths can update thost state words concurrently with
> lockless bitops, which reported by KCSAN.
>
> Use a snapshot to guarantees that the value used for
> warning, copying, and handle checks is internally consistent
> at current read moment.
>
> KCSAN report:
> ==============================================
> BUG: KCSAN: data-race in __add_stripe_bio / break_stripe_batch_list
>
> write (marked) to 0xffff8e89d4f0b988 of 8 bytes by task 4323 on cpu 3:
> __add_stripe_bio+0x35e/0x400
> raid5_make_request+0x6ac/0x2930
> md_handle_request+0x4a2/0xa40
> md_submit_bio+0x109/0x1a0
> __submit_bio+0x2ec/0x390
> submit_bio_noacct_nocheck+0x457/0x710
> submit_bio_noacct+0x2a7/0xc20
> submit_bio+0x56/0x250
> blkdev_direct_IO+0x54c/0xda0
> blkdev_write_iter+0x38f/0x570
> aio_write+0x22b/0x490
> io_submit_one+0xa51/0xf70
>
> read to 0xffff8e89d4f0b988 of 8 bytes by task 4290 on cpu 4:
> break_stripe_batch_list+0x3ce/0x480
> handle_stripe_clean_event+0x720/0x9b0
> handle_stripe+0x32fb/0x4500
> handle_active_stripes.isra.0+0x6e0/0xa50
> raid5d+0x7e0/0xba0
>
> Signed-off-by: Chen Cheng<chencheng@fnnas.com>
> ---
> drivers/md/raid5.c | 45 ++++++++++++++++++++++++++-------------------
> 1 file changed, 26 insertions(+), 19 deletions(-)
Applied to md-7.2
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH] md/raid5: avoid R5_Overlap races while breaking stripe batches
From: yu kuai @ 2026-06-20 21:51 UTC (permalink / raw)
To: Chen Cheng, linux-raid, yukuai; +Cc: chenchneg33, linux-kernel
In-Reply-To: <20260619041013.1207148-1-chencheng@fnnas.com>
在 2026/6/19 12:10, Chen Cheng 写道:
> From: Chen Cheng <chencheng@fnnas.com>
>
> KCSAN report a race in break_stripe_batch_list() vs. raid5_make_request()
> on sh->dev[i].flags (plain word write vs. atomic bit op)..
>
> and .. one possible scenario is:
>
> CPU1 CPU2
> break_stripe_batch_list(sh1)
> -> handle sh2
> -> lock(sh2)
> -> sh2->batch_head = NULL
> -> unlock(sh2)
> -> test_and_clear_bit(R5_Overlap, sh2->dev[i].flags)
> -> wake_up_bit(sh2->dev[i].flags)
> raid5_make_request()
> -> add_all_stripe_bios(sh2)
> -> lock(sh2)
> -> stripe_bio_overlaps(sh2) returns true
> batch_head is NULL, so new bio overlap
> exist bio on sh2 -> true
> -> set_bit(R5_Overlap, sh2->dev[i].flags)
> -> unlock(sh2)
> -> wait_on_bit(sh2->dev[i].flags)
> -> sh2->dev[i].flags = sh1->dev[i].flags & ~R5_Overlap
>
> No wait_up_bit(), CPU2 could be wait_on_bit() forever...
>
> Fix by :
> - Expand the protect zone.
> - Use batch_head's device flag's snaphot when no held head_sh->stripe_lock.
> - Move sh/head_sh->batch_head = NULL to the end of protected zone , and ,
> any concurrent add_all_stripe_bios() grabs sh->stripe_lock now either:
> - see batch_head != null, and , is rejected by stripe_bio_overlaps()
> under the lock (no R5_Overlap wait ) , or ,
> - sees batch_head == NULL, only after dev[i].flags has already been
> set and the prior R5_Overlap waiters worken.
>
> KCSAN report:
> ================================================
> BUG: KCSAN: data-race in break_stripe_batch_list / raid5_make_request
>
> write (marked) to 0xffff8e89c8117548 of 8 bytes by task 4042 on cpu 0:
> raid5_make_request+0xea0/0x2930
> md_handle_request+0x4a2/0xa40
> md_submit_bio+0x109/0x1a0
> __submit_bio+0x2ec/0x390
> submit_bio_noacct_nocheck+0x457/0x710
> submit_bio_noacct+0x2a7/0xc20
> submit_bio+0x56/0x250
> blkdev_direct_IO+0x54c/0xda0
> blkdev_write_iter+0x38f/0x570
> aio_write+0x22b/0x490
> io_submit_one+0xa51/0xf70
> __x64_sys_io_submit+0xf7/0x220
> x64_sys_call+0x1907/0x1c60
> do_syscall_64+0x130/0x570
> entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> read to 0xffff8e89c8117548 of 8 bytes by task 4010 on cpu 5:
> break_stripe_batch_list+0x249/0x480
> handle_stripe_clean_event+0x720/0x9b0
> handle_stripe+0x32fb/0x4500
> handle_active_stripes.isra.0+0x6e0/0xa50
> raid5d+0x7e0/0xba0
> md_thread+0x15a/0x2d0
> kthread+0x1e3/0x220
> ret_from_fork+0x37a/0x410
> ret_from_fork_asm+0x1a/0x30
>
> value changed: 0x0000000000000019 -> 0x0000000000000099 --> R5_Overlap
>
> Fixes: fb642b92c267b
Fix this tag and applied to md-7.2
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
> drivers/md/raid5.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index a376560be92e..5521051a9425 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -4939,30 +4939,30 @@ static void break_stripe_batch_list(struct stripe_head *head_sh,
> state & (1 << STRIPE_INSYNC));
>
> sh->check_state = head_sh->check_state;
> sh->reconstruct_state = head_sh->reconstruct_state;
> spin_lock_irq(&sh->stripe_lock);
> - sh->batch_head = NULL;
> - spin_unlock_irq(&sh->stripe_lock);
> for (i = 0; i < sh->disks; i++) {
> if (test_and_clear_bit(R5_Overlap, &sh->dev[i].flags))
> wake_up_bit(&sh->dev[i].flags, R5_Overlap);
> - sh->dev[i].flags = head_sh->dev[i].flags &
> + sh->dev[i].flags = READ_ONCE(head_sh->dev[i].flags) &
> (~((1 << R5_WriteError) | (1 << R5_Overlap)));
> }
> + sh->batch_head = NULL;
> + spin_unlock_irq(&sh->stripe_lock);
>
> state = READ_ONCE(sh->state);
> if (handle_flags == 0 || (state & handle_flags))
> set_bit(STRIPE_HANDLE, &sh->state);
> raid5_release_stripe(sh);
> }
> spin_lock_irq(&head_sh->stripe_lock);
> - head_sh->batch_head = NULL;
> - spin_unlock_irq(&head_sh->stripe_lock);
> for (i = 0; i < head_sh->disks; i++)
> if (test_and_clear_bit(R5_Overlap, &head_sh->dev[i].flags))
> wake_up_bit(&head_sh->dev[i].flags, R5_Overlap);
> + head_sh->batch_head = NULL;
> + spin_unlock_irq(&head_sh->stripe_lock);
>
> state = READ_ONCE(head_sh->state);
> if (state & handle_flags)
> set_bit(STRIPE_HANDLE, &head_sh->state);
> }
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH v2] md/raid5: read batch_head under stripe_lock in make_stripe_request
From: yu kuai @ 2026-06-20 22:01 UTC (permalink / raw)
To: Chen Cheng, linux-raid, yukuai; +Cc: linux-kernel
In-Reply-To: <20260619081109.1218112-1-chencheng@fnnas.com>
Hi,
在 2026/6/19 16:11, Chen Cheng 写道:
> From: Chen Cheng <chencheng@fnnas.com>
>
> KCSAN reports race in raid5_make_request() vs. stripe_add_to_batch_list()
>
> Writer flow (stripe_add_to_batch_list):
> 1. grab `head` stripe;
> 2. lock_two_stripes(head, sh);
> 3. re-check stripe_can_batch() for both head and sh, which requires
> STRIPE_BATCH_READY set on both;
> 4. write head->batch_head = head and sh->batch_head = head;
> 5. unlock_two_stripes.
>
> STRIPE_BATCH_READY is cleared in two places:
> - clear_batch_ready(), at the entry of handle_stripe();
> - __add_stripe_bio(), for non-batchable bios.
> And, both need to acquire `stripe_lock`.
>
> Under stripe_lock, if STRIPE_BATCH_READY is clear, then:
> - New writers cannot install a batch_head;
> - Existing writers have already finished.
> So .. handle_stripe() readers can ready `batch_head` locklessly.
This does not explain the race clearly, I still have no clue yet.
>
> Fix way:
> Writer side make_stripe_request() under STRIPE_BATCH_READY, so , need
> to be protected by stripe_lock when read something..
>
> v1 -> v2:
> - re-expalin how stripe_lock and batch_head work in commit message , and ,
> - modify comment in raid5.h.
>
> Fixs: f4aec6a097387
Weird fix tag again.
>
>
> KCSAN report:
> ======================================
> BUG: KCSAN: data-race in raid5_make_request / raid5_make_request
>
> write to 0xffff8f03062432d8 of 8 bytes by task 210246 on cpu 6:
> raid5_make_request+0x175e/0x2ab0
> md_handle_request+0x2c5/0x700
> md_submit_bio+0x126/0x320
> [.........]
> btrfs_sync_file+0x181/0x970
> vfs_fsync_range+0x71/0x110
> do_fsync+0x46/0xa0
> __x64_sys_fsync+0x20/0x30
>
> read to 0xffff8f03062432d8 of 8 bytes by task 210251 on cpu 0:
> raid5_make_request+0x7c7/0x2ab0
> md_handle_request+0x2c5/0x700
> md_submit_bio+0x126/0x320
> [.........]
> btrfs_remap_file_range+0x266/0x980
> vfs_clone_file_range+0x16d/0x610
> ioctl_file_clone+0x64/0xd0
> do_vfs_ioctl+0x87f/0xbc0
> __x64_sys_ioctl+0xb8/0x130
>
> value changed: 0x0000000000000000 -> 0xffff8f0307798728
Is this a mismatch report?
>
>
> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
> ---
> drivers/md/raid5.c | 2 ++
> drivers/md/raid5.h | 8 +++++++-
> 2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 5521051a9425..efc63740f867 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -6108,14 +6108,16 @@ static enum stripe_result make_stripe_request(struct mddev *mddev,
> ctx->do_flush = false;
> }
>
> set_bit(STRIPE_HANDLE, &sh->state);
> clear_bit(STRIPE_DELAYED, &sh->state);
> + spin_lock_irq(&sh->stripe_lock);
> if ((!sh->batch_head || sh == sh->batch_head) &&
> (bi->bi_opf & REQ_SYNC) &&
> !test_and_set_bit(STRIPE_PREREAD_ACTIVE, &sh->state))
> atomic_inc(&conf->preread_active_stripes);
> + spin_unlock_irq(&sh->stripe_lock);
>
> release_stripe_plug(mddev, sh);
> return STRIPE_SUCCESS;
>
> out_release:
> diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
> index 1c7b710fc9c1..9ff825697ba3 100644
> --- a/drivers/md/raid5.h
> +++ b/drivers/md/raid5.h
> @@ -221,11 +221,17 @@ struct stripe_head {
> enum reconstruct_states reconstruct_state;
> spinlock_t stripe_lock;
> int cpu;
> struct r5worker_group *group;
>
> - struct stripe_head *batch_head; /* protected by stripe lock */
> + /*
> + * Writer protected by stripe_lock.
> + * Reader hold stripe_lock when STRIPE_BATCH_READY is set.
> + * Without STRIPE_BATCH_READY means no concurrent write,
> + * lockless read is ok.
> + */
> + struct stripe_head *batch_head;
> spinlock_t batch_lock; /* only header's lock is useful */
> struct list_head batch_list; /* protected by head's batch lock*/
>
> union {
> struct r5l_io_unit *log_io;
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH v2 2/4] md/raid10: fix writes_pending leak on write request failures
From: Abd-Alrhman Masalkhi @ 2026-06-21 11:05 UTC (permalink / raw)
To: yu kuai, sashiko-reviews; +Cc: linux-raid, yukuai
In-Reply-To: <ecaef394-23a2-43ec-a85b-2e25786914aa@fygo.io>
Hi Kaui,
On Sun, Jun 21, 2026 at 02:25 +0800, yu kuai wrote:
> Hi,
>
> 在 2026/6/14 2:40, sashiko-bot@kernel.org 写道:
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>>
>> Pre-existing issues:
>> - [High] The `r10_bio` structure allocated from the mempool is leaked when `raid10_write_request()` bails out early due to `REQ_NOWAIT`.
>> --
> This looks like a real issue, do you plan to fix this as well?
Yes, I plan to fix that issue as well. I'll include this fix along with
the other patches.
>>
>> commit 3940ccdb4c7c7e19c20f9b5e2e1d12545c2558b5
>> Author: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>>
>> md/raid10: fix writes_pending leak on write request failures
>>
>> raid10_make_request() acquires a writes_pending reference with
>> md_write_start() before dispatching write requests. Several failure
>> paths in raid10_write_request() complete the bio and return without
>> reaching the normal write completion path, causing the corresponding
>> md_write_end() to be skipped.
>>
>>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>>> index cee5a253a281d..c123a8c76ddcd 100644
>>> --- a/drivers/md/raid10.c
>>> +++ b/drivers/md/raid10.c
>> [ ... ]
>>> @@ -1365,7 +1365,7 @@ static void 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);
>>> - return;
>>> + return false;
>> [Severity: High]
>> This isn't a bug introduced by this patch, but does this early return leak
>> the r10_bio structure?
>>
>> __make_request() allocates r10_bio using:
>> r10_bio = mempool_alloc(&conf->r10bio_pool, GFP_NOIO);
>>
>> It then passes it to raid10_write_request(). When the bio has REQ_NOWAIT set
>> during a clustered area resync, we complete the bio with an error and return
>> false without freeing the r10_bio. Could repeatedly triggering this exhaust
>> the r10bio_pool, causing deadlocks on subsequent operations?
>>
>> Note that the regular_request_wait() error path right below correctly calls
>> free_r10bio(r10_bio) before returning.
>>
>>> }
>>> for (;;) {
>> [ ... ]
>>> @@ -1398,7 +1398,7 @@ static void raid10_write_request(struct mddev *mddev, struct bio *bio,
>>> if (bio->bi_opf & REQ_NOWAIT) {
>>> allow_barrier(conf);
>>> bio_wouldblock_error(bio);
>>> - return;
>>> + return false;
>> [Severity: High]
>> This is a pre-existing issue, but does this path also leak the r10_bio
>> mempool allocation when waiting for a reshape metadata update?
>>
>> Similar to the clustered area resync case above, the r10_bio allocated in
>> __make_request() does not appear to be freed before returning here.
>>
>>> }
>>> mddev_add_trace_msg(conf->mddev,
>
> --
> Thanks,
> Kuai
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH] md/raid1: honor REQ_NOWAIT when waiting for behind writes
From: Abd-Alrhman Masalkhi @ 2026-06-21 18:08 UTC (permalink / raw)
To: Yu Kuai, song, yukuai, magiclinan, xiao, vverma, axboe
Cc: linux-raid, linux-kernel, yukuai
In-Reply-To: <61361f20-f81c-46c3-bdd2-3de24e90a0aa@fnnas.com>
Hi Kuai,
On Sun, Jun 21, 2026 at 04:18 +0800, Yu Kuai wrote:
> Hi,
>
> 在 2026/6/11 16:35, Abd-Alrhman Masalkhi 写道:
>> raid1 supports REQ_NOWAIT reads by avoiding waits in the barrier path
>> through wait_read_barrier(). However, a read can still block on a
>> WriteMostly device when the array uses a bitmap and there are
>> outstanding behind writes.
>>
>> In that case raid1 unconditionally calls wait_behind_writes(), which
>> may sleep until all behind writes complete. As a result, a REQ_NOWAIT
>> read can block despite the caller explicitly requesting non-blocking
>> behavior.
>>
>> This ensures that raid1 consistently honors REQ_NOWAIT reads across all
>> paths that may otherwise wait for behind writes.
>>
>> Fixes: 5aa705039c4f ("md: raid1 add nowait support")
>> Signed-off-by: Abd-Alrhman Masalkhi <abd.masalkhi@gmail.com>
>> ---
>> 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 | 10 +++++++---
>> 5 files changed, 24 insertions(+), 12 deletions(-)
>
> Applied to md-7.2.
>
> One note, remove nowait support is already in my to-do lists. There is a case
> that can't be handled for now, for example:
>
> 2 disk raid1, issue no wait write IO, rdev1 succeed while rdev2 failed. In this
> case, we don't know if rdev2 failed issue because nowait or disk really have
> badblocks. So we can't either record badblocks or issue again without nowait,
> for consequence, rdev1 and rdev2 will end up with different data.
>
> I think the best solution is to remove nowait support for mdraid. Feel free
> to cook a patch or if you have something else in mind.
>
What if on a partial nowait failure we just end the master bio as
success, but set NEEDED on that chunk's bitmap counter and start_sync()
picks it up for resync? That way we don't have to decide why rdev2
failed at all, resync just copies from rdev1 to rdev2 without nowait,
so if it's a real bad block, end_sync_write() records it then.
We kinda have the idea of ending the bio before the write lands on all
mirrors in write-behind already, though it's not quite the same, there
the deferred write still lands, here we ACK after it already failed and
lean on resync to redo it.
My worry is the loaded case: AGAIN under queue pressure isn't that rare,
so we might end up triggering resync frequently. Do you think that's
acceptable, or is dropping nowait better? Happy to prototype either way.
>>
>> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
>> index 028b9ca8ce52..1206e31f323a 100644
>> --- a/drivers/md/md-bitmap.c
>> +++ b/drivers/md/md-bitmap.c
>> @@ -2063,18 +2063,23 @@ static void bitmap_end_behind_write(struct mddev *mddev)
>> bitmap->mddev->bitmap_info.max_write_behind);
>> }
>>
>> -static void bitmap_wait_behind_writes(struct mddev *mddev)
>> +static bool bitmap_wait_behind_writes(struct mddev *mddev, bool nowait)
>> {
>> struct bitmap *bitmap = mddev->bitmap;
>>
>> /* wait for behind writes to complete */
>> if (bitmap && atomic_read(&bitmap->behind_writes) > 0) {
>> + if (nowait)
>> + return false;
>> +
>> pr_debug("md:%s: behind writes in progress - waiting to stop.\n",
>> mdname(mddev));
>> /* need to kick something here to make sure I/O goes? */
>> wait_event(bitmap->behind_wait,
>> atomic_read(&bitmap->behind_writes) == 0);
>> }
>> +
>> + return true;
>> }
>>
>> static void bitmap_destroy(struct mddev *mddev)
>> @@ -2084,7 +2089,7 @@ static void bitmap_destroy(struct mddev *mddev)
>> if (!bitmap) /* there was no bitmap */
>> return;
>>
>> - bitmap_wait_behind_writes(mddev);
>> + bitmap_wait_behind_writes(mddev, false);
>> if (!test_bit(MD_SERIALIZE_POLICY, &mddev->flags))
>> mddev_destroy_serial_pool(mddev, NULL);
>>
>> diff --git a/drivers/md/md-bitmap.h b/drivers/md/md-bitmap.h
>> index 214f623c7e79..f46674bdfeb9 100644
>> --- a/drivers/md/md-bitmap.h
>> +++ b/drivers/md/md-bitmap.h
>> @@ -98,7 +98,7 @@ struct bitmap_operations {
>>
>> void (*start_behind_write)(struct mddev *mddev);
>> void (*end_behind_write)(struct mddev *mddev);
>> - void (*wait_behind_writes)(struct mddev *mddev);
>> + bool (*wait_behind_writes)(struct mddev *mddev, bool nowait);
>>
>> md_bitmap_fn *start_write;
>> md_bitmap_fn *end_write;
>> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
>> index 1adc5b117821..5a4e2abaa757 100644
>> --- a/drivers/md/md-llbitmap.c
>> +++ b/drivers/md/md-llbitmap.c
>> @@ -1574,16 +1574,19 @@ static void llbitmap_end_behind_write(struct mddev *mddev)
>> wake_up(&llbitmap->behind_wait);
>> }
>>
>> -static void llbitmap_wait_behind_writes(struct mddev *mddev)
>> +static bool llbitmap_wait_behind_writes(struct mddev *mddev, bool nowait)
>> {
>> struct llbitmap *llbitmap = mddev->bitmap;
>>
>> - if (!llbitmap)
>> - return;
>> + if (llbitmap && atomic_read(&llbitmap->behind_writes) > 0) {
>> + if (nowait)
>> + return false;
>>
>> - wait_event(llbitmap->behind_wait,
>> - atomic_read(&llbitmap->behind_writes) == 0);
>> + wait_event(llbitmap->behind_wait,
>> + atomic_read(&llbitmap->behind_writes) == 0);
>> + }
>>
>> + return true;
>> }
>>
>> static ssize_t bits_show(struct mddev *mddev, char *page)
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index 096bb64e87bd..d1465bcd86c8 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -7050,7 +7050,7 @@ EXPORT_SYMBOL_GPL(md_stop_writes);
>> static void mddev_detach(struct mddev *mddev)
>> {
>> if (md_bitmap_enabled(mddev, false))
>> - mddev->bitmap_ops->wait_behind_writes(mddev);
>> + mddev->bitmap_ops->wait_behind_writes(mddev, false);
>> if (mddev->pers && mddev->pers->quiesce && !is_md_suspended(mddev)) {
>> mddev->pers->quiesce(mddev, 1);
>> mddev->pers->quiesce(mddev, 0);
>> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
>> index b1ed4cc6ade4..b2d7c13b64bd 100644
>> --- a/drivers/md/raid1.c
>> +++ b/drivers/md/raid1.c
>> @@ -1341,6 +1341,7 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
>> int max_sectors;
>> int rdisk;
>> bool r1bio_existed = !!r1_bio;
>> + bool nowait = bio->bi_opf & REQ_NOWAIT;
>>
>> /*
>> * An md cloned bio indicates we are in the error path.
>> @@ -1360,8 +1361,7 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
>> * Still need barrier for READ in case that whole
>> * array is frozen.
>> */
>> - if (!wait_read_barrier(conf, bio->bi_iter.bi_sector,
>> - bio->bi_opf & REQ_NOWAIT)) {
>> + if (!wait_read_barrier(conf, bio->bi_iter.bi_sector, nowait)) {
>> bio_wouldblock_error(bio);
>> return;
>> }
>> @@ -1402,7 +1402,11 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
>> * over-take any writes that are 'behind'
>> */
>> mddev_add_trace_msg(mddev, "raid1 wait behind writes");
>> - mddev->bitmap_ops->wait_behind_writes(mddev);
>> + if (!mddev->bitmap_ops->wait_behind_writes(mddev, nowait)) {
>> + bio_wouldblock_error(bio);
>> + set_bit(R1BIO_Returned, &r1_bio->state);
>> + goto err_handle;
>> + }
>> }
>>
>> if (max_sectors < bio_sectors(bio)) {
>
> --
> Thanks,
> Kuai
--
Best Regards,
Abd-Alrhman
^ permalink raw reply
* Re: [PATCH v2] md/raid5: protect bitmap batch counters aka seq_flush/seq_write consistency
From: Chen Cheng @ 2026-06-22 1:52 UTC (permalink / raw)
To: yukuai, linux-raid; +Cc: chenchneg33, linux-kernel
In-Reply-To: <d7011132-d213-4dea-9c1d-c8fd9d6e93e0@fygo.io>
在 2026/6/21 05:13, yu kuai 写道:
> Hi,
>
> 在 2026/6/17 22:28, Chen Cheng 写道:
>> From: Chen Cheng <chencheng@fnnas.com>
>>
>> kcsan detect race :
>> - raid5d() closes the current bitmap batch by updating
>> conf->seq_flush under conf->device_lock.
>> - __add_stripe_bio() read conf->seq_flush without that
>> lock when assigning sh->bm_seq.
>>
>> so, protect seq_flush/seq_write consistency for multiple CPUs by
>> READ_ONCE()/WRITE_ONCE() under the path without held device_lock.
>>
>> re-explain the stripe batch sequence number update flow:
>> 1. sh->bm_seq declare which batch number the stripe belongs to
>> when perform bitmap-related write.
>> ==> bm_seq = seq_flush+1
>>
>> 2. stripe be handled,
>> * if sh->bm_seq - conf->seq_write > 0, means the
>> batch stripes **newer than** the last written
>> batch, it cannot proceed yet, queued on bitmap_list.
>> * otherwise , has already proceed.
>>
>> 3. raid5d() `++seq_flush` to closes the current batch, means
>> * no more stripes join that old batch
>> * just-closed batch ready to write-out to disk
>>
>> 4. raid5d() calls bitmap hooks unplug() or writeout, then,
>> `++seq_write` to the same as bm_seq.
>>
>> - seq_flush - for producer, to close batches.
>> - seq_write - for consumer, the checkpoint number.
>>
>> the report:
>> ====================================
>> BUG: KCSAN: data-race in __add_stripe_bio / raid5d
>>
>> write to 0xffff88ba5625d470 of 4 bytes by task 82401 on cpu 0:
>> raid5d+0x1d9/0xba0
>> [.....]
>>
>> read to 0xffff88ba5625d470 of 4 bytes by task 82421 on cpu 8:
>> __add_stripe_bio+0x332/0x400
>> raid5_make_request+0x6ac/0x2930
>> md_handle_request+0x4a2/0xa40
>> md_submit_bio+0x109/0x1a0
>> __submit_bio+0x2ec/0x390
>> [.....]
>>
>> v1 -> v2:
>> - remove WRITE_ONCE(conf->seq_write) in held device_lock path.
>> - remove READ_ONCE(conf->seq_flush) in held device_lock path.
>>
>> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
>> ---
>> drivers/md/raid5.c | 9 +++++----
>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> index a320b71d7117..b2c5a1930841 100644
>> --- a/drivers/md/raid5.c
>> +++ b/drivers/md/raid5.c
>> @@ -3536,11 +3536,11 @@ static void __add_stripe_bio(struct stripe_head *sh, struct bio *bi,
>> pr_debug("added bi b#%llu to stripe s#%llu, disk %d, logical %llu\n",
>> (*bip)->bi_iter.bi_sector, sh->sector, dd_idx,
>> sh->dev[dd_idx].sector);
>>
>> if (conf->mddev->bitmap && firstwrite && !sh->batch_head) {
>> - sh->bm_seq = conf->seq_flush+1;
>> + sh->bm_seq = READ_ONCE(conf->seq_flush) + 1;
>> set_bit(STRIPE_BIT_DELAY, &sh->state);
>> }
>> }
>>
>> /*
>> @@ -5827,11 +5827,11 @@ static void make_discard_request(struct mddev *mddev, struct bio *bi)
>> md_write_inc(mddev, bi);
>> sh->overwrite_disks++;
>> }
>> spin_unlock_irq(&sh->stripe_lock);
>> if (conf->mddev->bitmap) {
>> - sh->bm_seq = conf->seq_flush + 1;
>> + sh->bm_seq = READ_ONCE(conf->seq_flush) + 1;
>> set_bit(STRIPE_BIT_DELAY, &sh->state);
>> }
>>
>> set_bit(STRIPE_HANDLE, &sh->state);
>> clear_bit(STRIPE_DELAYED, &sh->state);
>> @@ -6877,16 +6877,17 @@ static void raid5d(struct md_thread *thread)
>> clear_bit(R5_DID_ALLOC, &conf->cache_state);
>>
>> if (
>> !list_empty(&conf->bitmap_list)) {
>> /* Now is a good time to flush some bitmap updates */
>> - conf->seq_flush++;
>> + int seq = conf->seq_flush + 1;
>
> checkpatch should warn this about missing blank line after declaration.
ok.
>
>> + WRITE_ONCE(conf->seq_flush, seq);
>
> WRITE_ONCE(a, a + 1) is safe, you can see lots of examples.
Gotta re-use `conf->seq_flush` snapshot aka. `seq` below..
>
>> spin_unlock_irq(&conf->device_lock);
>> if (md_bitmap_enabled(mddev, true))
>> mddev->bitmap_ops->unplug(mddev, true);
>> spin_lock_irq(&conf->device_lock);
>> - conf->seq_write = conf->seq_flush;
>> + conf->seq_write = seq;
>> activate_bit_delay(conf, conf->temp_inactive_list);
>> }
>> raid5_activate_delayed(conf);
>>
>> while ((bio = remove_bio_from_retry(conf, &offset))) {
>
^ permalink raw reply
* Re: [PATCH v2] md/raid5: read batch_head under stripe_lock in make_stripe_request
From: Chen Cheng @ 2026-06-22 3:08 UTC (permalink / raw)
To: yukuai, linux-raid; +Cc: linux-kernel
In-Reply-To: <e958cb97-556a-4320-8e76-9aed18a435bc@fygo.io>
在 2026/6/21 06:01, yu kuai 写道:
> Hi,
>
> 在 2026/6/19 16:11, Chen Cheng 写道:
>> From: Chen Cheng <chencheng@fnnas.com>
>>
>> KCSAN reports race in raid5_make_request() vs. stripe_add_to_batch_list()
>>
>> Writer flow (stripe_add_to_batch_list):
>> 1. grab `head` stripe;
>> 2. lock_two_stripes(head, sh);
>> 3. re-check stripe_can_batch() for both head and sh, which requires
>> STRIPE_BATCH_READY set on both;
>> 4. write head->batch_head = head and sh->batch_head = head;
>> 5. unlock_two_stripes.
>>
>> STRIPE_BATCH_READY is cleared in two places:
>> - clear_batch_ready(), at the entry of handle_stripe();
>> - __add_stripe_bio(), for non-batchable bios.
>> And, both need to acquire `stripe_lock`.
>>
>> Under stripe_lock, if STRIPE_BATCH_READY is clear, then:
>> - New writers cannot install a batch_head;
>> - Existing writers have already finished.
>> So .. handle_stripe() readers can ready `batch_head` locklessly.
>
> This does not explain the race clearly, I still have no clue yet.
From the semantic correctness perspective, I think the lock is needed.
From the race consequence perspective, the worst consequence I can see
is that it could add to a batch member stripe. But
`conf->preread_active_stripes` should only add to batch head or lone stripe.
the scenario:
=========================
sh1 and sh2 are neighbor, wich means,
if sh1 start with sector X, then, sh2 start with sectorX + STRIPE_SECTORS,
CPU0 CPU1
make_stripe_request(sh2)
-> add_all_stripe_bios(sh2) make_stripe_request(sh2)
-> add_all_stripe_bios(sh2)
-> stripe_add_to_batch_list(sh2)
-> lock_two_stripes(sh1, sh2)
-> sh1->batch_head = sh1
-> sh2->batch_head = sh1
-> test_and_clear_bit(
STRIPE_PREREAD_ACTIVE,
&sh2->state)
-> unlock_two_stripes(sh1, sh2)
-> if ((!sh2->batch_head ||
sh2 == sh2->batch_head) &&
REQ_SYNC &&
!test_and_set_bit(STRIPE_PREREAD_ACTIVE, &sh2->state))
atomic_inc(&conf->preread_active_stripes)
After CPU2 batches 'sh', CPU1 can still treat it as a lone stripe and
charge preread_active_stripes. Since CPU2 has already run the follower
side compensation, the later increment has no matching decrement.
>
>>
>> Fix way:
>> Writer side make_stripe_request() under STRIPE_BATCH_READY, so , need
>> to be protected by stripe_lock when read something..
>>
>> v1 -> v2:
>> - re-expalin how stripe_lock and batch_head work in commit message , and ,
>> - modify comment in raid5.h.
>>
>> Fixs: f4aec6a097387
>
> Weird fix tag again.
>
>>
>>
>> KCSAN report:
>> ======================================
>> BUG: KCSAN: data-race in raid5_make_request / raid5_make_request
>>
>> write to 0xffff8f03062432d8 of 8 bytes by task 210246 on cpu 6:
>> raid5_make_request+0x175e/0x2ab0
>> md_handle_request+0x2c5/0x700
>> md_submit_bio+0x126/0x320
>> [.........]
>> btrfs_sync_file+0x181/0x970
>> vfs_fsync_range+0x71/0x110
>> do_fsync+0x46/0xa0
>> __x64_sys_fsync+0x20/0x30
>>
>> read to 0xffff8f03062432d8 of 8 bytes by task 210251 on cpu 0:
>> raid5_make_request+0x7c7/0x2ab0
>> md_handle_request+0x2c5/0x700
>> md_submit_bio+0x126/0x320
>> [.........]
>> btrfs_remap_file_range+0x266/0x980
>> vfs_clone_file_range+0x16d/0x610
>> ioctl_file_clone+0x64/0xd0
>> do_vfs_ioctl+0x87f/0xbc0
>> __x64_sys_ioctl+0xb8/0x130
>>
>> value changed: 0x0000000000000000 -> 0xffff8f0307798728
>
> Is this a mismatch report?
>
>>
>>
>> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
>> ---
>> drivers/md/raid5.c | 2 ++
>> drivers/md/raid5.h | 8 +++++++-
>> 2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>> index 5521051a9425..efc63740f867 100644
>> --- a/drivers/md/raid5.c
>> +++ b/drivers/md/raid5.c
>> @@ -6108,14 +6108,16 @@ static enum stripe_result make_stripe_request(struct mddev *mddev,
>> ctx->do_flush = false;
>> }
>>
>> set_bit(STRIPE_HANDLE, &sh->state);
>> clear_bit(STRIPE_DELAYED, &sh->state);
>> + spin_lock_irq(&sh->stripe_lock);
>> if ((!sh->batch_head || sh == sh->batch_head) &&
>> (bi->bi_opf & REQ_SYNC) &&
>> !test_and_set_bit(STRIPE_PREREAD_ACTIVE, &sh->state))
>> atomic_inc(&conf->preread_active_stripes);
>> + spin_unlock_irq(&sh->stripe_lock);
>>
>> release_stripe_plug(mddev, sh);
>> return STRIPE_SUCCESS;
>>
>> out_release:
>> diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
>> index 1c7b710fc9c1..9ff825697ba3 100644
>> --- a/drivers/md/raid5.h
>> +++ b/drivers/md/raid5.h
>> @@ -221,11 +221,17 @@ struct stripe_head {
>> enum reconstruct_states reconstruct_state;
>> spinlock_t stripe_lock;
>> int cpu;
>> struct r5worker_group *group;
>>
>> - struct stripe_head *batch_head; /* protected by stripe lock */
>> + /*
>> + * Writer protected by stripe_lock.
>> + * Reader hold stripe_lock when STRIPE_BATCH_READY is set.
>> + * Without STRIPE_BATCH_READY means no concurrent write,
>> + * lockless read is ok.
>> + */
>> + struct stripe_head *batch_head;
>> spinlock_t batch_lock; /* only header's lock is useful */
>> struct list_head batch_list; /* protected by head's batch lock*/
>>
>> union {
>> struct r5l_io_unit *log_io;
>
^ permalink raw reply
* Re: [PATCH v4 3/3] md/raid10: bound reused r10bio devs[] walks by used_nr_devs
From: Chen Cheng @ 2026-06-22 6:34 UTC (permalink / raw)
To: yukuai, linux-raid; +Cc: linux-kernel
In-Reply-To: <21def978-3c6e-4a9f-8c60-c213e760fb7c@fygo.io>
在 2026/6/21 04:02, yu kuai 写道:
> Hi,
>
> 在 2026/6/3 11:59, Chen Cheng 写道:
>> From: Chen Cheng <chencheng@fnnas.com>
>>
>> After reshape changes raid_disks, an in-flight r10bio from the old geometry
>> can still be completed or freed later. In that case, using the current
>> geometry to walk r10_bio->devs[] is unsafe. A failure was reproduced with a
>> simple write workload while reshaping a raid10 array from 4 disks to 5 disks.
>> e.g.:
>>
>> 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
>>
>> the sequence above can trigger:
>>
>> 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
>>
>> The buggy object was 200 bytes long, which matches an r10bio with space for
>> only four devs[] entries. However, put_all_bios() and find_bio_disk() walk
>> r10_bio->devs[] using the current conf->geo.raid_disks value. Once reshape
>> switches conf->geo.raid_disks from 4 to 5, an old 4-slot r10bio can be
>> completed or freed as if it had 5 slots, and the walk overruns devs[4]. The
> I don't understand, is this still possible after patch 1.
patch 1 and 2 just prepare work.
The true dangerous is in-flight r10bio which use old geometry.
And patch 3 record and use r10bio's own width to free r10bio's bios.
>> same stale-width mismatch can also surface during a 5-disk to 4-disk reshape.
>>
>> Track the number of valid devs[] entries in each reused r10bio with
>> used_nr_devs. Initialize it whenever an r10bio is prepared for regular I/O,
>> discard, or resync/recovery/reshape work, and use it to bound devs[] walks
>> in put_all_bios() and find_bio_disk().
>>
>> Signed-off-by: Chen Cheng <chencheng@fnnas.com>
>> ---
>> drivers/md/raid10.c | 8 ++++++--
>> drivers/md/raid10.h | 2 ++
>> 2 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>> index 5eca34432e63..f134b93fd593 100644
>> --- a/drivers/md/raid10.c
>> +++ b/drivers/md/raid10.c
>> @@ -273,11 +273,11 @@ static void r10buf_pool_free(void *__r10_bio, void *data)
>>
>> static void put_all_bios(struct r10conf *conf, struct r10bio *r10_bio)
>> {
>> int i;
>>
>> - for (i = 0; i < conf->geo.raid_disks; i++) {
>> + for (i = 0; i < r10_bio->used_nr_devs; i++) {
>> struct bio **bio = & r10_bio->devs[i].bio;
>> if (!BIO_SPECIAL(*bio))
>> bio_put(*bio);
>> *bio = NULL;
>> bio = &r10_bio->devs[i].repl_bio;
>> @@ -370,11 +370,11 @@ static int find_bio_disk(struct r10conf *conf, struct r10bio *r10_bio,
>> struct bio *bio, int *slotp, int *replp)
>> {
>> int slot;
>> int repl = 0;
>>
>> - for (slot = 0; slot < conf->geo.raid_disks; slot++) {
>> + for (slot = 0; slot < r10_bio->used_nr_devs; slot++) {
>> if (r10_bio->devs[slot].bio == bio)
>> break;
>> if (r10_bio->devs[slot].repl_bio == bio) {
>> repl = 1;
>> break;
>> @@ -1561,10 +1561,11 @@ static void __make_request(struct mddev *mddev, struct bio *bio, int sectors)
>>
>> r10_bio->mddev = mddev;
>> r10_bio->sector = bio->bi_iter.bi_sector;
>> r10_bio->state = 0;
>> r10_bio->read_slot = -1;
>> + r10_bio->used_nr_devs = conf->geo.raid_disks;
>> memset(r10_bio->devs, 0, sizeof(r10_bio->devs[0]) *
>> conf->geo.raid_disks);
>>
>> if (bio_data_dir(bio) == READ)
>> raid10_read_request(mddev, bio, r10_bio);
>> @@ -1749,10 +1750,11 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
>> 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;
>> + r10_bio->used_nr_devs = geo->raid_disks;
>> memset(r10_bio->devs, 0, sizeof(r10_bio->devs[0]) * geo->raid_disks);
>> wait_blocked_dev(mddev, r10_bio);
>>
>> /*
>> * For far layout it needs more than one r10bio to cover all regions.
>> @@ -3083,10 +3085,12 @@ static struct r10bio *raid10_alloc_init_r10buf(struct r10conf *conf)
>> test_bit(MD_RECOVERY_RESHAPE, &conf->mddev->recovery))
>> nalloc = conf->copies; /* resync */
>> else
>> nalloc = 2; /* recovery */
>>
>> + r10bio->used_nr_devs = nalloc;
>> +
>> for (i = 0; i < nalloc; i++) {
>> bio = r10bio->devs[i].bio;
>> rp = bio->bi_private;
>> bio_reset(bio, NULL, 0);
>> bio->bi_private = rp;
>> diff --git a/drivers/md/raid10.h b/drivers/md/raid10.h
>> index b711626a5db7..4751119f9770 100644
>> --- a/drivers/md/raid10.h
>> +++ b/drivers/md/raid10.h
>> @@ -125,10 +125,12 @@ struct r10bio {
>> struct bio *master_bio;
>> /*
>> * if the IO is in READ direction, then this is where we read
>> */
>> int read_slot;
>> + /* Used to bound devs[] walks when the object is reused. */
>> + unsigned int used_nr_devs;
>>
>> struct list_head retry_list;
>> /*
>> * if the IO is in WRITE direction, then multiple bios are used,
>> * one for each copy.
>
^ permalink raw reply
* Re: [PATCH v4 3/3] md/raid10: bound reused r10bio devs[] walks by used_nr_devs
From: yu kuai @ 2026-06-22 7:53 UTC (permalink / raw)
To: Chen Cheng, linux-raid; +Cc: linux-kernel, yukuai
In-Reply-To: <f877d9f4-4912-4331-8c14-e7726adf24bc@fnnas.com>
Hi，
在 2026/6/22 14:34, Chen Cheng 写道:
> The true dangerous is in-flight r10bio which use old geometry.
You didn't explain why there can be inflight r10bio when array is suspended,
if this can happen, this is the root cause need to be fixed.
--
Thanks,
Kuai
^ permalink raw reply
* [PATCH] md/raid5-ppl: convert pending_flushes from atomic_t to refcount_t
From: Sajal Gupta @ 2026-06-22 8:04 UTC (permalink / raw)
To: linux-raid, song
Cc: yukuai3, tomasz.majchrzak, linux-kernel, error27, skhan, me,
linux-kernel-mentees, Sajal Gupta
The old atomic_t based counter allowed ppl_do_flush() to continue using io
after it could already have been freed by ppl_io_unit_finished(), leading
to a use-after-free.
Convert pending_flushes from atomic_t to refcount_t with a proper ownership
model. The creator holds a reference for the duration of ppl_do_flush(),
and each submitted flush bio holds a reference until its endio callback
runs. This makes the io lifetime explicit and removes the need for the
second loop in ppl_do_flush().
Fixes: 1532d9e87e8b ("raid5-ppl: PPL support for disks with write-back cache enabled")
Reported-by: Dan Carpenter <error27@gmail.com>
Closes: https://lore.kernel.org/all/ajJF2wKYWRk4GGCK@stanley.mountain/
Signed-off-by: Sajal Gupta <sajal2005gupta@gmail.com>
---
drivers/md/raid5-ppl.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/md/raid5-ppl.c b/drivers/md/raid5-ppl.c
index a70cbec12ed0..157a89edd9c8 100644
--- a/drivers/md/raid5-ppl.c
+++ b/drivers/md/raid5-ppl.c
@@ -145,7 +145,7 @@ struct ppl_io_unit {
struct list_head stripe_list; /* stripes added to the io_unit */
atomic_t pending_stripes; /* how many stripes not written to raid */
- atomic_t pending_flushes; /* how many disk flushes are in progress */
+ refcount_t pending_flushes; /* how many disk flushes are in progress */
bool submitted; /* true if write to log started */
@@ -249,7 +249,7 @@ static struct ppl_io_unit *ppl_new_iounit(struct ppl_log *log,
INIT_LIST_HEAD(&io->log_sibling);
INIT_LIST_HEAD(&io->stripe_list);
atomic_set(&io->pending_stripes, 0);
- atomic_set(&io->pending_flushes, 0);
+ refcount_set(&io->pending_flushes, 1);
bio_init(&io->bio, log->rdev->bdev, io->biovec, PPL_IO_INLINE_BVECS,
REQ_OP_WRITE | REQ_FUA);
@@ -599,7 +599,7 @@ static void ppl_flush_endio(struct bio *bio)
bio_put(bio);
- if (atomic_dec_and_test(&io->pending_flushes)) {
+ if (refcount_dec_and_test(&io->pending_flushes)) {
ppl_io_unit_finished(io);
md_wakeup_thread(conf->mddev->thread);
}
@@ -611,11 +611,8 @@ static void ppl_do_flush(struct ppl_io_unit *io)
struct ppl_conf *ppl_conf = log->ppl_conf;
struct r5conf *conf = ppl_conf->mddev->private;
int raid_disks = conf->raid_disks;
- int flushed_disks = 0;
int i;
- atomic_set(&io->pending_flushes, raid_disks);
-
for_each_set_bit(i, &log->disk_flush_bitmap, raid_disks) {
struct md_rdev *rdev;
struct block_device *bdev = NULL;
@@ -632,20 +629,18 @@ static void ppl_do_flush(struct ppl_io_unit *io)
GFP_NOIO, &ppl_conf->flush_bs);
bio->bi_private = io;
bio->bi_end_io = ppl_flush_endio;
+ refcount_inc(&io->pending_flushes);
pr_debug("%s: dev: %ps\n", __func__, bio->bi_bdev);
submit_bio(bio);
- flushed_disks++;
}
}
log->disk_flush_bitmap = 0;
- for (i = flushed_disks ; i < raid_disks; i++) {
- if (atomic_dec_and_test(&io->pending_flushes))
- ppl_io_unit_finished(io);
- }
+ if (refcount_dec_and_test(&io->pending_flushes))
+ ppl_io_unit_finished(io);
}
static inline bool ppl_no_io_unit_submitted(struct r5conf *conf,
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] md/raid5-ppl: convert pending_flushes from atomic_t to refcount_t
From: Dan Carpenter @ 2026-06-22 8:42 UTC (permalink / raw)
To: Sajal Gupta
Cc: linux-raid, song, yukuai3, tomasz.majchrzak, linux-kernel, skhan,
me, linux-kernel-mentees
In-Reply-To: <20260622080656.22786-1-sajal2005gupta@gmail.com>
On Mon, Jun 22, 2026 at 01:34:32PM +0530, Sajal Gupta wrote:
> The old atomic_t based counter allowed ppl_do_flush() to continue using io
> after it could already have been freed by ppl_io_unit_finished(), leading
> to a use-after-free.
>
> Convert pending_flushes from atomic_t to refcount_t with a proper ownership
> model. The creator holds a reference for the duration of ppl_do_flush(),
> and each submitted flush bio holds a reference until its endio callback
> runs. This makes the io lifetime explicit and removes the need for the
> second loop in ppl_do_flush().
>
> Fixes: 1532d9e87e8b ("raid5-ppl: PPL support for disks with write-back cache enabled")
> Reported-by: Dan Carpenter <error27@gmail.com>
> Closes: https://lore.kernel.org/all/ajJF2wKYWRk4GGCK@stanley.mountain/
> Signed-off-by: Sajal Gupta <sajal2005gupta@gmail.com>
> ---
Have you tested this at all because it doesn't seem at all correct to
me...
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH] md/raid5-ppl: convert pending_flushes from atomic_t to refcount_t
From: Dan Carpenter @ 2026-06-22 8:43 UTC (permalink / raw)
To: Sajal Gupta
Cc: linux-raid, song, yukuai3, tomasz.majchrzak, linux-kernel, skhan,
me, linux-kernel-mentees
In-Reply-To: <ajj1WdhoW-Y3GE4Q@stanley.mountain>
On Mon, Jun 22, 2026 at 11:42:01AM +0300, Dan Carpenter wrote:
> On Mon, Jun 22, 2026 at 01:34:32PM +0530, Sajal Gupta wrote:
> > The old atomic_t based counter allowed ppl_do_flush() to continue using io
> > after it could already have been freed by ppl_io_unit_finished(), leading
> > to a use-after-free.
> >
> > Convert pending_flushes from atomic_t to refcount_t with a proper ownership
> > model. The creator holds a reference for the duration of ppl_do_flush(),
> > and each submitted flush bio holds a reference until its endio callback
> > runs. This makes the io lifetime explicit and removes the need for the
> > second loop in ppl_do_flush().
> >
> > Fixes: 1532d9e87e8b ("raid5-ppl: PPL support for disks with write-back cache enabled")
> > Reported-by: Dan Carpenter <error27@gmail.com>
> > Closes: https://lore.kernel.org/all/ajJF2wKYWRk4GGCK@stanley.mountain/
> > Signed-off-by: Sajal Gupta <sajal2005gupta@gmail.com>
> > ---
>
> Have you tested this at all because it doesn't seem at all correct to
> me...
How I imagined this would work would be:
patch 1: add a break statement to fix the use after free
patch 2: s/atomic_t/recount_t/
The difference between atomic_t and refcount_t is that refount_t warns
about overflows and underflows.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH] md/raid5-ppl: convert pending_flushes from atomic_t to refcount_t
From: Sajal Gupta @ 2026-06-22 10:28 UTC (permalink / raw)
To: error27
Cc: linux-raid, song, yukuai3, tomasz.majchrzak, linux-kernel, skhan,
me, linux-kernel-mentees
In-Reply-To: <ajj1yljwrUk0bZwp@stanley.mountain>
Hi Dan,
> Have you tested this at all because it doesn't seem at all correct to
> me...
I have only done compile test, sorry, I forgot to mention that.
> How I imagined this would work would be:
> patch 1: add a break statement to fix the use after free
> patch 2: s/atomic_t/recount_t/
> The difference between atomic_t and refcount_t is that refount_t warns
> about overflows and underflows.
I did it like this because that is the pattern mostly used in the codebase.
Simple s/atomic_t/recount_t/ would have
refcount_set(&io->pending_flushes, 0) in init
and then later refcount_set(&io->pending_flushes, raid_disks) in
ppl_do_flush, which is not the usual way. I am treating it as a proper reference
counter rather than a mechanical type swap.
If that is too invasive, I’ll rework it into the break fix first, and do
s/atomic_t/recount_t cleanup separately.
Thanks,
Sajal
^ permalink raw reply
* Re: [PATCH] md/raid5-ppl: convert pending_flushes from atomic_t to refcount_t
From: Dan Carpenter @ 2026-06-22 10:57 UTC (permalink / raw)
To: Sajal Gupta
Cc: linux-raid, song, yukuai3, tomasz.majchrzak, linux-kernel, skhan,
me, linux-kernel-mentees
In-Reply-To: <20260622102859.38034-1-sajal2005gupta@gmail.com>
On Mon, Jun 22, 2026 at 03:58:58PM +0530, Sajal Gupta wrote:
> Hi Dan,
>
> > Have you tested this at all because it doesn't seem at all correct to
> > me...
>
> I have only done compile test, sorry, I forgot to mention that.
>
> > How I imagined this would work would be:
> > patch 1: add a break statement to fix the use after free
> > patch 2: s/atomic_t/recount_t/
>
> > The difference between atomic_t and refcount_t is that refount_t warns
> > about overflows and underflows.
>
> I did it like this because that is the pattern mostly used in the codebase.
> Simple s/atomic_t/recount_t/ would have
> refcount_set(&io->pending_flushes, 0) in init
> and then later refcount_set(&io->pending_flushes, raid_disks) in
> ppl_do_flush, which is not the usual way. I am treating it as a proper reference
> counter rather than a mechanical type swap.
>
> If that is too invasive, I’ll rework it into the break fix first, and do
> s/atomic_t/recount_t cleanup separately.
Heh. Yeah... There is not a chance I would merge a patch like this
without testing.
It also really feels like an AI patch and you're supposed to say when you
use AI to generate patches.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH v4 3/3] md/raid10: bound reused r10bio devs[] walks by used_nr_devs
From: Chen Cheng @ 2026-06-22 11:13 UTC (permalink / raw)
To: yukuai, linux-raid; +Cc: linux-kernel
In-Reply-To: <a2f3a5da-71c3-4cff-ba97-db39e3ec4964@fygo.io>
在 2026/6/22 15:53, yu kuai 写道:
> Hi，
>
> 在 2026/6/22 14:34, Chen Cheng 写道:
>> The true dangerous is in-flight r10bio which use old geometry.
>
> You didn't explain why there can be inflight r10bio when array is suspended,
> if this can happen, this is the root cause need to be fixed.
>
one scenario is .
CPU A (softirq, raid_end_bio_io) CPU B (action_store)
================================ ===============================
bio_endio(master_bio)
md_end_clone_io
percpu_ref_put → 0
wait_event wakeup,
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 geo obj free into new pool
The fix, as your advise , raid_end_bio_io:
1. put_all_bios, mempool_free
2. bio_endio if returned
3. allow_barrier
would be replace with patch 03.
^ permalink raw reply
* Re: [PATCH] md/raid5-ppl: convert pending_flushes from atomic_t to refcount_t
From: Sajal Gupta @ 2026-06-22 11:28 UTC (permalink / raw)
To: error27; +Cc: linux-raid, song, linux-kernel, skhan, me, linux-kernel-mentees
In-Reply-To: <ajkVDGAdveCH_urr@stanley.mountain>
> Heh. Yeah... There is not a chance I would merge a patch like this
> without testing.
> It also really feels like an AI patch and you're supposed to say when you
> use AI to generate patches.
I wrote the code myself. I did use Grammarly to help with my grammar and
phrasing.
I'll send a v2 with just the break fix.
Thanks,
Sajal
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox