Contributing to the Linux Kernel the Easy Way - Part 1

In this blog, Paul McKenney, a long-term Linux kernel subsystem maintainer, shares his insights on how to get a smooth kernel upstream contribution experience. If you haven’t yet read the post Joining the Linux Kernel Community, please read it to learn how to start contributing to the Linux kernel.

Although there are familiar steps involved in getting a new feature or a bugfix accepted upstream, we’re taking the time to deep dive on the process over two blogs. This ensures we can provide a comprehensive overview of key points in the process, and make it easier for readers and future Linux kernel contributors, just like you, to understand the process fully.

This blog focuses on the process of getting a bugfix accepted upstream. In the second blog, we’ll explain the process of getting a new feature accepted to the upstream kernel.

Creating the Best Bugfix: Review and Feedback

So, you found a problem that all the project members agreed constituted a critical bug, produced a fix, ensured that your fix adhered to the Linux kernel code style and algorithms, and did appropriate testing. Congratulations!

If you’re lucky or the fix is small, your patch may be accepted directly without any feedback. However, this is unusual, and you should always be prepared to update your patch based on review. This could be for a number of reasons, including:

• The bug had additional, non-obvious implications that your fix has not addressed.
• Your fix introduces a bug that your testing missed, so your fix itself needs to be fixed.
• Another participant is working on a feature that conflicts with your fix.
• Your fix is against an old version of the source and needs to be forward-ported to the current version in order to be accepted.

It is completely normal to receive feedback on your proposed changes, and the review process is what improves the kernel in the long run. It can also help you as a contributor and engineer, too. Having a series of bug fixes accepted is an excellent way to get some practice in, and build trust with Linux kernel community members (which in turn will ease your creation of better contributions in the future).

The Importance of Forward-Porting

The point about forward-porting in the feedback examples above bears repeating. Old versions of the Linux kernel will continue to be used in production, so fixes need to be applied to more than just the mainline version. For example, as of December 2022, v6.1 was released, but most of the fleet was still running v5.12, with only a tiny fraction using v5.19. It is tempting just to fix the bug in one version and move onto something else, but that way lies madness.

First, there is a good chance that the bug has already been fixed. It is faster and easier to try to reproduce the bug in mainline, and if it does not reproduce, find the fix, and backport it – or, better yet, take an already backported fix from one of the -stable trees. In this case, not only has someone else tested the fix, but in the all-too-likely case where the fix is less straightforward than it first appears, they might well also have handled the non-obvious aspects of the fix. This will allow you to get on with life much more quickly and with fewer unintended consequences.

Second, if you only ever fix the old version, these fixes will accumulate into a surprisingly large pile. This pile of fixes will make it increasingly difficult to move to the next version, and will cost you a lot of time and effort.

Third, if the bug has not yet been fixed in mainline, the testing and review process required to get your fix into mainline will help you locate the non-obvious aspects of the bug and the required fix. Plus, as aforementioned, this process will help the community become more familiar with you, making your next interaction with the community much easier. What was previously a problem can be made into an opportunity.

But what if this is an emergency? In that case, the first thing you should do is try the simplest thing that mitigates the problem. This might mean changing the configuration, disabling a feature, or providing a work-around for the bug in an application. Once the bug is mitigated, get the real fix into the mainline first, as discussed above. Only when no mitigation is possible should you first fix the bug in the old version.

If, for some reason, you absolutely must fix a bug in an old version, do not simply post the fix in the old version and expect busy maintainers to do anything with it. Instead, forward-port it to the current mainline and then post it. Follow the review and testing process to get your fix into the mainline. That way, it will only need to be part of the patch pile for a very few releases, which will in turn keep that pile manageably small.

Optimizing Communication in Your Fix

Although communication feels like it should be easy, it can often be tricky, especially online. It can be tempting to simply proclaim a change in your fix, but there is a great deal more to “good” communication (for both you and the people you are speaking to) than simply ordering the changes. While you believe your changes to be necessary and worthwhile, others may need more context to share your point of view.

When explaining your fix, consider things like:

• Does your target audience share your goals? If your proposal adds complexity to provide some feature or fix that they do not believe is important, they are unlikely to welcome your suggested change.
• Does your target audience share your understanding of the words and concepts making up your proposal? If you doubt this, suppose that you make an authoritative statement, “No problem, just use RCU there!” - If the target audience didn’t know about or didn’t trust RCU, their reaction would likely be: “Not in my code!!!”
• How much of your statement is telling your target audience things that they already know? If too much of your statement is already obvious to them, they might stop reading long before they reach the important parts of your proclamation.
• Does your target audience share your view of the universe? Mismatched goals were one cause of the 2010 Android flame wars. The Android developers could not understand why acquiring a global lock on each I/O could be a problem, while the Linux-kernel community could not understand that the Android developers might have a good grasp of optimizing battery lifetime. But eventually, a compromise was reached.

The most effective communication builds a bridge from where your target audience is to where you need them to go and gets them to trust you enough to walk that bridge. This means that communicating with a wider and more diverse audience can sometimes be more challenging. But, then again, what is life without a challenge? Practice makes perfect.

Receiving “Open and Honest” Feedback

Feedback you receive for your patches can come in different tones. It can sometimes be helpful and polite, and sometimes not. Regardless of how the feedback looks, first and foremost, you must interpret it and respond to it as feedback on your technical work and not as a personal criticism.

Everyone, no matter their seniority or experience, should avoid giving impolite feedback. So why does it happen sometimes? Well, try to keep in mind that reviewers and maintainers need to:

• Get everything right regarding ongoing kernel operations.
• Avoid breaking user space.
• Avoid unnecessary complexity.
• Keep the code maintainable.
• Get their jobs done as efficiently as possible, which may limit the amount of hand-holding they can do.

The best advice for responding to feedback is to “react thoughtfully, rather than just react.” This can be harder than it sounds sometimes, but it is important to assume best intent from your reviewers. Feedback that appears curt at first glance might simply be that the reviewer was short on time/sleep, or didn’t have enough capacity to think in detail about your patch at present.

Some helpful tips:

• Think carefully about what context that project member might be missing and try to provide it (ideally, before you ask for feedback!). Many disagreements happen because people have made mutually contradictory unspoken assumptions. Therefore:
  • Check your assumptions. Have you stated those assumptions clearly? Are those assumptions correct?
  • Remember the wise words of George Bernard Shaw: "The single biggest problem in communication is the illusion that it has taken place."
• Keep in mind that in the Linux kernel, “getting it right” often means keeping track of a surprisingly large set of requirements. And even when you have your head around a large set of requirements, be sure that you have not missed some. So, take some time to consider whether the brusque feedback might be due to you having missed an obvious-to-them requirement.
• Sleep on it before replying. For whatever reason, lack of sleep makes us more likely to interpret honest feedback as negative. Very often, things look better the next morning.

When you’ve processed the feedback, respond calmly and professionally, focusing solely on the technical points raised in a feedback email. Remember that an ill-considered response to impolite feedback in an open email list is archived forever.

Responding to Feedback

If a requested change is trivial or obviously correct, just make the change and resubmit the patch. Be sure that your resubmitted patch makes it very clear that you have made the requested change.

Sometimes, you might not agree with the requested changes, or be unclear on what exactly needs changing.

If you don’t completely understand what the reviewer was asking for, ask for more information or explain in your own words the portion of the feedback that you understand, along with what you intend to do in response to that feedback. This approach will often save a round of review.

If you believe the reviewer was wrong, explain clearly why you think it needs to be done the way you initially suggested. Better yet, just try the approach suggested by the reviewer, especially in the case where the reviewer is also the maintainer. Who knows - you might learn something! And in the worst case, you will be able to make an apples-to-apples comparison between your approach and that of the reviewer, which just might help convince people that your way is best.

If the reviewer suggests an inappropriate change, think carefully about the suggestion. The suggestion, though incorrect, will often be motivated by a specific concern that the reviewer has with your patch. You can then suggest a different solution that is appropriate to the problem at hand, and that also addresses the reviewer’s concern. (Or you just might learn how the reviewer's inappropriate change really is appropriate after all.)

Lastly, if a lot of the comments focus on an unimportant part of your patch, offer to remove or postpone that part of your patch in order to speed up acceptance of the important parts. In fact, it is much better to provide a series of small patches than one big patch.

Question Rather Than Contradict

This last section focuses on a general concept that is useful in all types of feedback - not just Linux kernel contributions. Instead of asserting or contradicting, create opportunities for learning by asking questions.

When someone asserts that your code does not work, the instinct for most is to contradict them. But contradicting will likely push them farther into an oppositional mode, making it less likely that your code will be accepted. Or, at the very least, delaying its eventual acceptance.

It is usually much more productive to ask questions. For example, instead of asserting “Hey, my code does work because it is holding foo_lock!”, you ask “Given that the code is holding foo_lock, how can that bug happen?” Yes, there is still some chance that you will receive some pushback (perhaps being told to read the code), but you will at least increase the odds that the reviewer will answer the question. The answer might pinpoint your bug, which makes it easier for you to fix it and post an updated version of your code. Or the reviewer might withdraw the objection.

Asking questions can also be helpful when you are the reviewer. For example, instead of baldly asserting "This code is broken because it does not hold foo_lock!!!", say something like "It looks to me like this code is not holding foo_lock. If that is the case, how does this code safely handle the following sequence of events?", then listing that sequence of events. And then if the code really does have some subtle way of handling that sequence of events, you can always ask the contributor to document it. Or you could come up with a more straightforward approach. Just be very sure that it meets all the requirements! Also, make sure your approach is simpler and more straightforward to others.

Your life is usually easier if you can get your code in while ruffling as few feathers as possible - and asking questions usually beats assertions.

Stay Tuned…

In the next blog, we’ll discuss the process of getting a new feature accepted to the upstream kernel.

Other Material

The following files in the Linux source tree should be read before submitting anything to the Linux kernel community:

Documentation/process/submit-checklist.rst
Documentation/process/submitting-drivers.rst
Documentation/process/submitting-patches.rst

To learn more about Meta Open Source, visit our open source site, subscribe to our YouTube channel, or follow us on Twitter, Facebook and LinkedIn.