On Thu, Sep 28, 2023 at 5:55 AM Mariusz Tkaczyk
<mariusz.tkaczyk@xxxxxxxxxxxxxxx> wrote:
>
> We don't need to lock device to reject not supported request
> in array_state_store(). No functional changes intended.
>
> There are differences between ioctl and sysfs handling during stopping.
> With this change, it will be easier to add additional steps which needs
> to be completed before mddev_lock() is taken.
>
> Reviewed-by: Yu Kuai <yukuai3@xxxxxxxxxx>
> Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@xxxxxxxxxxxxxxx>
Applied to md-next. Thanks!
Song
> ---
> Song, feel free to remove second chapter if you think it is confusing.
>
> drivers/md/md.c | 37 ++++++++++++++++++++-----------------
> 1 file changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index b8f232840f7c..469b1376e66c 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -4359,6 +4359,18 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
> int err = 0;
> enum array_state st = match_word(buf, array_states);
>
> + /* No lock dependent actions */
> + switch (st) {
> + case suspended: /* not supported yet */
> + case write_pending: /* cannot be set */
> + case active_idle: /* cannot be set */
> + case broken: /* cannot be set */
> + case bad_word:
> + return -EINVAL;
> + default:
> + break;
> + }
> +
> if (mddev->pers && (st == active || st == clean) &&
> mddev->ro != MD_RDONLY) {
> /* don't take reconfig_mutex when toggling between
> @@ -4383,23 +4395,16 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
> err = mddev_lock(mddev);
> if (err)
> return err;
> - err = -EINVAL;
> - switch(st) {
> - case bad_word:
> - break;
> - case clear:
> - /* stopping an active array */
> - err = do_md_stop(mddev, 0, NULL);
> - break;
> +
> + switch (st) {
> case inactive:
> - /* stopping an active array */
> + /* stop an active array, return 0 otherwise */
> if (mddev->pers)
> err = do_md_stop(mddev, 2, NULL);
> - else
> - err = 0; /* already inactive */
> break;
> - case suspended:
> - break; /* not supported yet */
> + case clear:
> + err = do_md_stop(mddev, 0, NULL);
> + break;
> case readonly:
> if (mddev->pers)
> err = md_set_readonly(mddev, NULL);
> @@ -4450,10 +4455,8 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)
> err = do_md_run(mddev);
> }
> break;
> - case write_pending:
> - case active_idle:
> - case broken:
> - /* these cannot be set */
> + default:
> + err = -EINVAL;
> break;
> }
>
> --
> 2.26.2
>
