There are a number of things to keep in mind when merging code into the upstream kernel. The main thing to remember is that no matter how busy you are, Andrew and Linus are receiving all patches from everybody, all the time, meaning they are a lot busier than you.
This strategy for merging code upstream makes things easier for them, meaning your code can go into the upstream kernel faster.
Contents
Bite-size pieces
It is tempting to work on your project for months, "until it is perfect", and then send one huge patch submission to the linux-kernel mailing list. However, this is a bad idea for a number of reasons.
- A large project is effectively a branch of the Linux kernel. Maintaining a branch takes a lot of effort. Effort which could have been better spent doing actual development.
- It takes a lot of time to review a large patch. Most patch reviewers will not have that much time, and will simply skip your patch.
- Linus and Andrew are busier than most patch reviewers, and will often not bother reviewing patches that nobody else has shown interest in. In this context, "interest" often means the reviews from the bullet point above.
- The larger your submitted patch is, the higher the chance that it contains bugs and will be rejected.
- Even if you test your code very well, chances are it will still break for somebody else's strange configuration.
Patch reviewers and tree maintainers will not like you if you regularly burden them with hard to review submissions. So-called hit and run patch bombs can damage your reputation.
Luckily most of the above problems can be solved by submitting your code in small pieces. This is especially easy if your changes are independent of each other and could be merged separately.
- Aim for small, individually functional patches. Ideally a reviewer should be able to thoroughly review your patch in 5 to 10 minutes.
- If your work is broken up in 8 small, easily verifyiable patches, chances are that the majority of them will be correct (you can verify them easily) and will get accepted upstream.
- The ones that are not correct are small, and can often be fixed quite easily.
- By getting some of your patches accepted upstream quickly, you can focus your attention on development, not maintaining a large branch of the kernel.
- Patch reviewers and tree maintainers will like you for making their life easier. This helps build your reputation in the community and should make it easier to get code accepted in the future.
The way to split up patches is by function, not necessarily by file. Each patch should contain one easily verifiable functional change, even if you end up touching the same files in multiple patches.
An example
Say that you are hacking the bar driver, to add support for foo devices. However, the straightforward way to add the functionality would turn the code into a big mess. The clean solution requires that the existing code be reorganized, after that your functionality will just slide in. Your patch sequence could look something like this:
Subject: [PATCH 0/3] add foo device support to the bar driver Subject: [PATCH 1/3] foo support: add device type abstraction to bar_struct Subject: [PATCH 2/3] foo support: bar driver code cleanups Subject: [PATCH 3/3] foo support: add foo device support
Traditionally the first email does not contain any patches, but simply explains what your patches do and why they are needed. The other emails are sent as replies to the first email, so people who are not interested in your patch can just collapse the thread in their mail reader.
In this example, the first two patches would not contain the actual functionality you want to add to the kernel. All they do is refactor the code base so it becomes easier to add the interesting code to the kernel. Patch reviewers and tree maintainers like it when people leave the source code in an easier to maintain state than they found it. Piling hack on top of hack would lead to an unmaintainable code base over time, so the "easy way" to add functionality is often frowned upon.
Hints on mailing patches
- Each patch should contain a logical change. If your function rename patch touches 20 files, that's fine. No need to split it up any further.
- Splitting up your work is hard. It is much easier to keep your changes separate throughout development.
- The patches, which are in reply to the initial mail, should have a In-Reply-To: and/or References: header, which does indeed reference the initial email.
- Send your patches in plain text, inline in the email. This makes it easy for reviewers to reply to sections of code.
Test your mail. First send the patch series to yourself, and make sure the emailed patches still apply cleanly to a kernel tree. Sometimes mail clients mangle patches, so be careful.
- The quilt patch management system has a built-in mail sending script that does the right thing for sending patch series. Quilt also helps you manage a series of patches during development. Consider using something like quilt.
Some mail clients are really dumb. Consider using something like quilt or git send-email. See https://elixir.bootlin.com/linux/latest/source/Documentation/process/email-clients.rst for email client help.
Acknowledge feedback
Now that you have the habit of submitting your code in small patches, reviewers will be able to quickly give you feedback on your patches. Sometimes within an hour. Now that you have their attention, it would be nice if you could act on their feedback before they move on to other code.
You may not be able to implement all feedback or test the changed code at the same speed feedback comes in. This is fine.
However, you should probably let the reviewers know that you read their feedback and are making some of the changes they suggested. Better still, let them know before their attention has moved on to other matters.
The benefit of the doubt
Often the patch reviewers point out obvious bugs and improvements. The kinds of changes you would have made yourself, had you discovered the flaws. Of course you will act on this feedback.
However, sometimes it is not clear whether it is better to stick to your original code, or implement the alternative suggested by a reviewer. In this case it is best to change your code to what the reviewer suggested.
This may seem counter-intuitive. After all, changing the code is extra work and changes could introduce new bugs.
However, by implementing the idea contributed by the patch reviewer you are indicating that you can work with the community. More importantly, the patch reviewer is likely to support his own idea and you will have gathered a supporter for inclusion of your patch. By being accomodating to community suggestions, your code will gain acceptance easier.
At times the reviewers are plain wrong, though. In that case the best thing you can do is keep your own code and explain why things need to be the way you made them.
Release early, release often
Software projects can take a long time and have many phases, including requirement gathering, design, prototyping, implementation and testing. It is tempting to work on a project in-house and only show it to the Linux kernel community when it is ready.
However, the Linux kernel community is a lot larger than your own organization, and is likely to have different requirements. After a year long project, you do not want to go back to the drawing board because you did not know about somebody else's requirements.
A better approach is to go to the Linux kernel community with your initial design and prototype. Not only does this allow you to get feedback and additional requirements at an early stage, it often brings you into contact with other people interested in solving the same problem. You could end up working together with developers from other organizations, and work on an implementation that is much better than anything you could have done yourself.
The kind of solution that solves a bunch of problems at once.
The kind that Linus and Andrew like.
The kind that actually gets merged upstream, solving your problem for good.
Listen to the man
Some of the most prolific patch reviewers on the linux-kernel mailing list have gotten a reputation of being hard to work with. This is probably because they spend their time on reviewing code and not on being diplomatic. Sometimes the feedback that patches receive is downright harsh. However, that does not make the feedback any less valid!
The reason people review your patches and send you feedback is because they believe you are capable of acting on the feedback and improving your code. If you are serious about improving your code, listen to the feedback you receive on the mailing list.
Even if the feedback is harsh or hostile, it could contain useful information. If you care about your code, grab your oven gloves and pull the nuggets of wisdom from the flames.
If you do not understand part of the feedback you got, ask for clarification. The end goal is to improve your code. Learning something new in the process is a bonus.
Reward good behaviour by thanking people when they send you friendly feedback. Don't forget to tell them how you plan to act on their feedback.
RTFM
Check the UpstreamMerge/SubmitChecklist and other kernel documentation to ensure that you submit your patches the right way. Not only is it easy to get these little details right, it is also necessary.
Back to UpstreamMerge.