Re: [PATCH -next v3 00/25] md: synchronize io with array reconfiguration

[Song Liu]

On Wed, Oct 4, 2023 at 8:42 PM Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> wrote:
>
> Hi,
>
> 在 2023/09/29 3:15, Song Liu 写道:
> > Hi Kuai,
> >
> > Thanks for the patchset!
> >
> > A few high level questions/suggestions:
>
> Thanks a lot for these!
> >
> > 1. This is a big change that needs a lot of explanation. While you managed to
> > keep each patch relatively small (great job btw), it is not very clear why we
> > need these changes. Specifically, we are adding a new mutex, it is worth
> > mentioning why we cannot achieve the same goal without it. Please add
> > more information in the cover letter. We will put part of the cover letter in
> > the merge commit.
>
> Yeah, I realize that I explain too little. I will add background and
> design.
> >
> > 2. In the cover letter, please also highlight that we are removing
> >   MD_ALLOW_SB_UPDATE and MD_UPDATING_SB. This is a big improvement.
> >
>
> Okay.
> > 3. Please rearrange the patch set so that the two "READ_ONCE/WRITE_ONCE"
> > patches are at the beginning.
>
> Okay.
> >
> > 4. Please consider merging some patches. Current "add-api => use-api =>
> > remove-old-api" makes it tricky to follow what is being changed. For this set,
> > I found the diff of the whole set easier to follow than some of the big patches.
> I refer to some other big patchset to replace an old api, for example:
>
> https://lore.kernel.org/all/20230818123232.2269-1-jack@xxxxxxx/

Yes, this is a safe way to replace old APIs. Since the scale of this
patchset is
smaller, I was thinking it might not be necessary to go that path. But
I will let
you make the decision.

> Currently I prefer to use one patch for each function point. And I do
> merged some patches in this version, and for remaining patches, do you
> prefer to use one patch for one file instead of one function point?(For
> example, merge patch 10-12 for md/raid5-cache, and 13-16 for md/raid5).

I think 10 should be a separate patch, and we can merge 11 and 12. We can
merge 13-16, and maybe also 5-7 and 18-20.

Thanks,
Song