Hey,
On 2022-06-06 20:03, Guoqing Jiang wrote:
> Since the bug which commit 8b48ec23cc51a ("md: don't unregister sync_thread
> with reconfig_mutex held") fixed is related with action_store path, other
> callers which reap sync_thread didn't need to be changed.
>
> Let's pull md_unregister_thread from md_reap_sync_thread, then fix previous
> bug with belows.
>
> 1. unlock mddev before md_reap_sync_thread in action_store.
> 2. save reshape_position before unlock, then restore it to ensure position
> not changed accidentally by others.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@xxxxxxxxx>
> ---
> drivers/md/dm-raid.c | 1 +
> drivers/md/md.c | 12 ++++++++++--
> 2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
> index 9526ccbedafb..d43b8075c055 100644
> --- a/drivers/md/dm-raid.c
> +++ b/drivers/md/dm-raid.c
> @@ -3725,6 +3725,7 @@ 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)
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 2e83a19e3aba..4d70672f8ea8 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -4830,6 +4830,12 @@ 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);
> + md_unregister_thread(&mddev->sync_thread);
> + mddev_lock_nointr(mddev);
> + mddev->reshape_position = save_rp;
> set_bit(MD_RECOVERY_INTR, &mddev->recovery);
> md_reap_sync_thread(mddev);
> }
So with this patch, with 07revert-grow: I see the warning at the end of
this email. Seems there's a new bug in the hunk above: MD_RECOVERY_INTR
should be set before md_unregsiter_thread().
When I make that change, 07revert-grow fails in the same way it did with
the previous patch.
Logan
--
177.804352] ------------[ cut here ]------------
[ 177.805200] WARNING: CPU: 2 PID: 1382 at drivers/md/md.c:490
mddev_suspend+0x26c/0x330
[ 177.806541] Modules linked in:
[ 177.807078] CPU: 2 PID: 1382 Comm: md0_raid5 Not tainted
5.18.0-rc3-eid-vmlocalyes-dbg-00035-gd1d91cae9ac1 #2237
[ 177.809122] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
1.14.0-2 04/01/2014
[ 177.810649] RIP: 0010:mddev_suspend+0x26c/0x330
[ 177.811654] Code: ab 5c 13 00 00 e9 a3 fe ff ff 48 8d bb d8 02 00 00
be ff ff ff ff e8 d3 97 5f 00 85 c0 0f 85 74 fe ff ff 0f 0b e9 6d fe ff
ff <0f> 0b e9 4c fe ff ff 4c 8d bd 68 ff ff ff 31 f6 4c 89 ff e8 1c f7
[ 177.815028] RSP: 0018:ffff88828656faa8 EFLAGS: 00010246
[ 177.815995] RAX: 0000000000000000 RBX: ffff8881765e8000 RCX:
ffffffff9b3b1ed4
[ 177.817306] RDX: dffffc0000000000 RSI: ffff88817333e478 RDI:
ffff88816e861670
[ 177.818555] RBP: ffff88828656fb78 R08: ffffffff9b38e442 R09:
ffffffff9e94db07
[ 177.819757] R10: fffffbfff3d29b60 R11: 0000000000000001 R12:
ffff88816e861600
[ 177.821043] R13: ffff88829c34cf00 R14: ffff8881765e8000 R15:
ffff88817333e248
[ 177.822307] FS: 0000000000000000(0000) GS:ffff888472e00000(0000)
knlGS:0000000000000000
[ 177.823699] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 177.824701] CR2: 00007faa60ada843 CR3: 000000028e9ca002 CR4:
0000000000370ee0
[ 177.825889] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
[ 177.827111] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
0000000000000400
[ 177.828346] Call Trace:
[ 177.828785] <TASK>
[ 177.829275] ? rdev_read_only+0x150/0x150
[ 177.830180] ? lock_is_held_type+0xf3/0x150
[ 177.830888] ? raid5_calc_degraded+0x1f9/0x650
[ 177.831665] check_reshape+0x289/0x500
[ 177.832360] raid5_check_reshape+0x93/0x150
[ 177.833060] md_check_recovery+0x864/0xa80
[ 177.833757] raid5d+0xc6/0x930
[ 177.834272] ? __this_cpu_preempt_check+0x13/0x20
[ 177.835060] ? lock_downgrade+0x3f0/0x3f0
[ 177.835727] ? raid5_do_work+0x330/0x330
[ 177.836392] ? __this_cpu_preempt_check+0x13/0x20
[ 177.837313] ? lockdep_hardirqs_on+0x82/0x110
[ 177.838156] ? _raw_spin_unlock_irqrestore+0x31/0x60
[ 177.838977] ? trace_hardirqs_on+0x2b/0x110
[ 177.839678] ? preempt_count_sub+0x18/0xc0
[ 177.840383] ? _raw_spin_unlock_irqrestore+0x41/0x60
[ 177.841241] md_thread+0x1a2/0x2c0
[ 177.841836] ? suspend_lo_store+0x140/0x140
[ 177.842515] ? __this_cpu_preempt_check+0x13/0x20
[ 177.843290] ? lockdep_hardirqs_on+0x82/0x110
[ 177.844111] ? destroy_sched_domains_rcu+0x40/0x40
[ 177.844979] ? preempt_count_sub+0x18/0xc0
[ 177.845769] ? __kasan_check_read+0x11/0x20
[ 177.846514] ? suspend_lo_store+0x140/0x140
[ 177.847210] kthread+0x177/0x1b0
[ 177.847756] ? kthread_complete_and_exit+0x30/0x30
[ 177.848570] ret_from_fork+0x22/0x30
[ 177.849204] </TASK>
[ 177.849584] irq event stamp: 609927
[ 177.850163] hardirqs last enabled at (609935): [<ffffffff9a216ea4>]
__up_console_sem+0x64/0x70
[ 177.851601] hardirqs last disabled at (609942): [<ffffffff9a216e89>]
__up_console_sem+0x49/0x70
[ 177.853048] softirqs last enabled at (609546): [<ffffffff9a14751e>]
__irq_exit_rcu+0xfe/0x150
[ 177.854706] softirqs last disabled at (609499): [<ffffffff9a14751e>]
__irq_exit_rcu+0xfe/0x150
[ 177.856240] ---[ end trace 0000000000000000 ]---
