[PATCH v3 0/1] md: freeze the queue in mddev_suspend()

[Jack Wang <jinpu.wang@ionos.com>]

From: Jack Wang <jinpu.wang@cloud.ionos.com>

Writing to a queue limits attribute of an md array while a spare is
being re-added deadlocks the array.  I reported this earlier here:

  https://lore.kernel.org/linux-raid/CAMGffE=heGA3y8FjQ0Sm1jj-kd-=H9Y54WozKASSEZhc9UNKjA@mail.gmail.com/

Four tasks, one array:

  udev-worker    queue_attr_store() holds q->limits_lock, waits in
                 blk_mq_freeze_queue() for q_usage_counter to drain
  fio            holds a q_usage_counter reference, parked in
                 md_handle_request()'s is_suspended() loop
  mdadm          suspended the array, waits for reconfig_mutex
  md_start_sync  holds reconfig_mutex, waits for q->limits_lock

Since commit c99f66e4084a ("block: fix queue freeze vs limits lock
order in sysfs store methods") the sysfs store holds q->limits_lock
across the freeze.  A bio that is parked in md_handle_request() while
the array is suspended still holds a q_usage_counter reference, so that
freeze can never complete, and everything behind it stalls.

v2 reworked the lock ordering so that q->limits_lock is taken outside
reconfig_mutex and the suspend everywhere, which took eight patches.
Yu Kuai pointed out in his reply to v2 that the freeze is of little use
for bio based devices on its own, since it cannot guarantee that
in-flight bios have drained, and that the deadlock is really a
blk_mq_freeze_queue() running concurrently with mddev_suspend().  He
suggested freezing the queue in mddev_suspend() instead.  This version
does that, and replaces the whole of v2 with one patch.

With the queue frozen by the suspend, bios submitted while the array is
suspended block in bio_queue_enter() without holding a reference, so a
concurrent limits update can finish and release q->limits_lock.  No
lock ordering has to change.

The freeze cannot always wait for the queue to drain.  A writer waiting
on the suspend_lo/suspend_hi window and raid456 I/O waiting on an
interrupted reshape sit in submit_bio() holding a reference, and only
make progress once the suspend has completed.  In those two cases the
freeze is started but not waited for.  The original deadlock can still
happen there, the same as before the patch.  dm-raid has no gendisk and
is not frozen.

v2: https://lore.kernel.org/linux-raid/e2c36f31-d198-40a7-a18d-156e1fd6fbb2@fygo.io/
v1: https://lore.kernel.org/linux-raid/20260909063029.GC29874@lst.de/T/#t

Changes since v2:
 - Drop the series.  Freeze the queue in mddev_suspend() as suggested
   by Yu Kuai, instead of changing the q->limits_lock vs
   reconfig_mutex / disk->open_mutex ordering.
 - Skip the drain wait for the suspend_lo/suspend_hi window and for an
   interrupted raid456 reshape.
 - Do not freeze dm-raid, which has no gendisk.
 - Rebased on v7.4-rc1.

Testing, on a 6.12 based kernel with the same change, lockdep enabled
(PROVE_LOCKING, DEBUG_LOCK_ALLOC):

 - the reproducer below, 20 fail/remove/add cycles with fio in flight
   and a loop writing queue/max_sectors_kb, which wedges the array
   without the patch
 - a raid5 3 -> 4 reshape with fio writing, which moves the
   suspend_lo/suspend_hi window while writers are active, and
   mdadm --stop afterwards

No lockdep reports and no hung tasks.  I have not yet run the mdadm
test tests/25raid456-reshape-deadlock, dm-raid, or an upstream kernel
with this patch.

A blktests case covering this deadlock was sent separately:

  https://lore.kernel.org/all/20260910100916.1638378-1-jinpu.wang@ionos.com/

The reproducer:

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

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

  while :; do
      echo 128 > /sys/block/md111/queue/max_sectors_kb 2>/dev/null
  done &

  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

Jack Wang (1):
  md: freeze the queue in mddev_suspend()

 drivers/md/md.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)


base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.43.0