[BUG] md/raid1: deadlock between a queue limits sysfs store and a spare re-add

[BUG] md/raid1: deadlock between a queue limits sysfs store and a spare re-add

[Jinpu Wang]

Hi,

writing to a queue limit attribute of an md array while a spare is being
re-added deadlocks the array, with three tasks left unkillable in D state.
The host has to be rebooted to recover.

We hit this in production on 6.12.100 with RAID1 arrays, triggered by a
udev rule writing queue/max_sectors_kb.  Reading the code, v7.2 looks
affected too; see "Which versions" below for exactly what was tested and
what was not.

The cycle
=========

Three tasks, one array:

  udev-worker     queue_attr_store() holds q->limits_lock and waits in
                  blk_mq_freeze_queue() for q_usage_counter to drain

  fio             holds a q_usage_counter reference, waits in
                  md_handle_request()'s is_suspended() loop

  md_start_sync   holds mddev->suspended, waits for q->limits_lock

Nobody can proceed: the freeze needs the in-flight I/O to finish, that
I/O needs mddev->suspended cleared, and clearing it needs the spare add
to finish, which is blocked on the lock the first task holds.

The three legs in v7.2 (8d3ae59288f1):

block/blk-sysfs.c, queue_attr_store():

struct queue_limits lim = queue_limits_start_update(q);

res = entry->store_limit(disk, page, length, &lim);
if (res < 0) {
queue_limits_cancel_update(q);
return res;
}

res = queue_limits_commit_update_frozen(q, &lim);

queue_limits_start_update() takes q->limits_lock, and
queue_limits_commit_update_frozen() calls blk_mq_freeze_queue() with it
still held.  max_sectors_kb is a QUEUE_LIM_RW_ENTRY, so a plain

echo 1024 > /sys/block/mdN/queue/max_sectors_kb

reaches this path.

drivers/md/md.c, md_start_sync():

if (mddev->reshape_position == MaxSector &&
   md_spares_need_change(mddev)) {
suspend = true;
mddev_suspend(mddev, false);
}

mddev_lock_nointr(mddev);

and from there md_choose_sync_action() -> remove_and_add_spares() ->
->hot_add_disk() -> raid1_add_disk() -> mddev_stack_new_rdev(), which
does a blocking

lim = queue_limits_start_update(mddev->gendisk->queue);

while mddev->suspended is set.

drivers/md/md.c, md_handle_request(), where the in-flight I/O waits:

if (is_suspended(mddev, bio)) {
...
wait_event(mddev->sb_wait, !is_suspended(mddev, bio));

So md acquires q->limits_lock while holding a quiescing primitive that
blocks exactly the I/O a concurrent freeze is waiting to drain.

Backtraces
==========

From a 6.12.100 based kernel:

  INFO: task kworker/2:2 blocked for more than 184 seconds.
  Workqueue: md_misc md_start_sync [md_mod]
  Call Trace:
   __mutex_lock.constprop.0+0x31c/0x6d0
   mddev_stack_new_rdev+0x59/0x150 [md_mod]
   raid1_add_disk+0x97/0x180 [raid1]
   remove_and_add_spares+0xe8/0x230 [md_mod]
   md_start_sync+0x14c/0x3e0 [md_mod]
   process_one_work+0x162/0x370

  INFO: task (udev-worker) blocked for more than 184 seconds.
  Call Trace:
   blk_mq_freeze_queue_wait+0x9e/0xd0
   queue_limits_commit_update_frozen+0x12/0x40
   queue_attr_store+0xc9/0x1c0
   kernfs_fop_write_iter+0x133/0x220
   vfs_write+0x29c/0x450

  INFO: task fio blocked for more than 184 seconds.
  Call Trace:
   md_handle_request+0x10d/0x2b0 [md_mod]
   __submit_bio+0x23e/0x2f0
   submit_bio_noacct_nocheck+0x1a3/0x3c0
   blkdev_direct_IO+0x265/0x5d0

/proc/mdstat at that point, with the spare add never completing:

  md0 : active raid1 rnbd0[0] rnbd1[1](S)
        5238784 blocks super 1.2 [2/1] [U_]

Reproducer
==========

On a scratch machine, with two ram devices:

  mdadm -C /dev/md111 --force -e 1.2 --assume-clean -l 1 \
        --bitmap=internal -n 2 /dev/ram0 /dev/ram1

  # keep I/O in flight
  fio --direct=1 --rw=randrw --ioengine=libaio --iodepth=32 --numjobs=4 \
      --time_based=1 --runtime=180 --filename=/dev/md111 --name=repro &

  # stand in for the udev worker
  while :; do
      echo 128 > /sys/block/md111/queue/max_sectors_kb 2>/dev/null
  done &

  # drive spare re-adds
  for i in $(seq 20); do
      mdadm /dev/md111 --fail /dev/ram0
      mdadm /dev/md111 --remove /dev/ram0
      mdadm /dev/md111 --add /dev/ram0
      mdadm --wait /dev/md111
  done

It reproduced on the first iteration for us, though it is a race, so it
may need a few attempts on other machines.

Which versions
==============

Reproduced: 6.12.100 (distro kernel carrying the stable backport of
c99f66e4084a), RAID1, repeatedly, on several hosts.

Not reproduced, code inspection only: v7.2 (8d3ae59288f1).  All three
legs quoted above are from the v7.2 tree and are unchanged there, so it
looks affected, but we have not run the reproducer on a mainline build.
Happy to do that if it helps.

raid10 calls mddev_stack_new_rdev() from raid10_add_disk() in the same
way, so it looks exposed too; we have only tested raid1.

When it started
===============

Before commit c99f66e4084a ("block: fix queue freeze vs limits lock
order in sysfs store methods"), queue_attr_store() froze the queue first
and took limits_lock afterwards, so limits_lock was never held across the
freeze wait and this cycle could not form.  That commit moved the freeze
inside queue_limits_commit_update_frozen(), i.e. under limits_lock:

  Fixes: c99f66e4084a ("block: fix queue freeze vs limits lock order
in sysfs store methods")

That is not an argument for reverting it: it exists so sd_revalidate_disk()
can issue SCSI commands while holding the limits lock, which cannot work
on a frozen queue.  The md side is on the wrong side of the ordering the
block layer expects, as spelled out in commit 06a2ff603f1f ("loop: Fix
recently introduced lock inversion"): all block driver code takes
queue_limits_start_update() before freezing.  md instead takes a
quiescing primitive of its own first.

What we are running
===================

We fixed it on the md side, by taking the limits update in
md_start_sync() before mddev_suspend(), threading the queue_limits
through ->hot_add_disk() so the personality stacks into it without
taking the lock itself, and committing before resuming, so the update
still lands while the array is quiesced.  That removes the inversion
without touching the shared block layer code.

Two approaches we tried first and discarded, in case they save someone
the detour:

- mutex_trylock() in mddev_stack_new_rdev() with a retry on contention.
  A writer that keeps retaking limits_lock wins nearly every time, so
  the retry does not converge: we measured 1132 backoffs against 1
  success, the array staying degraded with an idle spare throughout, and
  md_check_recovery() suspending and resuming the array on every pass.

- Dropping limits_lock around the freeze wait inside
  queue_limits_commit_update_frozen().  This works, but it inverts the
  ordering the block layer has standardised on, and a concurrent update
  committing in the window is then silently overwritten.

We can post the md-side patch if that direction looks right, or defer to
whatever you prefer.  Reproducer script and the full logs are available.

Thanks,
Jack

Re: [BUG] md/raid1: deadlock between a queue limits sysfs store and a spare re-add

[Abd-Alrhman Masalkhi]

Hi Jack,

On Sun, Sep 06, 2026 at 06:58 +0200, Jinpu Wang wrote:
> Hi,
>
> writing to a queue limit attribute of an md array while a spare is being
> re-added deadlocks the array, with three tasks left unkillable in D state.
> The host has to be rebooted to recover.
>
> We hit this in production on 6.12.100 with RAID1 arrays, triggered by a
> udev rule writing queue/max_sectors_kb.  Reading the code, v7.2 looks
> affected too; see "Which versions" below for exactly what was tested and
> what was not.
>
> The cycle
> =========
>
> Three tasks, one array:
>
>   udev-worker     queue_attr_store() holds q->limits_lock and waits in
>                   blk_mq_freeze_queue() for q_usage_counter to drain
>
>   fio             holds a q_usage_counter reference, waits in
>                   md_handle_request()'s is_suspended() loop
>
>   md_start_sync   holds mddev->suspended, waits for q->limits_lock
>
> Nobody can proceed: the freeze needs the in-flight I/O to finish, that
> I/O needs mddev->suspended cleared, and clearing it needs the spare add
> to finish, which is blocked on the lock the first task holds.
>
> The three legs in v7.2 (8d3ae59288f1):
>
> block/blk-sysfs.c, queue_attr_store():
>
> struct queue_limits lim = queue_limits_start_update(q);
>
> res = entry->store_limit(disk, page, length, &lim);
> if (res < 0) {
> queue_limits_cancel_update(q);
> return res;
> }
>
> res = queue_limits_commit_update_frozen(q, &lim);
>
> queue_limits_start_update() takes q->limits_lock, and
> queue_limits_commit_update_frozen() calls blk_mq_freeze_queue() with it
> still held.  max_sectors_kb is a QUEUE_LIM_RW_ENTRY, so a plain
>
> echo 1024 > /sys/block/mdN/queue/max_sectors_kb
>
> reaches this path.
>
> drivers/md/md.c, md_start_sync():
>
> if (mddev->reshape_position == MaxSector &&
>    md_spares_need_change(mddev)) {
> suspend = true;
> mddev_suspend(mddev, false);
> }
>
> mddev_lock_nointr(mddev);
>
> and from there md_choose_sync_action() -> remove_and_add_spares() ->
> ->hot_add_disk() -> raid1_add_disk() -> mddev_stack_new_rdev(), which
> does a blocking
>
> lim = queue_limits_start_update(mddev->gendisk->queue);
>
> while mddev->suspended is set.
>
> drivers/md/md.c, md_handle_request(), where the in-flight I/O waits:
>
> if (is_suspended(mddev, bio)) {
> ...
> wait_event(mddev->sb_wait, !is_suspended(mddev, bio));
>
> So md acquires q->limits_lock while holding a quiescing primitive that
> blocks exactly the I/O a concurrent freeze is waiting to drain.
>
> Backtraces
> ==========
>
> From a 6.12.100 based kernel:
>
>   INFO: task kworker/2:2 blocked for more than 184 seconds.
>   Workqueue: md_misc md_start_sync [md_mod]
>   Call Trace:
>    __mutex_lock.constprop.0+0x31c/0x6d0
>    mddev_stack_new_rdev+0x59/0x150 [md_mod]
>    raid1_add_disk+0x97/0x180 [raid1]
>    remove_and_add_spares+0xe8/0x230 [md_mod]
>    md_start_sync+0x14c/0x3e0 [md_mod]
>    process_one_work+0x162/0x370
>
>   INFO: task (udev-worker) blocked for more than 184 seconds.
>   Call Trace:
>    blk_mq_freeze_queue_wait+0x9e/0xd0
>    queue_limits_commit_update_frozen+0x12/0x40
>    queue_attr_store+0xc9/0x1c0
>    kernfs_fop_write_iter+0x133/0x220
>    vfs_write+0x29c/0x450
>
>   INFO: task fio blocked for more than 184 seconds.
>   Call Trace:
>    md_handle_request+0x10d/0x2b0 [md_mod]
>    __submit_bio+0x23e/0x2f0
>    submit_bio_noacct_nocheck+0x1a3/0x3c0
>    blkdev_direct_IO+0x265/0x5d0
>
> /proc/mdstat at that point, with the spare add never completing:
>
>   md0 : active raid1 rnbd0[0] rnbd1[1](S)
>         5238784 blocks super 1.2 [2/1] [U_]
>
> Reproducer
> ==========
>
> On a scratch machine, with two ram devices:
>
>   mdadm -C /dev/md111 --force -e 1.2 --assume-clean -l 1 \
>         --bitmap=internal -n 2 /dev/ram0 /dev/ram1
>
>   # keep I/O in flight
>   fio --direct=1 --rw=randrw --ioengine=libaio --iodepth=32 --numjobs=4 \
>       --time_based=1 --runtime=180 --filename=/dev/md111 --name=repro &
>
>   # stand in for the udev worker
>   while :; do
>       echo 128 > /sys/block/md111/queue/max_sectors_kb 2>/dev/null
>   done &
>
>   # drive spare re-adds
>   for i in $(seq 20); do
>       mdadm /dev/md111 --fail /dev/ram0
>       mdadm /dev/md111 --remove /dev/ram0
>       mdadm /dev/md111 --add /dev/ram0
>       mdadm --wait /dev/md111
>   done
>
> It reproduced on the first iteration for us, though it is a race, so it
> may need a few attempts on other machines.
>
> Which versions
> ==============
>
> Reproduced: 6.12.100 (distro kernel carrying the stable backport of
> c99f66e4084a), RAID1, repeatedly, on several hosts.
>
> Not reproduced, code inspection only: v7.2 (8d3ae59288f1).  All three
> legs quoted above are from the v7.2 tree and are unchanged there, so it
> looks affected, but we have not run the reproducer on a mainline build.
> Happy to do that if it helps.
>
> raid10 calls mddev_stack_new_rdev() from raid10_add_disk() in the same
> way, so it looks exposed too; we have only tested raid1.
>
> When it started
> ===============
>
> Before commit c99f66e4084a ("block: fix queue freeze vs limits lock
> order in sysfs store methods"), queue_attr_store() froze the queue first
> and took limits_lock afterwards, so limits_lock was never held across the
> freeze wait and this cycle could not form.  That commit moved the freeze
> inside queue_limits_commit_update_frozen(), i.e. under limits_lock:
>
>   Fixes: c99f66e4084a ("block: fix queue freeze vs limits lock order
> in sysfs store methods")
>
> That is not an argument for reverting it: it exists so sd_revalidate_disk()
> can issue SCSI commands while holding the limits lock, which cannot work
> on a frozen queue.  The md side is on the wrong side of the ordering the
> block layer expects, as spelled out in commit 06a2ff603f1f ("loop: Fix
> recently introduced lock inversion"): all block driver code takes
> queue_limits_start_update() before freezing.  md instead takes a
> quiescing primitive of its own first.
>
> What we are running
> ===================
>
> We fixed it on the md side, by taking the limits update in
> md_start_sync() before mddev_suspend(), threading the queue_limits
> through ->hot_add_disk() so the personality stacks into it without
> taking the lock itself, and committing before resuming, so the update
> still lands while the array is quiesced.  That removes the inversion
> without touching the shared block layer code.
>

I am working on a bug in slot_store(). It calls remove_and_add_spares()
without suspending the array. If I just suspend the array without taking
care of q->limits_lock, it would  trigger the same issue you mentioned.
2- state_store() also calls remove_and_add_spares(), while the array is
suspended by rdev_attr_store(), so it would probably trigger the same
issue as well.

> Two approaches we tried first and discarded, in case they save someone
> the detour:
>
> - mutex_trylock() in mddev_stack_new_rdev() with a retry on contention.
>   A writer that keeps retaking limits_lock wins nearly every time, so
>   the retry does not converge: we measured 1132 backoffs against 1
>   success, the array staying degraded with an idle spare throughout, and
>   md_check_recovery() suspending and resuming the array on every pass.
>
> - Dropping limits_lock around the freeze wait inside
>   queue_limits_commit_update_frozen().  This works, but it inverts the
>   ordering the block layer has standardised on, and a concurrent update
>   committing in the window is then silently overwritten.
>
> We can post the md-side patch if that direction looks right, or defer to
> whatever you prefer.  Reproducer script and the full logs are available.
>
> Thanks,
> Jack
>

-- 
Best Regards,
Abd-Alrhman

Re: [BUG] md/raid1: deadlock between a queue limits sysfs store and a spare re-add

[Nilay Shroff]

On 9/6/26 10:28 AM, Jinpu Wang wrote:
> Hi,
> 
> writing to a queue limit attribute of an md array while a spare is being
> re-added deadlocks the array, with three tasks left unkillable in D state.
> The host has to be rebooted to recover.
> 
> We hit this in production on 6.12.100 with RAID1 arrays, triggered by a
> udev rule writing queue/max_sectors_kb.  Reading the code, v7.2 looks
> affected too; see "Which versions" below for exactly what was tested and
> what was not.
> 
> The cycle
> =========
> 
> Three tasks, one array:
> 
>    udev-worker     queue_attr_store() holds q->limits_lock and waits in
>                    blk_mq_freeze_queue() for q_usage_counter to drain
> 
>    fio             holds a q_usage_counter reference, waits in
>                    md_handle_request()'s is_suspended() loop
> 
>    md_start_sync   holds mddev->suspended, waits for q->limits_lock
> 
> Nobody can proceed: the freeze needs the in-flight I/O to finish, that
> I/O needs mddev->suspended cleared, and clearing it needs the spare add
> to finish, which is blocked on the lock the first task holds.
> 
> The three legs in v7.2 (8d3ae59288f1):
> 
> block/blk-sysfs.c, queue_attr_store():
> 
> struct queue_limits lim = queue_limits_start_update(q);
> 
> res = entry->store_limit(disk, page, length, &lim);
> if (res < 0) {
> queue_limits_cancel_update(q);
> return res;
> }
> 
> res = queue_limits_commit_update_frozen(q, &lim);
> 
> queue_limits_start_update() takes q->limits_lock, and
> queue_limits_commit_update_frozen() calls blk_mq_freeze_queue() with it
> still held.  max_sectors_kb is a QUEUE_LIM_RW_ENTRY, so a plain
> 
> echo 1024 > /sys/block/mdN/queue/max_sectors_kb
> 
> reaches this path.
> 
> drivers/md/md.c, md_start_sync():
> 
> if (mddev->reshape_position == MaxSector &&
>     md_spares_need_change(mddev)) {
> suspend = true;
> mddev_suspend(mddev, false);
> }
> 
> mddev_lock_nointr(mddev);
> 
> and from there md_choose_sync_action() -> remove_and_add_spares() ->
> ->hot_add_disk() -> raid1_add_disk() -> mddev_stack_new_rdev(), which
> does a blocking
> 
> lim = queue_limits_start_update(mddev->gendisk->queue);
> 
> while mddev->suspended is set.
> 
> drivers/md/md.c, md_handle_request(), where the in-flight I/O waits:
> 
> if (is_suspended(mddev, bio)) {
> ...
> wait_event(mddev->sb_wait, !is_suspended(mddev, bio));
> 
> So md acquires q->limits_lock while holding a quiescing primitive that
> blocks exactly the I/O a concurrent freeze is waiting to drain.
> 
> Backtraces
> ==========
> 
>  From a 6.12.100 based kernel:
> 
>    INFO: task kworker/2:2 blocked for more than 184 seconds.
>    Workqueue: md_misc md_start_sync [md_mod]
>    Call Trace:
>     __mutex_lock.constprop.0+0x31c/0x6d0
>     mddev_stack_new_rdev+0x59/0x150 [md_mod]
>     raid1_add_disk+0x97/0x180 [raid1]
>     remove_and_add_spares+0xe8/0x230 [md_mod]
>     md_start_sync+0x14c/0x3e0 [md_mod]
>     process_one_work+0x162/0x370
> 
>    INFO: task (udev-worker) blocked for more than 184 seconds.
>    Call Trace:
>     blk_mq_freeze_queue_wait+0x9e/0xd0
>     queue_limits_commit_update_frozen+0x12/0x40
>     queue_attr_store+0xc9/0x1c0
>     kernfs_fop_write_iter+0x133/0x220
>     vfs_write+0x29c/0x450
> 
>    INFO: task fio blocked for more than 184 seconds.
>    Call Trace:
>     md_handle_request+0x10d/0x2b0 [md_mod]
>     __submit_bio+0x23e/0x2f0
>     submit_bio_noacct_nocheck+0x1a3/0x3c0
>     blkdev_direct_IO+0x265/0x5d0
> 
> /proc/mdstat at that point, with the spare add never completing:
> 
>    md0 : active raid1 rnbd0[0] rnbd1[1](S)
>          5238784 blocks super 1.2 [2/1] [U_]
> 
> Reproducer
> ==========
> 
> On a scratch machine, with two ram devices:
> 
>    mdadm -C /dev/md111 --force -e 1.2 --assume-clean -l 1 \
>          --bitmap=internal -n 2 /dev/ram0 /dev/ram1
> 
>    # keep I/O in flight
>    fio --direct=1 --rw=randrw --ioengine=libaio --iodepth=32 --numjobs=4 \
>        --time_based=1 --runtime=180 --filename=/dev/md111 --name=repro &
> 
>    # stand in for the udev worker
>    while :; do
>        echo 128 > /sys/block/md111/queue/max_sectors_kb 2>/dev/null
>    done &
> 
>    # drive spare re-adds
>    for i in $(seq 20); do
>        mdadm /dev/md111 --fail /dev/ram0
>        mdadm /dev/md111 --remove /dev/ram0
>        mdadm /dev/md111 --add /dev/ram0
>        mdadm --wait /dev/md111
>    done
> 
> It reproduced on the first iteration for us, though it is a race, so it
> may need a few attempts on other machines.
> 
> Which versions
> ==============
> 
> Reproduced: 6.12.100 (distro kernel carrying the stable backport of
> c99f66e4084a), RAID1, repeatedly, on several hosts.
> 
> Not reproduced, code inspection only: v7.2 (8d3ae59288f1).  All three
> legs quoted above are from the v7.2 tree and are unchanged there, so it
> looks affected, but we have not run the reproducer on a mainline build.
> Happy to do that if it helps.
> 
> raid10 calls mddev_stack_new_rdev() from raid10_add_disk() in the same
> way, so it looks exposed too; we have only tested raid1.
> 
> When it started
> ===============
> 
> Before commit c99f66e4084a ("block: fix queue freeze vs limits lock
> order in sysfs store methods"), queue_attr_store() froze the queue first
> and took limits_lock afterwards, so limits_lock was never held across the
> freeze wait and this cycle could not form.  That commit moved the freeze
> inside queue_limits_commit_update_frozen(), i.e. under limits_lock:
> 
>    Fixes: c99f66e4084a ("block: fix queue freeze vs limits lock order
> in sysfs store methods")
> 
> That is not an argument for reverting it: it exists so sd_revalidate_disk()
> can issue SCSI commands while holding the limits lock, which cannot work
> on a frozen queue.  The md side is on the wrong side of the ordering the
> block layer expects, as spelled out in commit 06a2ff603f1f ("loop: Fix
> recently introduced lock inversion"): all block driver code takes
> queue_limits_start_update() before freezing.  md instead takes a
> quiescing primitive of its own first.
> 
> What we are running
> ===================
> 
> We fixed it on the md side, by taking the limits update in
> md_start_sync() before mddev_suspend(), threading the queue_limits
> through ->hot_add_disk() so the personality stacks into it without
> taking the lock itself, and committing before resuming, so the update
> still lands while the array is quiesced.  That removes the inversion
> without touching the shared block layer code.
> 
I think you nailed it. We settled on the locking order in the block layer
with commit c99f66e4084a ("block: fix queue freeze vs limits lock order in
sysfs store methods"), where we now acquire q->limits_lock before
freezing the queue. This ordering is followed by the block layer code that
updates queue limits, while md appears to be an exception since it still
uses mddev_suspend() to stall I/O instead of blk_mq_freeze_queue().

So to me, your proposed approach looks reasonable: acquire
q->limits_lock first and then call mddev_suspend(). This should avoid
the lock inversion described above while still allowing the queue limits
update to be performed while the array is quiesced.

> Two approaches we tried first and discarded, in case they save someone
> the detour:
> 
> - mutex_trylock() in mddev_stack_new_rdev() with a retry on contention.
>    A writer that keeps retaking limits_lock wins nearly every time, so
>    the retry does not converge: we measured 1132 backoffs against 1
>    success, the array staying degraded with an idle spare throughout, and
>    md_check_recovery() suspending and resuming the array on every pass.
> 
> - Dropping limits_lock around the freeze wait inside
>    queue_limits_commit_update_frozen().  This works, but it inverts the
>    ordering the block layer has standardised on, and a concurrent update
>    committing in the window is then silently overwritten.
> 
> We can post the md-side patch if that direction looks right, or defer to
> whatever you prefer.  Reproducer script and the full logs are available.
> 
I think you should send out the md-side changes so that others can review the
locking changes and comment on the approach.

Thanks,
--Nilay

Re: [BUG] md/raid1: deadlock between a queue limits sysfs store and a spare re-add

[Abd-Alrhman Masalkhi]

On Sun, Sep 06, 2026 at 12:13 +0200, Abd-Alrhman Masalkhi wrote:
> Hi Jack,
>
> On Sun, Sep 06, 2026 at 06:58 +0200, Jinpu Wang wrote:
>> Hi,
>>
>> writing to a queue limit attribute of an md array while a spare is being
>> re-added deadlocks the array, with three tasks left unkillable in D state.
>> The host has to be rebooted to recover.
>>
>> We hit this in production on 6.12.100 with RAID1 arrays, triggered by a
>> udev rule writing queue/max_sectors_kb.  Reading the code, v7.2 looks
>> affected too; see "Which versions" below for exactly what was tested and
>> what was not.
>>
>> The cycle
>> =========
>>
>> Three tasks, one array:
>>
>>   udev-worker     queue_attr_store() holds q->limits_lock and waits in
>>                   blk_mq_freeze_queue() for q_usage_counter to drain
>>
>>   fio             holds a q_usage_counter reference, waits in
>>                   md_handle_request()'s is_suspended() loop
>>
>>   md_start_sync   holds mddev->suspended, waits for q->limits_lock
>>
>> Nobody can proceed: the freeze needs the in-flight I/O to finish, that
>> I/O needs mddev->suspended cleared, and clearing it needs the spare add
>> to finish, which is blocked on the lock the first task holds.
>>
>> The three legs in v7.2 (8d3ae59288f1):
>>
>> block/blk-sysfs.c, queue_attr_store():
>>
>> struct queue_limits lim = queue_limits_start_update(q);
>>
>> res = entry->store_limit(disk, page, length, &lim);
>> if (res < 0) {
>> queue_limits_cancel_update(q);
>> return res;
>> }
>>
>> res = queue_limits_commit_update_frozen(q, &lim);
>>
>> queue_limits_start_update() takes q->limits_lock, and
>> queue_limits_commit_update_frozen() calls blk_mq_freeze_queue() with it
>> still held.  max_sectors_kb is a QUEUE_LIM_RW_ENTRY, so a plain
>>
>> echo 1024 > /sys/block/mdN/queue/max_sectors_kb
>>
>> reaches this path.
>>
>> drivers/md/md.c, md_start_sync():
>>
>> if (mddev->reshape_position == MaxSector &&
>>    md_spares_need_change(mddev)) {
>> suspend = true;
>> mddev_suspend(mddev, false);
>> }
>>
>> mddev_lock_nointr(mddev);
>>
>> and from there md_choose_sync_action() -> remove_and_add_spares() ->
>> ->hot_add_disk() -> raid1_add_disk() -> mddev_stack_new_rdev(), which
>> does a blocking
>>
>> lim = queue_limits_start_update(mddev->gendisk->queue);
>>
>> while mddev->suspended is set.
>>
>> drivers/md/md.c, md_handle_request(), where the in-flight I/O waits:
>>
>> if (is_suspended(mddev, bio)) {
>> ...
>> wait_event(mddev->sb_wait, !is_suspended(mddev, bio));
>>
>> So md acquires q->limits_lock while holding a quiescing primitive that
>> blocks exactly the I/O a concurrent freeze is waiting to drain.
>>
>> Backtraces
>> ==========
>>
>> From a 6.12.100 based kernel:
>>
>>   INFO: task kworker/2:2 blocked for more than 184 seconds.
>>   Workqueue: md_misc md_start_sync [md_mod]
>>   Call Trace:
>>    __mutex_lock.constprop.0+0x31c/0x6d0
>>    mddev_stack_new_rdev+0x59/0x150 [md_mod]
>>    raid1_add_disk+0x97/0x180 [raid1]
>>    remove_and_add_spares+0xe8/0x230 [md_mod]
>>    md_start_sync+0x14c/0x3e0 [md_mod]
>>    process_one_work+0x162/0x370
>>
>>   INFO: task (udev-worker) blocked for more than 184 seconds.
>>   Call Trace:
>>    blk_mq_freeze_queue_wait+0x9e/0xd0
>>    queue_limits_commit_update_frozen+0x12/0x40
>>    queue_attr_store+0xc9/0x1c0
>>    kernfs_fop_write_iter+0x133/0x220
>>    vfs_write+0x29c/0x450
>>
>>   INFO: task fio blocked for more than 184 seconds.
>>   Call Trace:
>>    md_handle_request+0x10d/0x2b0 [md_mod]
>>    __submit_bio+0x23e/0x2f0
>>    submit_bio_noacct_nocheck+0x1a3/0x3c0
>>    blkdev_direct_IO+0x265/0x5d0
>>
>> /proc/mdstat at that point, with the spare add never completing:
>>
>>   md0 : active raid1 rnbd0[0] rnbd1[1](S)
>>         5238784 blocks super 1.2 [2/1] [U_]
>>
>> Reproducer
>> ==========
>>
>> On a scratch machine, with two ram devices:
>>
>>   mdadm -C /dev/md111 --force -e 1.2 --assume-clean -l 1 \
>>         --bitmap=internal -n 2 /dev/ram0 /dev/ram1
>>
>>   # keep I/O in flight
>>   fio --direct=1 --rw=randrw --ioengine=libaio --iodepth=32 --numjobs=4 \
>>       --time_based=1 --runtime=180 --filename=/dev/md111 --name=repro &
>>
>>   # stand in for the udev worker
>>   while :; do
>>       echo 128 > /sys/block/md111/queue/max_sectors_kb 2>/dev/null
>>   done &
>>
>>   # drive spare re-adds
>>   for i in $(seq 20); do
>>       mdadm /dev/md111 --fail /dev/ram0
>>       mdadm /dev/md111 --remove /dev/ram0
>>       mdadm /dev/md111 --add /dev/ram0
>>       mdadm --wait /dev/md111
>>   done
>>
>> It reproduced on the first iteration for us, though it is a race, so it
>> may need a few attempts on other machines.
>>
>> Which versions
>> ==============
>>
>> Reproduced: 6.12.100 (distro kernel carrying the stable backport of
>> c99f66e4084a), RAID1, repeatedly, on several hosts.
>>
>> Not reproduced, code inspection only: v7.2 (8d3ae59288f1).  All three
>> legs quoted above are from the v7.2 tree and are unchanged there, so it
>> looks affected, but we have not run the reproducer on a mainline build.
>> Happy to do that if it helps.
>>
>> raid10 calls mddev_stack_new_rdev() from raid10_add_disk() in the same
>> way, so it looks exposed too; we have only tested raid1.
>>
>> When it started
>> ===============
>>
>> Before commit c99f66e4084a ("block: fix queue freeze vs limits lock
>> order in sysfs store methods"), queue_attr_store() froze the queue first
>> and took limits_lock afterwards, so limits_lock was never held across the
>> freeze wait and this cycle could not form.  That commit moved the freeze
>> inside queue_limits_commit_update_frozen(), i.e. under limits_lock:
>>
>>   Fixes: c99f66e4084a ("block: fix queue freeze vs limits lock order
>> in sysfs store methods")
>>
>> That is not an argument for reverting it: it exists so sd_revalidate_disk()
>> can issue SCSI commands while holding the limits lock, which cannot work
>> on a frozen queue.  The md side is on the wrong side of the ordering the
>> block layer expects, as spelled out in commit 06a2ff603f1f ("loop: Fix
>> recently introduced lock inversion"): all block driver code takes
>> queue_limits_start_update() before freezing.  md instead takes a
>> quiescing primitive of its own first.
>>
>> What we are running
>> ===================
>>
>> We fixed it on the md side, by taking the limits update in
>> md_start_sync() before mddev_suspend(), threading the queue_limits
>> through ->hot_add_disk() so the personality stacks into it without
>> taking the lock itself, and committing before resuming, so the update
>> still lands while the array is quiesced.  That removes the inversion
>> without touching the shared block layer code.
>>
>
> I am working on a bug in slot_store(). It calls remove_and_add_spares()
> without suspending the array. If I just suspend the array without taking
> care of q->limits_lock, it would  trigger the same issue you
> mentioned.

To clarify, I haven't submitted my patch for slot_store() yet. I was 
going to, but your report shows that won't be enough. I'll hold off on
my changes so I can use your queue_limits threading approach.

> 2- state_store() also calls remove_and_add_spares(), while the array is
> suspended by rdev_attr_store(), so it would probably trigger the same
> issue as well.
>
>> Two approaches we tried first and discarded, in case they save someone
>> the detour:
>>
>> - mutex_trylock() in mddev_stack_new_rdev() with a retry on contention.
>>   A writer that keeps retaking limits_lock wins nearly every time, so
>>   the retry does not converge: we measured 1132 backoffs against 1
>>   success, the array staying degraded with an idle spare throughout, and
>>   md_check_recovery() suspending and resuming the array on every pass.
>>
>> - Dropping limits_lock around the freeze wait inside
>>   queue_limits_commit_update_frozen().  This works, but it inverts the
>>   ordering the block layer has standardised on, and a concurrent update
>>   committing in the window is then silently overwritten.
>>
>> We can post the md-side patch if that direction looks right, or defer to
>> whatever you prefer.  Reproducer script and the full logs are available.
>>
>> Thanks,
>> Jack
>>
>
> -- 
> Best Regards,
> Abd-Alrhman

-- 
Best Regards,
Abd-Alrhman

Re: [BUG] md/raid1: deadlock between a queue limits sysfs store and a spare re-add

[Jinpu Wang]

Hi Nilay, hi Abd,

Thank you for the feedback. I will test my fix against the upstream
kernel and send it out later today.

Thanks,
Jack

On Sun, Sep 6, 2026 at 2:33 PM Nilay Shroff <nilay@linux.ibm.com> wrote:
>
> On 9/6/26 10:28 AM, Jinpu Wang wrote:
> > Hi,
> >
> > writing to a queue limit attribute of an md array while a spare is being
> > re-added deadlocks the array, with three tasks left unkillable in D state.
> > The host has to be rebooted to recover.
> >
> > We hit this in production on 6.12.100 with RAID1 arrays, triggered by a
> > udev rule writing queue/max_sectors_kb.  Reading the code, v7.2 looks
> > affected too; see "Which versions" below for exactly what was tested and
> > what was not.
> >
> > The cycle
> > =========
> >
> > Three tasks, one array:
> >
> >    udev-worker     queue_attr_store() holds q->limits_lock and waits in
> >                    blk_mq_freeze_queue() for q_usage_counter to drain
> >
> >    fio             holds a q_usage_counter reference, waits in
> >                    md_handle_request()'s is_suspended() loop
> >
> >    md_start_sync   holds mddev->suspended, waits for q->limits_lock
> >
> > Nobody can proceed: the freeze needs the in-flight I/O to finish, that
> > I/O needs mddev->suspended cleared, and clearing it needs the spare add
> > to finish, which is blocked on the lock the first task holds.
> >
> > The three legs in v7.2 (8d3ae59288f1):
> >
> > block/blk-sysfs.c, queue_attr_store():
> >
> > struct queue_limits lim = queue_limits_start_update(q);
> >
> > res = entry->store_limit(disk, page, length, &lim);
> > if (res < 0) {
> > queue_limits_cancel_update(q);
> > return res;
> > }
> >
> > res = queue_limits_commit_update_frozen(q, &lim);
> >
> > queue_limits_start_update() takes q->limits_lock, and
> > queue_limits_commit_update_frozen() calls blk_mq_freeze_queue() with it
> > still held.  max_sectors_kb is a QUEUE_LIM_RW_ENTRY, so a plain
> >
> > echo 1024 > /sys/block/mdN/queue/max_sectors_kb
> >
> > reaches this path.
> >
> > drivers/md/md.c, md_start_sync():
> >
> > if (mddev->reshape_position == MaxSector &&
> >     md_spares_need_change(mddev)) {
> > suspend = true;
> > mddev_suspend(mddev, false);
> > }
> >
> > mddev_lock_nointr(mddev);
> >
> > and from there md_choose_sync_action() -> remove_and_add_spares() ->
> > ->hot_add_disk() -> raid1_add_disk() -> mddev_stack_new_rdev(), which
> > does a blocking
> >
> > lim = queue_limits_start_update(mddev->gendisk->queue);
> >
> > while mddev->suspended is set.
> >
> > drivers/md/md.c, md_handle_request(), where the in-flight I/O waits:
> >
> > if (is_suspended(mddev, bio)) {
> > ...
> > wait_event(mddev->sb_wait, !is_suspended(mddev, bio));
> >
> > So md acquires q->limits_lock while holding a quiescing primitive that
> > blocks exactly the I/O a concurrent freeze is waiting to drain.
> >
> > Backtraces
> > ==========
> >
> >  From a 6.12.100 based kernel:
> >
> >    INFO: task kworker/2:2 blocked for more than 184 seconds.
> >    Workqueue: md_misc md_start_sync [md_mod]
> >    Call Trace:
> >     __mutex_lock.constprop.0+0x31c/0x6d0
> >     mddev_stack_new_rdev+0x59/0x150 [md_mod]
> >     raid1_add_disk+0x97/0x180 [raid1]
> >     remove_and_add_spares+0xe8/0x230 [md_mod]
> >     md_start_sync+0x14c/0x3e0 [md_mod]
> >     process_one_work+0x162/0x370
> >
> >    INFO: task (udev-worker) blocked for more than 184 seconds.
> >    Call Trace:
> >     blk_mq_freeze_queue_wait+0x9e/0xd0
> >     queue_limits_commit_update_frozen+0x12/0x40
> >     queue_attr_store+0xc9/0x1c0
> >     kernfs_fop_write_iter+0x133/0x220
> >     vfs_write+0x29c/0x450
> >
> >    INFO: task fio blocked for more than 184 seconds.
> >    Call Trace:
> >     md_handle_request+0x10d/0x2b0 [md_mod]
> >     __submit_bio+0x23e/0x2f0
> >     submit_bio_noacct_nocheck+0x1a3/0x3c0
> >     blkdev_direct_IO+0x265/0x5d0
> >
> > /proc/mdstat at that point, with the spare add never completing:
> >
> >    md0 : active raid1 rnbd0[0] rnbd1[1](S)
> >          5238784 blocks super 1.2 [2/1] [U_]
> >
> > Reproducer
> > ==========
> >
> > On a scratch machine, with two ram devices:
> >
> >    mdadm -C /dev/md111 --force -e 1.2 --assume-clean -l 1 \
> >          --bitmap=internal -n 2 /dev/ram0 /dev/ram1
> >
> >    # keep I/O in flight
> >    fio --direct=1 --rw=randrw --ioengine=libaio --iodepth=32 --numjobs=4 \
> >        --time_based=1 --runtime=180 --filename=/dev/md111 --name=repro &
> >
> >    # stand in for the udev worker
> >    while :; do
> >        echo 128 > /sys/block/md111/queue/max_sectors_kb 2>/dev/null
> >    done &
> >
> >    # drive spare re-adds
> >    for i in $(seq 20); do
> >        mdadm /dev/md111 --fail /dev/ram0
> >        mdadm /dev/md111 --remove /dev/ram0
> >        mdadm /dev/md111 --add /dev/ram0
> >        mdadm --wait /dev/md111
> >    done
> >
> > It reproduced on the first iteration for us, though it is a race, so it
> > may need a few attempts on other machines.
> >
> > Which versions
> > ==============
> >
> > Reproduced: 6.12.100 (distro kernel carrying the stable backport of
> > c99f66e4084a), RAID1, repeatedly, on several hosts.
> >
> > Not reproduced, code inspection only: v7.2 (8d3ae59288f1).  All three
> > legs quoted above are from the v7.2 tree and are unchanged there, so it
> > looks affected, but we have not run the reproducer on a mainline build.
> > Happy to do that if it helps.
> >
> > raid10 calls mddev_stack_new_rdev() from raid10_add_disk() in the same
> > way, so it looks exposed too; we have only tested raid1.
> >
> > When it started
> > ===============
> >
> > Before commit c99f66e4084a ("block: fix queue freeze vs limits lock
> > order in sysfs store methods"), queue_attr_store() froze the queue first
> > and took limits_lock afterwards, so limits_lock was never held across the
> > freeze wait and this cycle could not form.  That commit moved the freeze
> > inside queue_limits_commit_update_frozen(), i.e. under limits_lock:
> >
> >    Fixes: c99f66e4084a ("block: fix queue freeze vs limits lock order
> > in sysfs store methods")
> >
> > That is not an argument for reverting it: it exists so sd_revalidate_disk()
> > can issue SCSI commands while holding the limits lock, which cannot work
> > on a frozen queue.  The md side is on the wrong side of the ordering the
> > block layer expects, as spelled out in commit 06a2ff603f1f ("loop: Fix
> > recently introduced lock inversion"): all block driver code takes
> > queue_limits_start_update() before freezing.  md instead takes a
> > quiescing primitive of its own first.
> >
> > What we are running
> > ===================
> >
> > We fixed it on the md side, by taking the limits update in
> > md_start_sync() before mddev_suspend(), threading the queue_limits
> > through ->hot_add_disk() so the personality stacks into it without
> > taking the lock itself, and committing before resuming, so the update
> > still lands while the array is quiesced.  That removes the inversion
> > without touching the shared block layer code.
> >
> I think you nailed it. We settled on the locking order in the block layer
> with commit c99f66e4084a ("block: fix queue freeze vs limits lock order in
> sysfs store methods"), where we now acquire q->limits_lock before
> freezing the queue. This ordering is followed by the block layer code that
> updates queue limits, while md appears to be an exception since it still
> uses mddev_suspend() to stall I/O instead of blk_mq_freeze_queue().
>
> So to me, your proposed approach looks reasonable: acquire
> q->limits_lock first and then call mddev_suspend(). This should avoid
> the lock inversion described above while still allowing the queue limits
> update to be performed while the array is quiesced.
>
> > Two approaches we tried first and discarded, in case they save someone
> > the detour:
> >
> > - mutex_trylock() in mddev_stack_new_rdev() with a retry on contention.
> >    A writer that keeps retaking limits_lock wins nearly every time, so
> >    the retry does not converge: we measured 1132 backoffs against 1
> >    success, the array staying degraded with an idle spare throughout, and
> >    md_check_recovery() suspending and resuming the array on every pass.
> >
> > - Dropping limits_lock around the freeze wait inside
> >    queue_limits_commit_update_frozen().  This works, but it inverts the
> >    ordering the block layer has standardised on, and a concurrent update
> >    committing in the window is then silently overwritten.
> >
> > We can post the md-side patch if that direction looks right, or defer to
> > whatever you prefer.  Reproducer script and the full logs are available.
> >
> I think you should send out the md-side changes so that others can review the
> locking changes and comment on the approach.
>
> Thanks,
> --Nilay
>
>