Why did you write this?
A lot of this stuff isn't written down, even in CodingStyle.
- It has to be repeated relatively often.
- I hope this is a guide for _behavior_ more than the patches themselves.
Why should you do all of this?
- It will help you to find more bugs
- It will help me to find your bugs
- It will make your code more maintainable if anyone other than you ever becomes responsible for it.
For starters
- Go fetch the latest version of the kernel, and read:
[http://lxr.linux.no/source/Documentation/CodingStyle Documentation/CodingStyle ]
[http://lxr.linux.no/source/Documentation/SubmittingPatches Documentation/SubmittingPatches]
[http://lxr.linux.no/source/Documentation/SubmittingDrivers Documentation/SubmittingDrivers]
Email etiquette:
- Remember, the public Linux community has a much different culture than just about any company. Posting a patch to LKML is a lot like going to a foreign land: things that are everyday practices for you might be a major faux pas for them.
- Try to have thick skin. Some people are much more gentle than others, but almost everyone just wants to make Linux better in the end. Remember, criticism is generally made about your code, not you.
- Use a mailer which has a reputation for doing the right thing with plain-text attachments (which patches are). This means that you may want to consider using a mailer other than the one your company provides (Microsoft Outlook, Lotus Notes, etc...), at least to send or discuss patches. Mailers like pine, mutt, thunderbird, and even Evolution are known to be quite functional in this regard.
- HTML mail is not permitted on the Linux Kernel Mailing List (and many others). This is partly due to the fact that many developers read their mail in console-based mail clients. On many lists, any mail containing HTML may be silently and automatically dropped by mailing list management software. Please verify that your mail is sent only as plain-text. You can often do this by sending a mail to yourself, viewing its source, and looking for any HTML sections.
Evolution: View->Message Display->Show Email Source
- Use standard quoting techniques:
- Although "my reply in blue/red/whatever" is common inside of companies, it is not a usual way of responding to messages on any Internet mailing lists that I know of (violates "no HTML" rule anyway)
- Here's a random example from Linus which demonstrates how replies should look:
- "Top Posting" is discouraged:
- Respond quickly. Don't post a patch, then go on vacation for a week. Kernel hackers have short attention spans.
Be humble
- You always have something to learn. Learn from others, help them to teach you.
- Your driver/patch is a small pearl in a great sea. You can try to go against the flow, but...
Release early, release often
- The world of Linux changes direction on a dime, and you need to be flexible enough to change with it
- The more you work on something, the more invested in it you get. Remember that a random community member will not have the same level of commitment to your code. Try to enlighten them and confer to them why this feature is important and why you need it.
- Don't work in isolation. Code developed in a vacuum can deviate significantly enough from what is acceptable in Linux that it will never get accepted. Receiving constant feedback from other people who work on Linux should keep this from ever happening.
- You don't have to constantly release code, but you should have others' involved in your development at every step. This especially includes people outside of your company, or group.
- But, don't expect complete review until your code actually functions.
While releasing often, try to have multiple levels of review
- The people reviewing your patch are resources, don't abuse them by sending junk. Keep good records of all issues brought up with your patches and never make a reviewer repeat themselves.
- Always send patches back to yourself, first. You'll be amazed how many bugs you find when you're not looking at them through your normal editor.
- For large amounts of new work send them out to ever-increasing audiences
- Start with close coworkers
- (hope that those that know you best and will be most tolerant with you will catch most of your stupid bugs)
- Work out to larger lists, perhaps departmental
- Then, out to the smaller, more specific public mailing lists
- Lastly, if you patch is widely applicable, you can think about the larger lists like netdev or LKML
- Start with close coworkers
- Make sure to directly cc maintainers and other core people so that they're in the loop by the time you finally submit your patch.
- If you receive comments from people in one of these steps that you have addressed, post there again. Don't move on until you've completed getting reviews at each level.
Solve generic problems
- Separate these parts from your main patch (see "break patches up")
- Making a controversial change to generic code, and cementing it to your driver/patch can have disastrous results. There are many ways to approach a given problem, and it should be obvious that you're willing to try several methods to get what you want. (this doesn't really apply to things like Makefiles or Kconfig)
- This goes hand-in-hand with "release early, release often". Each release, you'll get minor redirections to keep you on track, and from "deviating too much."
- Assume others have the same problem, and try to fix it
- Sometimes, when I'm about to make a horrible hack in my code, I take a quick look around at other code that might be doing the same thing. One instance of a hack is just a hack, but two calls into the same hack establishes a new API
- Here's an example that I particularly like (because I wrote it):
[http://git.kernel.org/?p=linux/kernel/git/tglx/history.git;a=commit;h=f7e3b0c7b98b748fe462dd95918dc17afff3bc00 git link] [http://linus.bkbits.net:8080/linux-2.5/cset@4230761758K5xP7AfJjt65A5fOmLzQ bitkeeper link]
- It solves a generic problem, namely that some core VM structures were being zeroed multiple times. By fixing this "bug" these core VM routines could be reused by memory hotplug. Win-win patches are much less controversial.
- Avoid copying and pasting code. If you need a bit of code out of the middle of another function, move it out of there and into a common, global function, call it from the old place, and your new code.
- Do not ever hesitate to break functions up for performance reasons. There is zero overhead from calling an "inline" function call.
Break patches up
See 3) in [http://lxr.linux.no/source/Documentation/SubmittingPatches SubmittingPatches]
- Makes things easier to review, and ensures that you receive feedback in smaller chunks which you should be able to more easily digest.
- If your driver/patch needs changes in core, generic areas, submit those separately from my patch itself.
Especially break things up that are just icing on the cake. If a bit of your patch is "just a cleanup" or "just an optimization" but isn't part of the core work, separate it. Many times, the core functionality of your patch will be accepted without the contentious extra bits.
It has to work everywhere
- This may seem daunting: you're new to kernel development, and your patch is required to work on every architecture, 32-bit, 64-bit, big-endian, little-endian, Alpha, MIPS, even Sparc64. However, Linux has the infrastructure to do this quite easily. If someone gives you a suggestion to make it more portable, use it, even if you don't think it's needed.
- The fact that your hardware is tied to a particular platform or architecture is not a good reason not to use these abstractions. A similar device may pop up on a different architecture in the future.
- Your code can't cause anything else to break.
It helps if you bring something else to the table
- Large patches are often the most contentious. If you present a large patch it should have more and more applications and users the larger it gets.
- For instance, "I rewrote foo.c because it was messy" doesn't tend to go over well. "I rewrote foo.c, and now we can remove bar.c and baz.c" carries much more weight.
- It doesn't even have to be an immediate benefit. Saying a feature will be used by subsystem A in the next few months, and getting the maintainer to publicly agree with you can also be very effective.
Things beyond the code itself are important
- Make sure that your comments and your patch descriptions make sense. A good description can make the difference between acceptance and rejection.
[http://marc.info/?l=linux-mm&m=120821390007638&w=2 Advice from Andrew Morton on Changelogs] and [http://userweb.kernel.org/~akpm/stuff/tpp.txt the perfect patch] A review should be able to easily determine this after reading the changelog:
- What problems does the patchset solve?
- How does the code work?
- What is the overall design?
- Any implementation details or shortcomings or todos which we should know about?
- If you are not a native english speaker, seek someone out who is to help you review. Sometimes, there are subtle language implications from variable names, or other things that are hard to pick out if you are not a native speaker.
Testing is very important
- Ensure your patch series is bisectable (builds and boots at each point in the series).
- If you can't find a test case, write your own, and share it when posting your patches.
See Also
[http://ldn.linuxfoundation.org/book/how-participate-linux-community Linux Foundation How to Participate in the Linux Community]