From: Li Nan <redacted>
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'.
By the way, md_ioctl() is not readable now, I wanna to re-write it later
to make it only have one 'switch' like other drivers.
Li Nan (8):
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: check mddev->pers before calling md_set_readonly()
drivers/md/md.c | 149 ++++++++++++++++++++++++++----------------------
1 file changed, 80 insertions(+), 69 deletions(-)
--
2.39.2
From: Li Nan <redacted>
There is no functional change. Just to make code cleaner.
Signed-off-by: Li Nan <redacted>
---
drivers/md/md.c | 30 ++++++++++++------------------
1 file changed, 12 insertions(+), 18 deletions(-)
From: Li Nan <redacted>
There is only one case of this 'switch'. Change it to 'if'.
Signed-off-by: Li Nan <redacted>
---
drivers/md/md.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
From: Li Nan <redacted>
'disk->private_data' is set to mddev in md_alloc() and never set to NULL,
and users need to open mddev before submitting ioctl. So mddev must not
have been freed during ioctl, and there is no need to check mddev here.
Clean up it.
Signed-off-by: Li Nan <redacted>
---
drivers/md/md.c | 5 -----
1 file changed, 5 deletions(-)
@@ -7647,11 +7647,6 @@ static int md_ioctl(struct block_device *bdev, blk_mode_t mode,mddev=bdev->bd_disk->private_data;-if(!mddev){-BUG();-gotoout;-}-/* Some actions do not requires the mutex */switch(cmd){caseGET_ARRAY_INFO:
From: Li Nan <redacted>
There is nothing to do at 'out' before setting 'did_set_md_closing'
in md_ioctl(). Return directly, and it will help us to remove
'did_set_md_closing' later.
Signed-off-by: Li Nan <redacted>
---
drivers/md/md.c | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)
From: Li Nan <redacted>
There are no functional changes, prepare to sync mddev in
array_state_store().
Signed-off-by: Li Nan <redacted>
---
drivers/md/md.c | 32 +++++++++++++++++++++-----------
1 file changed, 21 insertions(+), 11 deletions(-)
From: Li Nan <redacted>
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 <redacted>
---
drivers/md/md.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
From: Li Nan <redacted>
Commit a05b7ea03d72 ("md: avoid crash when stopping md array races
with closing other open fds.") added sync_block before stopping raid and
setting readonly. Later in commit 260fa034ef7a ("md: avoid deadlock when
dirty buffers during md_stop.") it is moved to ioctl. array_state_store()
was ignored. Add sync blockdev to array_state_store() now.
Signed-off-by: Li Nan <redacted>
---
drivers/md/md.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
From: Li Nan <redacted>
If 'mddev->pers' is NULL, there is nothing to do in md_set_readonly().
Except for md_ioctl(), the other two callers of md_set_readonly() have
already checked 'mddev->pers'. To simplify the code, move the check of
'mddev->pers' to the caller.
Signed-off-by: Li Nan <redacted>
---
drivers/md/md.c | 31 +++++++++++++++----------------
1 file changed, 15 insertions(+), 16 deletions(-)
From: Li Nan <redacted>
There is only one case of this 'switch'. Change it to 'if'.
Signed-off-by: Li Nan <redacted>
---
drivers/md/md.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
From: Li Nan <redacted>
'disk->private_data' is set to mddev in md_alloc() and never set to NULL,
and users need to open mddev before submitting ioctl. So mddev must not
have been freed during ioctl, and there is no need to check mddev here.
Clean up it.
Signed-off-by: Li Nan <redacted>
---
drivers/md/md.c | 5 -----
1 file changed, 5 deletions(-)
From: Li Nan <redacted>
There is nothing to do at 'out' before setting 'did_set_md_closing'
in md_ioctl(). Return directly, and it will help us to remove
'did_set_md_closing' later.
Signed-off-by: Li Nan <redacted>
---
drivers/md/md.c | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)
From: Li Nan <redacted>
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 <redacted>
From: Li Nan <redacted>
Commit a05b7ea03d72 ("md: avoid crash when stopping md array races
with closing other open fds.") added sync_block before stopping raid and
setting readonly. Later in commit 260fa034ef7a ("md: avoid deadlock when
dirty buffers during md_stop.") it is moved to ioctl. array_state_store()
was ignored. Add sync blockdev to array_state_store() now.
You're not just adding sync_blockdev() here. Please rewrite the tittle
and commit message.
quoted hunk
Signed-off-by: Li Nan <redacted>
---
drivers/md/md.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
@@ -4493,6 +4493,16 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len)casebroken:/* cannot be set */casebad_word:return-EINVAL;+caseclear:+casereadonly:+caseinactive:+caseread_auto:+if(!mddev->pers||!md_is_rdwr(mddev))+break;+err=mddev_set_closing_and_sync_blockdev(mddev);
In this context, mddev->openers should be zero, and such check is in
do_md_stop() and md_set_readonly():
if (atomic_read(&mddev->openers) > !!bdev).
Thanks,
Kuai
From: Li Nan <redacted>
Commit a05b7ea03d72 ("md: avoid crash when stopping md array races
with closing other open fds.") added sync_block before stopping raid and
setting readonly. Later in commit 260fa034ef7a ("md: avoid deadlock when
dirty buffers during md_stop.") it is moved to ioctl. array_state_store()
was ignored. Add sync blockdev to array_state_store() now.
You're not just adding sync_blockdev() here. Please rewrite the tittle
and commit message.
quoted
Signed-off-by: Li Nan <redacted>
---
drivers/md/md.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
*buf, size_t len)
case broken: /* cannot be set */
case bad_word:
return -EINVAL;
+ case clear:
+ case readonly:
+ case inactive:
+ case read_auto:
+ if (!mddev->pers || !md_is_rdwr(mddev))
+ break;
+ err = mddev_set_closing_and_sync_blockdev(mddev);
In this context, mddev->openers should be zero, and such check is in
do_md_stop() and md_set_readonly():
Yeah, the checks in do_md_stop() and md_set_readonly() can be removed after
this patch. However, 'mddev->open_metux' is used to protect MD_CLOSING and
'mddev->openers', it can be removed in these functions, too.
I will fix it later. Thanks for your review.
if (atomic_read(&mddev->openers) > !!bdev).
Thanks,
Kuai