Revert "md: unlock mddev before reap sync_thread in action_store"
This reverts commit 9dfbdafda3
.
Because it will introduce a defect that sync_thread can be running while
MD_RECOVERY_RUNNING is cleared, which will cause some unexpected problems,
for example:
list_add corruption. prev->next should be next (ffff0001ac1daba0), but was ffff0000ce1a02a0. (prev=ffff0000ce1a02a0).
Call trace:
__list_add_valid+0xfc/0x140
insert_work+0x78/0x1a0
__queue_work+0x500/0xcf4
queue_work_on+0xe8/0x12c
md_check_recovery+0xa34/0xf30
raid10d+0xb8/0x900 [raid10]
md_thread+0x16c/0x2cc
kthread+0x1a4/0x1ec
ret_from_fork+0x10/0x18
This is because work is requeued while it's still inside workqueue:
t1: t2:
action_store
mddev_lock
if (mddev->sync_thread)
mddev_unlock
md_unregister_thread
// first sync_thread is done
md_check_recovery
mddev_try_lock
/*
* once MD_RECOVERY_DONE is set, new sync_thread
* can start.
*/
set_bit(MD_RECOVERY_RUNNING, &mddev->recovery)
INIT_WORK(&mddev->del_work, md_start_sync)
queue_work(md_misc_wq, &mddev->del_work)
test_and_set_bit(WORK_STRUCT_PENDING_BIT, ...)
// set pending bit
insert_work
list_add_tail
mddev_unlock
mddev_lock_nointr
md_reap_sync_thread
// MD_RECOVERY_RUNNING is cleared
mddev_unlock
t3:
// before queued work started from t2
md_check_recovery
// MD_RECOVERY_RUNNING is not set, a new sync_thread can be started
INIT_WORK(&mddev->del_work, md_start_sync)
work->data = 0
// work pending bit is cleared
queue_work(md_misc_wq, &mddev->del_work)
insert_work
list_add_tail
// list is corrupted
The above commit is reverted to fix the problem, the deadlock this
commit tries to fix will be fixed in following patches.
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
Signed-off-by: Song Liu <song@kernel.org>
Link: https://lore.kernel.org/r/20230529132037.2124527-2-yukuai1@huaweicloud.com
This commit is contained in:
Родитель
51d74ec9b6
Коммит
a865b96c51
|
@ -3725,7 +3725,6 @@ static int raid_message(struct dm_target *ti, unsigned int argc, char **argv,
|
|||
if (!strcasecmp(argv[0], "idle") || !strcasecmp(argv[0], "frozen")) {
|
||||
if (mddev->sync_thread) {
|
||||
set_bit(MD_RECOVERY_INTR, &mddev->recovery);
|
||||
md_unregister_thread(&mddev->sync_thread);
|
||||
md_reap_sync_thread(mddev);
|
||||
}
|
||||
} else if (decipher_sync_action(mddev, mddev->recovery) != st_idle)
|
||||
|
|
|
@ -4764,19 +4764,6 @@ action_store(struct mddev *mddev, const char *page, size_t len)
|
|||
if (work_pending(&mddev->del_work))
|
||||
flush_workqueue(md_misc_wq);
|
||||
if (mddev->sync_thread) {
|
||||
sector_t save_rp = mddev->reshape_position;
|
||||
|
||||
mddev_unlock(mddev);
|
||||
set_bit(MD_RECOVERY_INTR, &mddev->recovery);
|
||||
md_unregister_thread(&mddev->sync_thread);
|
||||
mddev_lock_nointr(mddev);
|
||||
/*
|
||||
* set RECOVERY_INTR again and restore reshape
|
||||
* position in case others changed them after
|
||||
* got lock, eg, reshape_position_store and
|
||||
* md_check_recovery.
|
||||
*/
|
||||
mddev->reshape_position = save_rp;
|
||||
set_bit(MD_RECOVERY_INTR, &mddev->recovery);
|
||||
md_reap_sync_thread(mddev);
|
||||
}
|
||||
|
@ -6176,7 +6163,6 @@ static void __md_stop_writes(struct mddev *mddev)
|
|||
flush_workqueue(md_misc_wq);
|
||||
if (mddev->sync_thread) {
|
||||
set_bit(MD_RECOVERY_INTR, &mddev->recovery);
|
||||
md_unregister_thread(&mddev->sync_thread);
|
||||
md_reap_sync_thread(mddev);
|
||||
}
|
||||
|
||||
|
@ -9327,7 +9313,6 @@ void md_check_recovery(struct mddev *mddev)
|
|||
* ->spare_active and clear saved_raid_disk
|
||||
*/
|
||||
set_bit(MD_RECOVERY_INTR, &mddev->recovery);
|
||||
md_unregister_thread(&mddev->sync_thread);
|
||||
md_reap_sync_thread(mddev);
|
||||
clear_bit(MD_RECOVERY_RECOVER, &mddev->recovery);
|
||||
clear_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
|
||||
|
@ -9363,7 +9348,6 @@ void md_check_recovery(struct mddev *mddev)
|
|||
goto unlock;
|
||||
}
|
||||
if (mddev->sync_thread) {
|
||||
md_unregister_thread(&mddev->sync_thread);
|
||||
md_reap_sync_thread(mddev);
|
||||
goto unlock;
|
||||
}
|
||||
|
@ -9443,7 +9427,8 @@ void md_reap_sync_thread(struct mddev *mddev)
|
|||
sector_t old_dev_sectors = mddev->dev_sectors;
|
||||
bool is_reshaped = false;
|
||||
|
||||
/* sync_thread should be unregistered, collect result */
|
||||
/* resync has finished, collect result */
|
||||
md_unregister_thread(&mddev->sync_thread);
|
||||
if (!test_bit(MD_RECOVERY_INTR, &mddev->recovery) &&
|
||||
!test_bit(MD_RECOVERY_REQUESTED, &mddev->recovery) &&
|
||||
mddev->degraded != mddev->raid_disks) {
|
||||
|
|
Загрузка…
Ссылка в новой задаче