md: don't leave 'MD_RECOVERY_FROZEN' in error path of md_set_readonly()
authorYu Kuai <yukuai3@huawei.com>
Tue, 5 Dec 2023 09:42:14 +0000 (17:42 +0800)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Wed, 13 Dec 2023 17:39:17 +0000 (18:39 +0100)
[ Upstream commit c9f7cb5b2bc968adcdc686c197ed108f47fd8eb0 ]

If md_set_readonly() failed, the array could still be read-write, however
'MD_RECOVERY_FROZEN' could still be set, which leave the array in an
abnormal state that sync or recovery can't continue anymore.
Hence make sure the flag is cleared after md_set_readonly() returns.

Fixes: 88724bfa68be ("md: wait for pending superblock updates before switching to read-only")
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
Acked-by: Xiao Ni <xni@redhat.com>
Signed-off-by: Song Liu <song@kernel.org>
Link: https://lore.kernel.org/r/20231205094215.1824240-3-yukuai1@huaweicloud.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
drivers/md/md.c

index 246af78..0c2801d 100644 (file)
@@ -6314,6 +6314,9 @@ static int md_set_readonly(struct mddev *mddev, struct block_device *bdev)
        int err = 0;
        int did_freeze = 0;
 
+       if (mddev->external && test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags))
+               return -EBUSY;
+
        if (!test_bit(MD_RECOVERY_FROZEN, &mddev->recovery)) {
                did_freeze = 1;
                set_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
@@ -6326,8 +6329,6 @@ static int md_set_readonly(struct mddev *mddev, struct block_device *bdev)
                 * which will now never happen */
                wake_up_process(mddev->sync_thread->tsk);
 
-       if (mddev->external && test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags))
-               return -EBUSY;
        mddev_unlock(mddev);
        wait_event(resync_wait, !test_bit(MD_RECOVERY_RUNNING,
                                          &mddev->recovery));
@@ -6340,29 +6341,30 @@ static int md_set_readonly(struct mddev *mddev, struct block_device *bdev)
            mddev->sync_thread ||
            test_bit(MD_RECOVERY_RUNNING, &mddev->recovery)) {
                pr_warn("md: %s still in use.\n",mdname(mddev));
-               if (did_freeze) {
-                       clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
-                       set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
-                       md_wakeup_thread(mddev->thread);
-               }
                err = -EBUSY;
                goto out;
        }
+
        if (mddev->pers) {
                __md_stop_writes(mddev);
 
-               err  = -ENXIO;
-               if (mddev->ro == MD_RDONLY)
+               if (mddev->ro == MD_RDONLY) {
+                       err  = -ENXIO;
                        goto out;
+               }
+
                mddev->ro = MD_RDONLY;
                set_disk_ro(mddev->gendisk, 1);
+       }
+
+out:
+       if ((mddev->pers && !err) || did_freeze) {
                clear_bit(MD_RECOVERY_FROZEN, &mddev->recovery);
                set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
                md_wakeup_thread(mddev->thread);
                sysfs_notify_dirent_safe(mddev->sysfs_state);
-               err = 0;
        }
-out:
+
        mutex_unlock(&mddev->open_mutex);
        return err;
 }