[PATCH 0/6] md: don't wait for q->limits_lock while md holds back I/O

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/ (local)

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

The last leg is mddev_stack_new_rdev() from ->hot_add_disk().  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, so md must not block on that lock while it is holding back the
I/O the freeze waits for.  That is the same hazard mddev_suspend()
already documents for reconfig_mutex.

The rule this series applies is that q->limits_lock nests outside both
reconfig_mutex and the suspend.  Where md cannot arrange that, because
it is called with reconfig_mutex already held or from the sync thread,
it takes the update with a trylock and does without one on a contended
pass.

Patches 1, 2 and 4 are plumbing with no functional change.  Patches 3
and 5 convert the two callers that cannot own an update.  Patch 6 does
the hoists, all in one patch because a mix of the two lock orders is an
ABBA.

Two callers still take q->limits_lock inside reconfig_mutex, both with
the array suspended, and patch 6 says why: ->start_reshape() from
action_store(), which suspends before flushing sync_work, and
raid*_run() -> queue_limits_set() from level_store(), which already
hangs on its own because it freezes the queue while suspended.  Both
need more restructuring than belongs here.

The patches are based on v7.3-rc2.

Tested there with a raid1 of two ram devices, fio in flight and a loop
writing queue/max_sectors_kb: 20 fail/remove/add cycles complete,
where the same test wedges the array before the series.  Every patch
builds on its own.  A reshape and a level change are not covered by that
test.

The reproducer, for anyone who wants it:

  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 (6):
  block: add queue_limits_start_update_trylock()
  md: pass a queue_limits down to ->hot_add_disk()
  md: don't wait for q->limits_lock in check_sb_changes()
  md: pass a queue_limits through the rdev sysfs stores
  md: don't wait for q->limits_lock in mddev_update_io_opt()
  md: take q->limits_lock before locking and suspending the array

 drivers/md/dm-raid.c       |   2 +-
 drivers/md/md-autodetect.c |   2 +-
 drivers/md/md-linear.c     |   3 +-
 drivers/md/md.c            | 276 +++++++++++++++++++++++++++++++------
 drivers/md/md.h            |  11 +-
 drivers/md/raid1.c         |   8 +-
 drivers/md/raid10.c        |  17 ++-
 drivers/md/raid5.c         |  38 +++--
 include/linux/blkdev.h     |  26 ++++
 9 files changed, 315 insertions(+), 68 deletions(-)


base-commit: df2908090cda368b01ff43709f51890076c56157
-- 
2.43.0