2024-02-26 03:22:15

by Li Nan

[permalink] [raw]
Subject: [PATCH v7 0/9] bugfix of MD_CLOSING and clean up md_ioctl()

From: Li Nan <[email protected]>

Changes in v7:
- adapt to md-6.9 branch.

Changes in v6:
- in patch 2, return directly.
- in patch 4, return directly in case GET_DISK_INFO and GET_ARRAY_INFO.
- in patch 7, rewrite commit message.
- add patch 8, clean up openers check.

Changes in v5:
- add patches 1-4 to clean up md_ioct(), pathc 4 can help us clean up
local variable 'clear_md_closing'.
- in patches 5 and 7, clean up local variable 'clear_md_closing'.


Li Nan (9):
md: merge the check of capabilities into md_ioctl_valid()
md: changed the switch of RAID_VERSION to if
md: clean up invalid BUG_ON in md_ioctl
md: return directly before setting did_set_md_closing
md: Don't clear MD_CLOSING when the raid is about to stop
md: factor out a helper to sync mddev
md: sync blockdev before stopping raid or setting readonly
md: clean up openers check in do_md_stop() and md_set_readonly()
md: check mddev->pers before calling md_set_readonly()

drivers/md/md.c | 183 ++++++++++++++++++++++++------------------------
1 file changed, 91 insertions(+), 92 deletions(-)

--
2.39.2



2024-02-26 03:22:16

by Li Nan

[permalink] [raw]
Subject: [PATCH v7 1/9] md: merge the check of capabilities into md_ioctl_valid()

From: Li Nan <[email protected]>

There is no functional change. Just to make code cleaner.

Signed-off-by: Li Nan <[email protected]>
Reviewed-by: Yu Kuai <[email protected]>
---
drivers/md/md.c | 30 ++++++++++++------------------
1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index e2a5f513dbb7..332eda680aea 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -7521,16 +7521,17 @@ static int md_getgeo(struct block_device *bdev, struct hd_geometry *geo)
return 0;
}

-static inline bool md_ioctl_valid(unsigned int cmd)
+static inline int md_ioctl_valid(unsigned int cmd)
{
switch (cmd) {
- case ADD_NEW_DISK:
case GET_ARRAY_INFO:
- case GET_BITMAP_FILE:
case GET_DISK_INFO:
+ case RAID_VERSION:
+ return 0;
+ case ADD_NEW_DISK:
+ case GET_BITMAP_FILE:
case HOT_ADD_DISK:
case HOT_REMOVE_DISK:
- case RAID_VERSION:
case RESTART_ARRAY_RW:
case RUN_ARRAY:
case SET_ARRAY_INFO:
@@ -7539,9 +7540,11 @@ static inline bool md_ioctl_valid(unsigned int cmd)
case STOP_ARRAY:
case STOP_ARRAY_RO:
case CLUSTERED_DISK_NACK:
- return true;
+ if (!capable(CAP_SYS_ADMIN))
+ return -EACCES;
+ return 0;
default:
- return false;
+ return -ENOTTY;
}
}

@@ -7601,18 +7604,9 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
struct mddev *mddev = NULL;
bool did_set_md_closing = false;

- if (!md_ioctl_valid(cmd))
- return -ENOTTY;
-
- switch (cmd) {
- case RAID_VERSION:
- case GET_ARRAY_INFO:
- case GET_DISK_INFO:
- break;
- default:
- if (!capable(CAP_SYS_ADMIN))
- return -EACCES;
- }
+ err = md_ioctl_valid(cmd);
+ if (err)
+ return err;

/*
* Commands dealing with the RAID driver but not any
--
2.39.2


2024-02-26 03:22:47

by Li Nan

[permalink] [raw]
Subject: [PATCH v7 5/9] md: Don't clear MD_CLOSING when the raid is about to stop

From: Li Nan <[email protected]>

The raid should not be opened anymore when it is about to be stopped.
However, other processes can open it again if the flag MD_CLOSING is
cleared before exiting. From now on, this flag will not be cleared when
the raid will be stopped.

Fixes: 065e519e71b2 ("md: MD_CLOSING needs to be cleared after called md_set_readonly or do_md_stop")
Signed-off-by: Li Nan <[email protected]>
Reviewed-by: Yu Kuai <[email protected]>
---
drivers/md/md.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 08b0f80274c0..43ca8a4c02ce 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -6264,7 +6264,15 @@ static void md_clean(struct mddev *mddev)
mddev->persistent = 0;
mddev->level = LEVEL_NONE;
mddev->clevel[0] = 0;
- mddev->flags = 0;
+ /*
+ * Don't clear MD_CLOSING, or mddev can be opened again.
+ * 'hold_active != 0' means mddev is still in the creation
+ * process and will be used later.
+ */
+ if (mddev->hold_active)
+ mddev->flags = 0;
+ else
+ mddev->flags &= BIT_ULL_MASK(MD_CLOSING);
mddev->sb_flags = 0;
mddev->ro = MD_RDWR;
mddev->metadata_type[0] = 0;
@@ -7602,7 +7610,6 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
int err = 0;
void __user *argp = (void __user *)arg;
struct mddev *mddev = NULL;
- bool did_set_md_closing = false;

err = md_ioctl_valid(cmd);
if (err)
@@ -7659,7 +7666,6 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
mutex_unlock(&mddev->open_mutex);
return -EBUSY;
}
- did_set_md_closing = true;
mutex_unlock(&mddev->open_mutex);
sync_blockdev(bdev);
}
@@ -7801,7 +7807,7 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
mddev_unlock(mddev);

out:
- if(did_set_md_closing)
+ if (cmd == STOP_ARRAY_RO || (err && cmd == STOP_ARRAY))
clear_bit(MD_CLOSING, &mddev->flags);
return err;
}
--
2.39.2


2024-02-26 03:22:58

by Li Nan

[permalink] [raw]
Subject: [PATCH v7 6/9] md: factor out a helper to sync mddev

From: Li Nan <[email protected]>

There are no functional changes, prepare to sync mddev in
array_state_store().

Signed-off-by: Li Nan <[email protected]>
---
drivers/md/md.c | 32 +++++++++++++++++++++-----------
1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 43ca8a4c02ce..cba1027b279e 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -529,6 +529,24 @@ void mddev_resume(struct mddev *mddev)
}
EXPORT_SYMBOL_GPL(mddev_resume);

+/* sync bdev before setting device to readonly or stopping raid*/
+static int mddev_set_closing_and_sync_blockdev(struct mddev *mddev, int opener_num)
+{
+ mutex_lock(&mddev->open_mutex);
+ if (mddev->pers && atomic_read(&mddev->openers) > opener_num) {
+ mutex_unlock(&mddev->open_mutex);
+ return -EBUSY;
+ }
+ if (test_and_set_bit(MD_CLOSING, &mddev->flags)) {
+ mutex_unlock(&mddev->open_mutex);
+ return -EBUSY;
+ }
+ mutex_unlock(&mddev->open_mutex);
+
+ sync_blockdev(mddev->gendisk->part0);
+ return 0;
+}
+
/*
* Generic flush handling for md
*/
@@ -7657,17 +7675,9 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,
/* Need to flush page cache, and ensure no-one else opens
* and writes
*/
- mutex_lock(&mddev->open_mutex);
- if (mddev->pers && atomic_read(&mddev->openers) > 1) {
- mutex_unlock(&mddev->open_mutex);
- return -EBUSY;
- }
- if (test_and_set_bit(MD_CLOSING, &mddev->flags)) {
- mutex_unlock(&mddev->open_mutex);
- return -EBUSY;
- }
- mutex_unlock(&mddev->open_mutex);
- sync_blockdev(bdev);
+ err = mddev_set_closing_and_sync_blockdev(mddev, 1);
+ if (err)
+ return err;
}

if (!md_is_rdwr(mddev))
--
2.39.2


2024-02-26 21:38:43

by Song Liu

[permalink] [raw]
Subject: Re: [PATCH v7 0/9] bugfix of MD_CLOSING and clean up md_ioctl()

On Sun, Feb 25, 2024 at 7:20 PM <[email protected]> wrote:
>
> From: Li Nan <[email protected]>
>
> Changes in v7:
> - adapt to md-6.9 branch.

LGTM. Applied to md-6.9 branch. Thanks!

Song