On Tue, 12 Dec 2023 11:21:28 +0800
Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> wrote:
> Hi,
>
> 在 2023/12/11 17:56, Mariusz Tkaczyk 写道:
> > On Mon, 11 Dec 2023 16:17:14 +0800
> > linan666@xxxxxxxxxxxxxxx wrote:
> >
> >> From: Li Nan <linan122@xxxxxxxxxx>
> >>
> >> 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@xxxxxxxxxx>
> >
> > 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);
Yes, MD_CLOSING is a bit number to not a bit value I can assign directly.
Thanks for clarifying!
Mariusz
>
> 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;
> >
> > .
> >
>
