[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPhsuW7TRODsR_N95AmXJCZvpTuSKgbOjnYGxMGAWtmt3x9Vkw@mail.gmail.com>
Date: Wed, 4 Oct 2023 20:55:23 -0700
From: Song Liu <song@...nel.org>
To: Yu Kuai <yukuai1@...weicloud.com>
Cc: xni@...hat.com, agk@...hat.com, snitzer@...nel.org,
dm-devel@...hat.com, linux-kernel@...r.kernel.org,
linux-raid@...r.kernel.org, yi.zhang@...wei.com,
yangerkun@...wei.com, "yukuai (C)" <yukuai3@...wei.com>
Subject: Re: [PATCH -next v3 00/25] md: synchronize io with array reconfiguration
On Wed, Oct 4, 2023 at 8:42 PM Yu Kuai <yukuai1@...weicloud.com> 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@suse.cz/
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
Powered by blists - more mailing lists