On Wed, Sep 25, 2019 at 09:53:04AM -0400, Josef Bacik wrote: > On Wed, Sep 25, 2019 at 03:47:47PM +0200, David Sterba wrote: > > On Tue, Sep 24, 2019 at 04:32:48PM -0400, Josef Bacik wrote: > > > Hopefully all of it makes it this time, if you want you can pull from > > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/josef/btrfs-next.git \ > > > extent-io-rearranging > > > > The size of the exported patch 1/4 is 109066 bytes and the diff itself > > is incomprehensible to even see what code moves where and what is new. > > > > I'm still thinking if this is a good idea to apply a monster patch, even > > it's just moving code around. The previous series splitting > > extent-tree.c were better so I'd rather take that approach again. Some > > of the functions belong logically together and won't break compilation > > and would actually make it possible for a human to review. > > extent-tree.c was way different because a bunch of things had to be renamed and > exported. Also extent-tree.c got split into many more files, so there was less > bulk being moved around. extent_io.c is different because basically everything > is exported already, so it's really just move definitions, move code. I > literally just split vim'ed, cut and paste between the two files. > > Things like this are going to be impossible to review. It's a one time pain for > more readability and better decisions down the road. I did my best to keep the > logical changes to their own patch, but the fact is we have _a lot_ of code for > each of these different logical things. I can turn it into 45 patches moving > one function at a time, but who's going to go and check each individual patch? I'm not talking about 45 patches but somethig like 2-4. One of the patches in extent-tree split has 65K, and I basically redone it (and several others) by hand due to small changes in the original code. This counts as a review, even though the patch was large, but with the difference that I was able to do that in one go per patch. The 100K scares me from the beginning, thus my question. If you say that 'this is going to be impossible to review' then, well we can't merge it in that form. And from past experience, monster patches are never one time pain and come up as a reminder of low patch standards. The bad thing is that it's never the patch author who has to suffer. The idea for split is to do eg. the following groups: - extent leak detection functions - state bit manipulation functions (set_extent_bit etc) - other io tree functions I'm looking for an achievable long-term strategy to do those large code movements and find the balance between writing the patches and reviewing them.
