Messages in this thread | ![/](/images/icornerl.gif) | | Date | Thu, 18 Jan 2024 09:49:48 +0800 | Subject | Re: [PATCH] md: Don't clear MD_CLOSING when the raid is about to stop | From | Li Nan <> |
| |
在 2023/12/12 11:21, Yu Kuai 写道: > Hi, > > 在 2023/12/11 17:56, Mariusz Tkaczyk 写道: >> On Mon, 11 Dec 2023 16:17:14 +0800 >> linan666@huaweicloud.com wrote: >> >>> From: Li Nan <linan122@huawei.com> >>> >>> 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 >>> <linan122@huawei.com> >> >> Hello Li Nan, >> I was there when I needed to fix this: >> https://git.kernel.org/pub/scm/linux/kernel/git/song/md.git/commit/?h=md-next&id=c8870379a21fbd9ad14ca36204ccfbe9d25def43 >> >> >> For sure, you have to consider applying same solution for array_store >> "clear". >> Minor nit below. >> >> Thanks, >> Mariusz >> >>> --- >>> drivers/md/md.c | 8 +++----- >>> 1 file changed, 3 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/md/md.c b/drivers/md/md.c >>> index 4e9fe5cbeedc..ebdfc9068a60 100644 >>> --- a/drivers/md/md.c >>> +++ b/drivers/md/md.c >>> @@ -6238,7 +6238,6 @@ static void md_clean(struct mddev *mddev) >>> mddev->persistent = 0; >>> mddev->level = LEVEL_NONE; >>> mddev->clevel[0] = 0; >>> - mddev->flags = 0; >> >> I recommend (safety recommendation): >> mddev->flags = MD_CLOSING; > > Taking a look I think both MD_CLOSING and MD_DELETED should not be > cleared, however, there is no guarantee that MD_CLOSING will be set > before md_clean, because mdadm can be removed without running. Hence I > think just set MD_CLOSING is werid. > > I think the proper way is to keep MD_CLOSING and MD_DELETED if they are > set. However, there is no such api to clear other bits at once. Since > we're not expecting anyone else to write flags, following maybe > acceptable: > > mddev->flags &= BIT_ULL_MASK(MD_CLOSING) | BIT_ULL_MASK(MD_DELETED); >
MD_DELETED is only set after mddev->active is put to 0. We need to open mddev and get it before stropping raid, so the active must not be 0 and MD_DELETED will not be set in md_clean.
> Or after making sure other flags cannot race, this patch is ok. > > Thanks, > Kuai > >> >> Unless you can prove that other flags cannot race. >> >>> mddev->sb_flags = 0; >>> mddev->ro = MD_RDWR; >>> mddev->metadata_type[0] = 0; >> >> . >> > > > .
-- Thanks, Nan
| ![\](/images/icornerr.gif) |