Doug, > Cutting a patchset that touches around 1500 lines of a 3000 line > driver, then adds new functionality amounting to an extra 3000 lines > of code (and comments), according to the "one change per patch" rule > would result in a patchset with hundreds of patches. The problem here is that you think of it as a single patch set transitioning the driver from major version X to version Y. Linux kernel development has moved away from that model. The kernel release cadence is more or less fixed at 10 weeks. The release process is not controlled by features or component versions or anything of that nature. It is set by passing of time. The notion that a driver or kernel component may have a version number applied to it is orthogonal to that process. A version is something you as a driver maintainer may decide to use to describe a certain set of commits. But it has no meaning wrt. the Linux development process. If you want to transition sg from what you call v3 to v4, then the process is that you submit a handful or two of small, easily digestible patches at a time. It may take a few kernel releases to get to where you want to be. But that is the process that everybody else is following to get their changes merged. Nobody says you can only make one submission per submission window. It's perfectly fine to submit patches 11-20 as soon as patches 1-10 have been reviewed and merged. As an example of this, the HBA vendors usually send several driver updates each release cycle. > Further if they are to be bisectable then they must not only > compile and build, but run properly. Absolutely. That's a requirement. > Of course that is impossible for new functionality as there is little > to test the new functionality against. The burden is on you to submit patches in an order in which they make logical sense and in which they can be reviewed, bisected, and tested. > I looked at the "Device Drivers" section. Most patchsets there > had between 10 and 20 patches, one had 33. The "number of patches in a driver submission" as a measure is a red herring. sg is not a new driver, it has users. > One, the SIW Infiniband driver, is over 10,000 lines long, and > contains 'only' 12 patches. But it was presumably developed out of tree in a separate repo. And the thousands of commits that resulted in this driver have been collapsed into 12 patches for initial submission. The number of lines is also a red herring. If a patch changes 10,000 lines to make one logical change that's perfectly fine. What's not fine is a patch changing 10,000 lines and also making an entirely different logical change. > Reviewers are obviously a scarce resource, but making their live's > easier shouldn't be a goal in itself. Couldn't disagree more. If you want your code merged, you will have to present it in a way that caters to the reviewers. Because without reviews, your code won't get merged. > If new functionality is being proposed, surely it is better to check > that it is documented and that test code exists. Then the design and > high level details of the implementation should be assessed. Testing and documentation are absolutely important. But so is documenting what compelled a code change. Reviews aid in verifying that the thought process outlined in the patch description matches the code changes performed. This is where the whole "one logical change" comes from. And when things subsequently break, it is then easy to identify which assumptions the patch author made that turned out not to be valid. > To date I have had no feedback about design document describing this > patchset: http://sg.danny.cz/sg/sg_v40.html This suffers the same problem as your patch series in that it encapsulates 26 years of thought in a single blob. Presumably that document developed over time and didn't go from nothing to 22K words in an instant. You need to document that process. Also, having a design document that describes a wealth of changes after the fact is not terribly helpful. I understand appreciate that you are focused on the end product. But to get there you need to slowly and iteratively submit patches against v3 that can be independently reviewed and merged. Please pick one feature at a time. Carve that into a few patches that each logically only do one thing. And then submit that as a patch set with an intro mail that describes the design of the feature, which assumptions are made, what the benefits are, who needs this capability, etc. -- Martin K. Petersen Oracle Linux Engineering
