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
