[PATCH v4 0/2] block, md: suspend md around queue limits updates

[Jack Wang]

From: Jack Wang <jinpu.wang@xxxxxxxxxxxxxxx>

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@xxxxxxxxxxxxxx/

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 parked in md_handle_request() while the array
is suspended still holds a q_usage_counter reference, so that freeze
never completes.

v3 froze the queue in mddev_suspend().  Review showed this deadlocks
against raid1d/raid10d resubmitting a split bio and against raid456 I/O
waiting on an interrupted reshape.  Following Yu Kuai's suggestion, v4
lets a bio based disk suspend itself instead of freezing its queue:

 - Patch 1 adds optional ->suspend() and ->resume() to
   block_device_operations.  queue_attr_store() and the integrity
   flag_store() call ->suspend() before taking q->limits_lock, commit the
   limits without a freeze, then call ->resume().
 - Patch 2 implements them for md with mddev_suspend() and
   mddev_resume().

The order is suspend, then q->limits_lock, as in mddev_update_io_opt().
A suspend already held by another task only nests.

Known limitation: md_submit_bio() reads the limits in
bio_split_to_limits() before taking active_io, so a bio already there
when the suspend completes can see a mix of old and new limits.
mddev_update_io_opt() has the same window.

Christoph, does a ->suspend()/->resume() pair in block_device_operations
fit what you had in mind?

v3: https://lore.kernel.org/linux-raid/20261102154847.198472-1-jinpu.wang@xxxxxxxxx/
v2: https://lore.kernel.org/linux-raid/e2c36f31-d198-40a7-a18d-156e1fd6fbb2@xxxxxxx/
v1: https://lore.kernel.org/linux-raid/20260909063029.GC29874@xxxxxx/T/#t

Changes since v3:
 - Drop the freeze in mddev_suspend(); add ->suspend()/->resume() to
   block_device_operations, as suggested by Yu Kuai.
 - Also cover the integrity flag_store() path.
 - Rebased on v7.4-rc1.

Testing: the reproducer below, 20 fail/remove/add cycles with fio in
flight and a loop writing queue/max_sectors_kb, on a 6.12 based kernel
with lockdep enabled.  No lockdep reports, no hung tasks.  That kernel
had the queue_attr_store() change only; the flag_store() change and the
rebase to v7.4-rc1 are build tested only.

A blktests case for this deadlock was sent separately:

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

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 (2):
  block: suspend bio based disks around queue limits updates
  md: suspend the array instead of freezing the queue for limits updates

 block/blk-integrity.c  | 10 ++++++++--
 block/blk-sysfs.c      | 19 +++++++++++++------
 block/blk.h            | 20 ++++++++++++++++++++
 drivers/md/md.c        | 12 ++++++++++++
 include/linux/blkdev.h |  7 +++++++
 5 files changed, 60 insertions(+), 8 deletions(-)


base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4