Philosophy of Linux kernel patches
What is a patch?
A patch is a small file that contains a short commit message (less than 50 characters), a description of the changes in paragraph form, and a diff of the code changes. Here's an example of a patch to a Linux kernel documentation file:
From 2c97a63f6fec91db91241981808d099ec60a4688 Mon Sep 17 00:00:00 2001 From: Sarah Sharp <sarah.a.sharp@linux.intel.com> To: linux-doc@vger.kernel.org Date: Sat, 13 Apr 2013 18:40:55 -0700 Subject: [PATCH] Docs: Add info on supported kernels to REPORTING-BUGS. One of the most common frustrations maintainers have with bug reporters is the email that starts with "I have a two year old kernel from an embedded vendor with some random drivers and fixes thrown in, and it's crashing". Be specific about what kernel versions the upstream maintainers will fix bugs in, and direct bug reporters to their Linux distribution or embedded vendor if the bug is in an unsupported kernel. Suggest that bug reporters should reproduce their bugs on the latest -rc kernel. Signed-off-by: Sarah Sharp <sarah.a.sharp@linux.intel.com> --- REPORTING-BUGS | 22 ++++++++++++++++++++++ 1 files changed, 22 insertions(+), 0 deletions(-) diff --git a/REPORTING-BUGS b/REPORTING-BUGS index f86e500..c1f6e43 100644 --- a/REPORTING-BUGS +++ b/REPORTING-BUGS @@ -1,3 +1,25 @@ +Background +========== + +The upstream Linux kernel maintainers only fix bugs for specific kernel +versions. Those versions include the current "release candidate" (or -rc) +kernel, any "stable" kernel versions, and any "long term" kernels. + +Please see https://www.kernel.org/ for a list of supported kernels. Any +kernel marked with [EOL] is "end of life" and will not have any fixes +backported to it. + +If you've found a bug on a kernel version isn't listed on kernel.org, +contact your Linux distribution or embedded vendor for support. +Alternatively, you can attempt to run one of the supported stable or -rc +kernels, and see if you can reproduce the bug on that. It's preferable +to reproduce the bug on the latest -rc kernel. + + +How to report Linux kernel bugs +=============================== + + Identify the problematic subsystem ---------------------------------- -- 1.7.9
Take a look at the short description portion of the patch:
Subject: [PATCH] Docs: Add info on supported kernels to REPORTING-BUGS.
This short description should tell the reader, at a glance, what this patch is about. Usually there will be a prefix to the short description, to let the reader know which subsystem it applies to. In this case, it applies to the Documentation subsystem.
The next part of the patch is the body of the patch:
{{{One of the most common frustrations maintainers have with bug reporters is the email that starts with "I have a two year old kernel from an embedded vendor with some random drivers and fixes thrown in, and it's crashing".
Be specific about what kernel versions the upstream maintainers will fix bugs in, and direct bug reporters to their Linux distribution or embedded vendor if the bug is in an unsupported kernel.
Suggest that bug reporters should reproduce their bugs on the latest -rc kernel.
Signed-off-by: Sarah Sharp <sarah.a.sharp@linux.intel.com>}}}
This description tells why the change was made. The "why" of a patch is more important than "what" the patch is doing. If your commit is just a restatement of the diff of the code (e.g. "I changed the X function to not do Y"), you should rethink your patch description.
The body of the patch is a chance to tell a maintainer why they should take your patch. This is your chance to convince them, beyond a doubt, that they should take the time to apply your patch. If your patch is a bug fix, you should describe how to trigger the bug, and reference any [http://bugzilla.kernel.org bugzilla entries] that describe the patch, and say how this patch fixes the bug. If your patch is a new feature, you should describe why the new feature is needed, and what other code will use it. New features and drivers are the hardest patches to get in, so your patch descriptions should be air tight.
All your patch descriptions must end with your "Signed-off-by" line. This means your code meets the [http://lxr.linux.no/#linux/Documentation/SubmittingPatches#L297 Developer's certificate of origin]. Please read that whole section to ensure you're submitting your own code to the Linux kernel.
What is a patchset?
A patchset is a series of patches that are related, that describe changes to add new functionality, or fix bugs in a specific driver. The idea of a patchset is to break changes into a logical series. To understand how to arrange your patches, you have to first understand some background.
Patches are git commits
In the Linux kernel, all our history is stored in git commits. If you send a patch to a maintainer, they will apply it to their own git tree using `git am`, in order to create a git commit for it. They'll also add their Signed-off-by line to the patch, below your own, in order to certify that they have reviewed, and possibly tested, your patch.
You can see the commits by running this command in your kernel git tree:
git log
We keep the kernel under revision control in order to be able to know who made a change (by running `git blame <file>`), to be able to revert individual changes if they prove problematic (with `git revert`), and so that if there's a bug, we can bisect the commits to the specific commit that caused the issue (with `git bisect`).
It is very important that every commit in your patchset compiles properly, in order not to break git bisect.
How to break up changes
Each patch should group changes into a logical sequence. Bug fixes must come first in the patchset, then new features. This is because we need to be able to backport bug fixes to older kernels, and they should not depend on new features.
A logical change may span multiple files. For example, if you're making whitespace changes to a particular driver that is compiled from several files (like drivers/usb/host/xhci.c and drivers/usb/host/xhci-mem.c), then you would combine all those whitespace changes in one commit. If you also wanted to make other cleanups to the driver (like removing one camel-case variable name), then you would make a separate commit.
The idea here is that you should break changes up in such a way that it will be easy to review. Whitespace changes are easy to review in one patch, but may be difficult to review if other changes are thrown in. A change to one variable name is visually easy to track, but throwing in two variable name changes into one patch is hard to track. As you read more patches on the mailing list, you will get a sense of how to break up your changes in logical ways. Some kernel developers are better at this than others.
When creating a new feature patchset, you may need to break up your changes into multiple commits. For example, if you're trying to add feature A, but you find you need to refactor some code to allow new functions to call it, your first commit would be the refactoring, and the second commit would be the new feature. This makes it easier to track down breakage with `git bisect`, in case you made a mistake in your refactoring.
Clean up patches that are over 200 lines long are discouraged, because they are hard to review. Break those patches up into smaller patches. Clean up patchsets with more than 10 patches are also hard to review, so you may want to send a subset of those patches, wait for them to be accepted, and then send another patchset. These are not hard-and-fast rules, but rather ways to avoid making Linux kernel maintainers feel like they're drinking from a firehose.
Further reading
The following files are included in the Linux kernel git tree, but you can also find them through the Linux Cross reference site:
http://lxr.linux.no/#linux/Documentation/CodingStyle