On Tue, Nov 28, 2023 at 8:32 PM Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> wrote: > > From: Yu Kuai <yukuai3@xxxxxxxxxx> > > 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. > > Signed-off-by: Yu Kuai <yukuai3@xxxxxxxxxx> > Acked-by: Xiao Ni <xni@xxxxxxxxxx> Since we are shipping this via the md-fixes branch, we need a Fixes tag. > --- > drivers/md/md.c | 24 +++++++++++++----------- > 1 file changed, 13 insertions(+), 11 deletions(-) > > diff --git a/drivers/md/md.c b/drivers/md/md.c > index 5640a948086b..2d8e45a1af23 100644 > --- a/drivers/md/md.c > +++ b/drivers/md/md.c > @@ -6355,6 +6355,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); > @@ -6369,8 +6372,6 @@ static int md_set_readonly(struct mddev *mddev, struct block_device *bdev) > */ > md_wakeup_thread_directly(mddev->sync_thread); > > - 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)); > @@ -6383,29 +6384,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); > - } This change (move did_freeze, etc.) is not explained in the commit log. Is it just refactor? Thanks, Song > 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; > } > -- > 2.39.2 >