[PATCH] md/raid5: serialize plug list add with device_lock

[Li Youhong]

From: Li Youhong <liyouhong@xxxxxxxxxx>

raid5_unplug() can spin forever under conf->device_lock when
raid5_plug_cb.list still points at a stripe whose sh->lru has
already been reinitialized (self-looped). That disables IRQs on
the holder CPU and causes multi-CPU hard lockups on waiters of
the same lock.

This happens because release_stripe_plug() sets
STRIPE_ON_UNPLUG_LIST and list_add_tail(sh->lru) without
device_lock, while do_release_stripe() may concurrently move the
same lru onto handle/inactive when the last reference drops.

Note: a 2020 proposal tried extra refs / checking
STRIPE_ON_UNPLUG_LIST in do_release_stripe() without serializing
the plug enqueue:
https://lore.kernel.org/linux-raid/20200108163023.9301-1-guoqing.jiang@xxxxxxxxxxxxxxx/
That still leaves a TOCTOU window where the bit is clear during
the check and set afterwards, allowing two list_add on the same
lru. It was not merged.

Serialize the bit update and list_add with device_lock. If
do_release_stripe() still sees STRIPE_ON_UNPLUG_LIST, restore the
reference and let raid5_unplug() own the final release.

Observed on production 9-disk NVMe RAID5 under MySQL AIO
(io_submit -> blk_finish_plug -> raid5_unplug).

Fixes: 8811b5968f62 ("raid5: make_request use batch stripe release")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Li Youhong <liyouhong@xxxxxxxxxx>
---
 drivers/md/raid5.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index b91545ce090d..c9197b63b1c4 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -229,6 +229,17 @@ static void do_release_stripe(struct r5conf *conf, struct stripe_head *sh,
 	int i;
 	int injournal = 0;	/* number of date pages with R5_InJournal */
 
+	/*
+	 * Stripe is owned by release_stripe_plug()'s cb->list. A concurrent
+	 * last-ref release can reach here after the stripe was queued for
+	 * unplug (lru may already be non-empty). Do not re-add lru elsewhere;
+	 * restore the reference and let raid5_unplug() finish the release.
+	 */
+	if (test_bit(STRIPE_ON_UNPLUG_LIST, &sh->state)) {
+		atomic_inc(&sh->count);
+		return;
+	}
+
 	BUG_ON(!list_empty(&sh->lru));
 	BUG_ON(atomic_read(&conf->active_stripes)==0);
 
@@ -5760,6 +5771,9 @@ static void release_stripe_plug(struct mddev *mddev,
 		raid5_unplug, mddev,
 		sizeof(struct raid5_plug_cb));
 	struct raid5_plug_cb *cb;
+	struct r5conf *conf = mddev->private;
+	unsigned long flags;
+	bool queued = false;
 
 	if (!blk_cb) {
 		raid5_release_stripe(sh);
@@ -5775,9 +5789,22 @@ static void release_stripe_plug(struct mddev *mddev,
 			INIT_LIST_HEAD(cb->temp_inactive_list + i);
 	}
 
-	if (!test_and_set_bit(STRIPE_ON_UNPLUG_LIST, &sh->state))
-		list_add_tail(&sh->lru, &cb->list);
-	else
+	/*
+	 * Serialize with do_release_stripe() on device_lock so sh->lru cannot
+	 * be added to handle/inactive and cb->list at the same time.
+	 */
+	spin_lock_irqsave(&conf->device_lock, flags);
+	if (!test_and_set_bit(STRIPE_ON_UNPLUG_LIST, &sh->state)) {
+		if (unlikely(!list_empty(&sh->lru))) {
+			clear_bit(STRIPE_ON_UNPLUG_LIST, &sh->state);
+		} else {
+			list_add_tail(&sh->lru, &cb->list);
+			queued = true;
+		}
+	}
+	spin_unlock_irqrestore(&conf->device_lock, flags);
+
+	if (!queued)
 		raid5_release_stripe(sh);
 }
 
-- 
2.25.1