Re: [PATCH] md: only unlock mddev from action_store

[Guoqing Jiang <guoqing.jiang@linux.dev>]

On 6/6/22 5:36 PM, Xiao Ni wrote:
>>> @@ -4827,12 +4827,14 @@ action_store(struct mddev *mddev, const char
>>> *page, size_t len)
>>>                           clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
>>>                   if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery) &&
>>>                       mddev_lock(mddev) == 0) {
>>> +                       mddev_suspend(mddev);
>>>                           if (work_pending(&mddev->del_work))
>>>                                   flush_workqueue(md_misc_wq);
>>>                           if (mddev->sync_thread) {
>>>                                   set_bit(MD_RECOVERY_INTR, &mddev->recovery);
>>>                                   md_reap_sync_thread(mddev);
>>>                           }
>>> +                       mddev_resume(mddev);
>>>                           mddev_unlock(mddev);
>>>                   }
>>>           } else if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery))
>> The action actually means sync action which are for internal IO instead
>> of external IO, I suppose the semantic is different with above change.
> The original problem should be i/o happen when interrupting the sync thread?
> So the external i/o calls md_write_start and sets MD_SB_CHANGE_PENDING.
> Then raid5d->md_check_recovery can't update superblock and handle internal
> stripes. So the `echo idle` action is stuck.

My point is action_store shouldn't disturb external IO, it should only 
deal with
sync IO and relevant stuffs as the function is for sync_action node, no?

Thanks,
Guoqing