Skip to content

Commit 151f66b

Browse files
YuKuai-huaweiliu-song-6
authored andcommitted
md/raid5: fix deadlock that raid5d() wait for itself to clear MD_SB_CHANGE_PENDING
Xiao reported that lvm2 test lvconvert-raid-takeover.sh can hang with small possibility, the root cause is exactly the same as commit bed9e27 ("Revert "md/raid5: Wait for MD_SB_CHANGE_PENDING in raid5d"") However, Dan reported another hang after that, and junxiao investigated the problem and found out that this is caused by plugged bio can't issue from raid5d(). Current implementation in raid5d() has a weird dependence: 1) md_check_recovery() from raid5d() must hold 'reconfig_mutex' to clear MD_SB_CHANGE_PENDING; 2) raid5d() handles IO in a deadloop, until all IO are issued; 3) IO from raid5d() must wait for MD_SB_CHANGE_PENDING to be cleared; This behaviour is introduce before v2.6, and for consequence, if other context hold 'reconfig_mutex', and md_check_recovery() can't update super_block, then raid5d() will waste one cpu 100% by the deadloop, until 'reconfig_mutex' is released. Refer to the implementation from raid1 and raid10, fix this problem by skipping issue IO if MD_SB_CHANGE_PENDING is still set after md_check_recovery(), daemon thread will be woken up when 'reconfig_mutex' is released. Meanwhile, the hang problem will be fixed as well. Fixes: 5e2cf33 ("md/raid5: Wait for MD_SB_CHANGE_PENDING in raid5d") Cc: stable@vger.kernel.org # v5.19+ Reported-and-tested-by: Dan Moulding <dan@danm.net> Closes: https://lore.kernel.org/all/20240123005700.9302-1-dan@danm.net/ Investigated-by: Junxiao Bi <junxiao.bi@oracle.com> Signed-off-by: Yu Kuai <yukuai3@huawei.com> Link: https://lore.kernel.org/r/20240322081005.1112401-1-yukuai1@huaweicloud.com Signed-off-by: Song Liu <song@kernel.org>
1 parent 688c8b9 commit 151f66b

1 file changed

Lines changed: 3 additions & 12 deletions

File tree

drivers/md/raid5.c

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
*/
3737

3838
#include <linux/blkdev.h>
39-
#include <linux/delay.h>
4039
#include <linux/kthread.h>
4140
#include <linux/raid/pq.h>
4241
#include <linux/async_tx.h>
@@ -6734,6 +6733,9 @@ static void raid5d(struct md_thread *thread)
67346733
int batch_size, released;
67356734
unsigned int offset;
67366735

6736+
if (test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags))
6737+
break;
6738+
67376739
released = release_stripe_list(conf, conf->temp_inactive_list);
67386740
if (released)
67396741
clear_bit(R5_DID_ALLOC, &conf->cache_state);
@@ -6770,18 +6772,7 @@ static void raid5d(struct md_thread *thread)
67706772
spin_unlock_irq(&conf->device_lock);
67716773
md_check_recovery(mddev);
67726774
spin_lock_irq(&conf->device_lock);
6773-
6774-
/*
6775-
* Waiting on MD_SB_CHANGE_PENDING below may deadlock
6776-
* seeing md_check_recovery() is needed to clear
6777-
* the flag when using mdmon.
6778-
*/
6779-
continue;
67806775
}
6781-
6782-
wait_event_lock_irq(mddev->sb_wait,
6783-
!test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags),
6784-
conf->device_lock);
67856776
}
67866777
pr_debug("%d stripes handled\n", handled);
67876778

0 commit comments

Comments
 (0)