* [PATCH v2] md: do not require mddev_lock() for all options
@ 2023-09-28 12:55 Mariusz Tkaczyk
2023-09-28 20:44 ` Song Liu
0 siblings, 1 reply; 2+ messages in thread
From: Mariusz Tkaczyk @ 2023-09-28 12:55 UTC (permalink / raw)
To: Song Liu; +Cc: linux-raid, Mariusz Tkaczyk, Yu Kuai
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@huawei.com>
Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
---
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2] md: do not require mddev_lock() for all options
2023-09-28 12:55 [PATCH v2] md: do not require mddev_lock() for all options Mariusz Tkaczyk
@ 2023-09-28 20:44 ` Song Liu
0 siblings, 0 replies; 2+ messages in thread
From: Song Liu @ 2023-09-28 20:44 UTC (permalink / raw)
To: Mariusz Tkaczyk; +Cc: linux-raid, Yu Kuai
On Thu, Sep 28, 2023 at 5:55 AM Mariusz Tkaczyk
<mariusz.tkaczyk@linux.intel.com> 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@huawei.com>
> Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
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
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-09-28 20:44 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-28 12:55 [PATCH v2] md: do not require mddev_lock() for all options Mariusz Tkaczyk
2023-09-28 20:44 ` Song Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).