| Message ID | 20240201063404.772797-8-linan666@huaweicloud.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | bugfix of MD_CLOSING and clean up md_ioctl() | expand |
Hi, 在 2024/02/01 14:34, linan666@huaweicloud.com 写道: > From: Li Nan <linan122@huawei.com> > > 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. > > Signed-off-by: Li Nan <linan122@huawei.com> > --- > drivers/md/md.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/md/md.c b/drivers/md/md.c > index 4c7a0225f77d..86becf0015f5 100644 > --- a/drivers/md/md.c > +++ b/drivers/md/md.c > @@ -4493,6 +4493,16 @@ array_state_store(struct mddev *mddev, const char *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(): if (atomic_read(&mddev->openers) > !!bdev). Thanks, Kuai > + if (err) > + return err; > + break; > default: > break; > } > @@ -4518,6 +4528,7 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len) > spin_unlock(&mddev->lock); > return err ?: len; > } > + > err = mddev_lock(mddev); > if (err) > return err; > @@ -4592,6 +4603,11 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len) > sysfs_notify_dirent_safe(mddev->sysfs_state); > } > mddev_unlock(mddev); > + > + if (st == readonly || st == read_auto || st == inactive || > + (err && st == clear)) > + clear_bit(MD_CLOSING, &mddev->flags); > + > return err ?: len; > } > static struct md_sysfs_entry md_array_state = >
在 2024/2/2 10:12, Yu Kuai 写道: > Hi, > > 在 2024/02/01 14:34, linan666@huaweicloud.com 写道: >> From: Li Nan <linan122@huawei.com> >> >> 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. > >> >> Signed-off-by: Li Nan <linan122@huawei.com> >> --- >> drivers/md/md.c | 16 ++++++++++++++++ >> 1 file changed, 16 insertions(+) >> >> diff --git a/drivers/md/md.c b/drivers/md/md.c >> index 4c7a0225f77d..86becf0015f5 100644 >> --- a/drivers/md/md.c >> +++ b/drivers/md/md.c >> @@ -4493,6 +4493,16 @@ array_state_store(struct mddev *mddev, const char >> *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 >
diff --git a/drivers/md/md.c b/drivers/md/md.c index 4c7a0225f77d..86becf0015f5 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -4493,6 +4493,16 @@ array_state_store(struct mddev *mddev, const char *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); + if (err) + return err; + break; default: break; } @@ -4518,6 +4528,7 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len) spin_unlock(&mddev->lock); return err ?: len; } + err = mddev_lock(mddev); if (err) return err; @@ -4592,6 +4603,11 @@ array_state_store(struct mddev *mddev, const char *buf, size_t len) sysfs_notify_dirent_safe(mddev->sysfs_state); } mddev_unlock(mddev); + + if (st == readonly || st == read_auto || st == inactive || + (err && st == clear)) + clear_bit(MD_CLOSING, &mddev->flags); + return err ?: len; } static struct md_sysfs_entry md_array_state =