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

Guoqing Jiang <guoqing.jiang@xxxxxxxxx> · Mon, 6 Jun 2022 17:55:06 +0800






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






