197 lines · plain
1.. _development_advancedtopics:2 3Advanced topics4===============5 6At this point, hopefully, you have a handle on how the development process7works. There is still more to learn, however! This section will cover a8number of topics which can be helpful for developers wanting to become a9regular part of the Linux kernel development process.10 11Managing patches with git12-------------------------13 14The use of distributed version control for the kernel began in early 2002,15when Linus first started playing with the proprietary BitKeeper16application. While BitKeeper was controversial, the approach to software17version management it embodied most certainly was not. Distributed version18control enabled an immediate acceleration of the kernel development19project. In current times, there are several free alternatives to20BitKeeper. For better or for worse, the kernel project has settled on git21as its tool of choice.22 23Managing patches with git can make life much easier for the developer,24especially as the volume of those patches grows. Git also has its rough25edges and poses certain hazards; it is a young and powerful tool which is26still being civilized by its developers. This document will not attempt to27teach the reader how to use git; that would be sufficient material for a28long document in its own right. Instead, the focus here will be on how git29fits into the kernel development process in particular. Developers who30wish to come up to speed with git will find more information at:31 32 https://git-scm.com/33 34 https://www.kernel.org/pub/software/scm/git/docs/user-manual.html35 36and on various tutorials found on the web.37 38The first order of business is to read the above sites and get a solid39understanding of how git works before trying to use it to make patches40available to others. A git-using developer should be able to obtain a copy41of the mainline repository, explore the revision history, commit changes to42the tree, use branches, etc. An understanding of git's tools for the43rewriting of history (such as rebase) is also useful. Git comes with its44own terminology and concepts; a new user of git should know about refs,45remote branches, the index, fast-forward merges, pushes and pulls, detached46heads, etc. It can all be a little intimidating at the outset, but the47concepts are not that hard to grasp with a bit of study.48 49Using git to generate patches for submission by email can be a good50exercise while coming up to speed.51 52When you are ready to start putting up git trees for others to look at, you53will, of course, need a server that can be pulled from. Setting up such a54server with git-daemon is relatively straightforward if you have a system55which is accessible to the Internet. Otherwise, free, public hosting sites56(Github, for example) are starting to appear on the net. Established57developers can get an account on kernel.org, but those are not easy to come58by; see https://kernel.org/faq/ for more information.59 60The normal git workflow involves the use of a lot of branches. Each line61of development can be separated into a separate "topic branch" and62maintained independently. Branches in git are cheap, there is no reason to63not make free use of them. And, in any case, you should not do your64development in any branch which you intend to ask others to pull from.65Publicly-available branches should be created with care; merge in patches66from development branches when they are in complete form and ready to go -67not before.68 69Git provides some powerful tools which can allow you to rewrite your70development history. An inconvenient patch (one which breaks bisection,71say, or which has some other sort of obvious bug) can be fixed in place or72made to disappear from the history entirely. A patch series can be73rewritten as if it had been written on top of today's mainline, even though74you have been working on it for months. Changes can be transparently75shifted from one branch to another. And so on. Judicious use of git's76ability to revise history can help in the creation of clean patch sets with77fewer problems.78 79Excessive use of this capability can lead to other problems, though, beyond80a simple obsession for the creation of the perfect project history.81Rewriting history will rewrite the changes contained in that history,82turning a tested (hopefully) kernel tree into an untested one. But, beyond83that, developers cannot easily collaborate if they do not have a shared84view of the project history; if you rewrite history which other developers85have pulled into their repositories, you will make life much more difficult86for those developers. So a simple rule of thumb applies here: history87which has been exported to others should generally be seen as immutable88thereafter.89 90So, once you push a set of changes to your publicly-available server, those91changes should not be rewritten. Git will attempt to enforce this rule if92you try to push changes which do not result in a fast-forward merge93(i.e. changes which do not share the same history). It is possible to94override this check, and there may be times when it is necessary to rewrite95an exported tree. Moving changesets between trees to avoid conflicts in96linux-next is one example. But such actions should be rare. This is one97of the reasons why development should be done in private branches (which98can be rewritten if necessary) and only moved into public branches when99it's in a reasonably advanced state.100 101As the mainline (or other tree upon which a set of changes is based)102advances, it is tempting to merge with that tree to stay on the leading103edge. For a private branch, rebasing can be an easy way to keep up with104another tree, but rebasing is not an option once a tree is exported to the105world. Once that happens, a full merge must be done. Merging occasionally106makes good sense, but overly frequent merges can clutter the history107needlessly. Suggested technique in this case is to merge infrequently, and108generally only at specific release points (such as a mainline -rc109release). If you are nervous about specific changes, you can always110perform test merges in a private branch. The git "rerere" tool can be111useful in such situations; it remembers how merge conflicts were resolved112so that you don't have to do the same work twice.113 114One of the biggest recurring complaints about tools like git is this: the115mass movement of patches from one repository to another makes it easy to116slip in ill-advised changes which go into the mainline below the review117radar. Kernel developers tend to get unhappy when they see that kind of118thing happening; putting up a git tree with unreviewed or off-topic patches119can affect your ability to get trees pulled in the future. Quoting Linus:120 121::122 123 You can send me patches, but for me to pull a git patch from you, I124 need to know that you know what you're doing, and I need to be able125 to trust things *without* then having to go and check every126 individual change by hand.127 128(https://lwn.net/Articles/224135/).129 130To avoid this kind of situation, ensure that all patches within a given131branch stick closely to the associated topic; a "driver fixes" branch132should not be making changes to the core memory management code. And, most133importantly, do not use a git tree to bypass the review process. Post an134occasional summary of the tree to the relevant list, and, when the time is135right, request that the tree be included in linux-next.136 137If and when others start to send patches for inclusion into your tree,138don't forget to review them. Also ensure that you maintain the correct139authorship information; the git "am" tool does its best in this regard, but140you may have to add a "From:" line to the patch if it has been relayed to141you via a third party.142 143When requesting a pull, be sure to give all the relevant information: where144your tree is, what branch to pull, and what changes will result from the145pull. The git request-pull command can be helpful in this regard; it will146format the request as other developers expect, and will also check to be147sure that you have remembered to push those changes to the public server.148 149.. _development_advancedtopics_reviews:150 151Reviewing patches152-----------------153 154Some readers will certainly object to putting this section with "advanced155topics" on the grounds that even beginning kernel developers should be156reviewing patches. It is certainly true that there is no better way to157learn how to program in the kernel environment than by looking at code158posted by others. In addition, reviewers are forever in short supply; by159looking at code you can make a significant contribution to the process as a160whole.161 162Reviewing code can be an intimidating prospect, especially for a new kernel163developer who may well feel nervous about questioning code - in public -164which has been posted by those with more experience. Even code written by165the most experienced developers can be improved, though. Perhaps the best166piece of advice for reviewers (all reviewers) is this: phrase review167comments as questions rather than criticisms. Asking "how does the lock168get released in this path?" will always work better than stating "the169locking here is wrong."170 171Another technique that is useful in case of a disagreement is to ask for others172to chime in. If a discussion reaches a stalemate after a few exchanges,173then call for opinions of other reviewers or maintainers. Often those in174agreement with a reviewer remain silent unless called upon.175The opinion of multiple people carries exponentially more weight.176 177Different developers will review code from different points of view. Some178are mostly concerned with coding style and whether code lines have trailing179white space. Others will focus primarily on whether the change implemented180by the patch as a whole is a good thing for the kernel or not. Yet others181will check for problematic locking, excessive stack usage, possible182security issues, duplication of code found elsewhere, adequate183documentation, adverse effects on performance, user-space ABI changes, etc.184All types of review, if they lead to better code going into the kernel, are185welcome and worthwhile.186 187There is no strict requirement to use specific tags like ``Reviewed-by``.188In fact reviews in plain English are more informative and encouraged189even when a tag is provided, e.g. "I looked at aspects A, B and C of this190submission and it looks good to me."191Some form of a review message or reply is obviously necessary otherwise192maintainers will not know that the reviewer has looked at the patch at all!193 194Last but not least patch review may become a negative process, focused195on pointing out problems. Please throw in a compliment once in a while,196particularly for newbies!197