On Thu, Jun 03, 2021 at 10:05:13AM -0700, Darrick J. Wong wrote: > On Thu, Jun 03, 2021 at 03:22:01PM +1000, Dave Chinner wrote: > > Hi folks, > > > > This is an update of the consolidated log scalability patchset I've been working > > on. Version 4 was posted here: > > > > https://lore.kernel.org/linux-xfs/20210519121317.585244-1-david@xxxxxxxxxxxxx/ > > > > This version contains the changes Darrick requested during review. The only > > patch remaining without at least one RVB tag is patch 30. > > > > Performance improvements are largely documented in the change logs of the > > individual patches. Headline numbers are an increase in transaction rate from > > 700k commits/s to 1.7M commits/s, and a reduction in fua/flush operations by > > 2-3 orders of magnitude on metadata heavy workloads that don't use fsync. > > > > Summary of series: > > > > Patches Modifications > > ------- ------------- > > 1-7: log write FUA/FLUSH optimisations > > 8: bug fix > > 9-11: Async CIL pushes > > 12-25: xlog_write() rework > > 26-39: CIL commit scalability > > From this latest posting, I see that the first nine patches all have > multiple reviews. Some of patches 10-19 have review tags split between > Brian and Christoph, but neither have added them all the way through. > I think I'm the only one who has supplied RVB tags for all forty. > > So my question is: at what point would you like me to pull the segments > of this patchset into upstream? "The maintainer reviewed everything" is > of course our usual standard, but this touches a /lot/ of core logging > code, and logging isn't one of my stronger areas of familiarity. > > I think 1-8 look fine for 5.14. Do you want me to wait for Brian and/or > Christoph (or really, any third pair of eyes) to finish working their > way through 9-11 and 12-25 before merging them? Quite frankly, I don't think waiting longer for more review is going to do much to improve the code further. It's largely unchanged since the last merge cycle went by when I was already waiting for reviews and it was considered "not enough time to review in this cycle". It's got enough reviews now to pass the merge bar, and the only way we are going to shake any remaining problems with this code out is to merge it and get it out to a wider testing base.... Indeed, if it is merged now it is going to sit another 3 months in for-next+rc kernels before it is released to users, so I don't think having it sit for another 3 months only in my test tree before it gets wider testing benefits anyone. All it does is slow me down and start pushing me towards having an entirely unmanageable review backlog like you already have, Darrick. Given that our rate-of-merge limitations are largely caused by a lack of review bandwidth, putting off merging code that has already met the review bar so it can have "more review" seems like a big step backwards in terms of working through our review backlog. We need to review and merge stuff faster, not block more stuff by trying to make review capture every possible problem before we merge the code. So, yeah, if I were maintainer and I saw every patch had a RVB on it, I'd be merging it straight away. But I'm not the maintainer, so I'll do whatever you want... I've fixed the little issues with the last posting, and it ran through fstests just fine last night, so I'm just about ready to send you a pull request for this series. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
