10e4f07a6SMauro Carvalho Chehab.. _development_followthrough: 20e4f07a6SMauro Carvalho Chehab 30e4f07a6SMauro Carvalho ChehabFollowthrough 40e4f07a6SMauro Carvalho Chehab============= 50e4f07a6SMauro Carvalho Chehab 60e4f07a6SMauro Carvalho ChehabAt this point, you have followed the guidelines given so far and, with the 70e4f07a6SMauro Carvalho Chehabaddition of your own engineering skills, have posted a perfect series of 80e4f07a6SMauro Carvalho Chehabpatches. One of the biggest mistakes that even experienced kernel 90e4f07a6SMauro Carvalho Chehabdevelopers can make is to conclude that their work is now done. In truth, 100e4f07a6SMauro Carvalho Chehabposting patches indicates a transition into the next stage of the process, 110e4f07a6SMauro Carvalho Chehabwith, possibly, quite a bit of work yet to be done. 120e4f07a6SMauro Carvalho Chehab 130e4f07a6SMauro Carvalho ChehabIt is a rare patch which is so good at its first posting that there is no 140e4f07a6SMauro Carvalho Chehabroom for improvement. The kernel development process recognizes this fact, 150e4f07a6SMauro Carvalho Chehaband, as a result, is heavily oriented toward the improvement of posted 160e4f07a6SMauro Carvalho Chehabcode. You, as the author of that code, will be expected to work with the 170e4f07a6SMauro Carvalho Chehabkernel community to ensure that your code is up to the kernel's quality 180e4f07a6SMauro Carvalho Chehabstandards. A failure to participate in this process is quite likely to 190e4f07a6SMauro Carvalho Chehabprevent the inclusion of your patches into the mainline. 200e4f07a6SMauro Carvalho Chehab 210e4f07a6SMauro Carvalho Chehab 220e4f07a6SMauro Carvalho ChehabWorking with reviewers 230e4f07a6SMauro Carvalho Chehab---------------------- 240e4f07a6SMauro Carvalho Chehab 250e4f07a6SMauro Carvalho ChehabA patch of any significance will result in a number of comments from other 260e4f07a6SMauro Carvalho Chehabdevelopers as they review the code. Working with reviewers can be, for 270e4f07a6SMauro Carvalho Chehabmany developers, the most intimidating part of the kernel development 280e4f07a6SMauro Carvalho Chehabprocess. Life can be made much easier, though, if you keep a few things in 290e4f07a6SMauro Carvalho Chehabmind: 300e4f07a6SMauro Carvalho Chehab 310e4f07a6SMauro Carvalho Chehab - If you have explained your patch well, reviewers will understand its 320e4f07a6SMauro Carvalho Chehab value and why you went to the trouble of writing it. But that value 330e4f07a6SMauro Carvalho Chehab will not keep them from asking a fundamental question: what will it be 340e4f07a6SMauro Carvalho Chehab like to maintain a kernel with this code in it five or ten years later? 350e4f07a6SMauro Carvalho Chehab Many of the changes you may be asked to make - from coding style tweaks 360e4f07a6SMauro Carvalho Chehab to substantial rewrites - come from the understanding that Linux will 370e4f07a6SMauro Carvalho Chehab still be around and under development a decade from now. 380e4f07a6SMauro Carvalho Chehab 390e4f07a6SMauro Carvalho Chehab - Code review is hard work, and it is a relatively thankless occupation; 400e4f07a6SMauro Carvalho Chehab people remember who wrote kernel code, but there is little lasting fame 410e4f07a6SMauro Carvalho Chehab for those who reviewed it. So reviewers can get grumpy, especially when 420e4f07a6SMauro Carvalho Chehab they see the same mistakes being made over and over again. If you get a 430e4f07a6SMauro Carvalho Chehab review which seems angry, insulting, or outright offensive, resist the 440e4f07a6SMauro Carvalho Chehab impulse to respond in kind. Code review is about the code, not about 450e4f07a6SMauro Carvalho Chehab the people, and code reviewers are not attacking you personally. 460e4f07a6SMauro Carvalho Chehab 470e4f07a6SMauro Carvalho Chehab - Similarly, code reviewers are not trying to promote their employers' 480e4f07a6SMauro Carvalho Chehab agendas at the expense of your own. Kernel developers often expect to 490e4f07a6SMauro Carvalho Chehab be working on the kernel years from now, but they understand that their 500e4f07a6SMauro Carvalho Chehab employer could change. They truly are, almost without exception, 510e4f07a6SMauro Carvalho Chehab working toward the creation of the best kernel they can; they are not 520e4f07a6SMauro Carvalho Chehab trying to create discomfort for their employers' competitors. 530e4f07a6SMauro Carvalho Chehab 54*b45d8f38SJakub Kicinski - Be prepared for seemingly silly requests for coding style changes 55*b45d8f38SJakub Kicinski and requests to factor out some of your code to shared parts of 56*b45d8f38SJakub Kicinski the kernel. One job the maintainers do is to keep things looking 57*b45d8f38SJakub Kicinski the same. Sometimes this means that the clever hack in your driver 58*b45d8f38SJakub Kicinski to get around a problem actually needs to become a generalized 59*b45d8f38SJakub Kicinski kernel feature ready for next time. 60*b45d8f38SJakub Kicinski 610e4f07a6SMauro Carvalho ChehabWhat all of this comes down to is that, when reviewers send you comments, 620e4f07a6SMauro Carvalho Chehabyou need to pay attention to the technical observations that they are 630e4f07a6SMauro Carvalho Chehabmaking. Do not let their form of expression or your own pride keep that 640e4f07a6SMauro Carvalho Chehabfrom happening. When you get review comments on a patch, take the time to 650e4f07a6SMauro Carvalho Chehabunderstand what the reviewer is trying to say. If possible, fix the things 660e4f07a6SMauro Carvalho Chehabthat the reviewer is asking you to fix. And respond back to the reviewer: 670e4f07a6SMauro Carvalho Chehabthank them, and describe how you will answer their questions. 680e4f07a6SMauro Carvalho Chehab 690e4f07a6SMauro Carvalho ChehabNote that you do not have to agree with every change suggested by 700e4f07a6SMauro Carvalho Chehabreviewers. If you believe that the reviewer has misunderstood your code, 710e4f07a6SMauro Carvalho Chehabexplain what is really going on. If you have a technical objection to a 720e4f07a6SMauro Carvalho Chehabsuggested change, describe it and justify your solution to the problem. If 730e4f07a6SMauro Carvalho Chehabyour explanations make sense, the reviewer will accept them. Should your 740e4f07a6SMauro Carvalho Chehabexplanation not prove persuasive, though, especially if others start to 750e4f07a6SMauro Carvalho Chehabagree with the reviewer, take some time to think things over again. It can 760e4f07a6SMauro Carvalho Chehabbe easy to become blinded by your own solution to a problem to the point 770e4f07a6SMauro Carvalho Chehabthat you don't realize that something is fundamentally wrong or, perhaps, 780e4f07a6SMauro Carvalho Chehabyou're not even solving the right problem. 790e4f07a6SMauro Carvalho Chehab 800e4f07a6SMauro Carvalho ChehabAndrew Morton has suggested that every review comment which does not result 810e4f07a6SMauro Carvalho Chehabin a code change should result in an additional code comment instead; that 820e4f07a6SMauro Carvalho Chehabcan help future reviewers avoid the questions which came up the first time 830e4f07a6SMauro Carvalho Chehabaround. 840e4f07a6SMauro Carvalho Chehab 850e4f07a6SMauro Carvalho ChehabOne fatal mistake is to ignore review comments in the hope that they will 860e4f07a6SMauro Carvalho Chehabgo away. They will not go away. If you repost code without having 870e4f07a6SMauro Carvalho Chehabresponded to the comments you got the time before, you're likely to find 880e4f07a6SMauro Carvalho Chehabthat your patches go nowhere. 890e4f07a6SMauro Carvalho Chehab 900e4f07a6SMauro Carvalho ChehabSpeaking of reposting code: please bear in mind that reviewers are not 910e4f07a6SMauro Carvalho Chehabgoing to remember all the details of the code you posted the last time 920e4f07a6SMauro Carvalho Chehabaround. So it is always a good idea to remind reviewers of previously 930e4f07a6SMauro Carvalho Chehabraised issues and how you dealt with them; the patch changelog is a good 940e4f07a6SMauro Carvalho Chehabplace for this kind of information. Reviewers should not have to search 950e4f07a6SMauro Carvalho Chehabthrough list archives to familiarize themselves with what was said last 960e4f07a6SMauro Carvalho Chehabtime; if you help them get a running start, they will be in a better mood 970e4f07a6SMauro Carvalho Chehabwhen they revisit your code. 980e4f07a6SMauro Carvalho Chehab 990e4f07a6SMauro Carvalho ChehabWhat if you've tried to do everything right and things still aren't going 1000e4f07a6SMauro Carvalho Chehabanywhere? Most technical disagreements can be resolved through discussion, 1010e4f07a6SMauro Carvalho Chehabbut there are times when somebody simply has to make a decision. If you 1020e4f07a6SMauro Carvalho Chehabhonestly believe that this decision is going against you wrongly, you can 1030e4f07a6SMauro Carvalho Chehabalways try appealing to a higher power. As of this writing, that higher 1040e4f07a6SMauro Carvalho Chehabpower tends to be Andrew Morton. Andrew has a great deal of respect in the 1050e4f07a6SMauro Carvalho Chehabkernel development community; he can often unjam a situation which seems to 1060e4f07a6SMauro Carvalho Chehabbe hopelessly blocked. Appealing to Andrew should not be done lightly, 1070e4f07a6SMauro Carvalho Chehabthough, and not before all other alternatives have been explored. And bear 1080e4f07a6SMauro Carvalho Chehabin mind, of course, that he may not agree with you either. 1090e4f07a6SMauro Carvalho Chehab 1100e4f07a6SMauro Carvalho Chehab 1110e4f07a6SMauro Carvalho ChehabWhat happens next 1120e4f07a6SMauro Carvalho Chehab----------------- 1130e4f07a6SMauro Carvalho Chehab 1140e4f07a6SMauro Carvalho ChehabIf a patch is considered to be a good thing to add to the kernel, and once 1150e4f07a6SMauro Carvalho Chehabmost of the review issues have been resolved, the next step is usually 1160e4f07a6SMauro Carvalho Chehabentry into a subsystem maintainer's tree. How that works varies from one 1170e4f07a6SMauro Carvalho Chehabsubsystem to the next; each maintainer has his or her own way of doing 1180e4f07a6SMauro Carvalho Chehabthings. In particular, there may be more than one tree - one, perhaps, 1190e4f07a6SMauro Carvalho Chehabdedicated to patches planned for the next merge window, and another for 1200e4f07a6SMauro Carvalho Chehablonger-term work. 1210e4f07a6SMauro Carvalho Chehab 1220e4f07a6SMauro Carvalho ChehabFor patches applying to areas for which there is no obvious subsystem tree 1230e4f07a6SMauro Carvalho Chehab(memory management patches, for example), the default tree often ends up 1240e4f07a6SMauro Carvalho Chehabbeing -mm. Patches which affect multiple subsystems can also end up going 1250e4f07a6SMauro Carvalho Chehabthrough the -mm tree. 1260e4f07a6SMauro Carvalho Chehab 1270e4f07a6SMauro Carvalho ChehabInclusion into a subsystem tree can bring a higher level of visibility to a 1280e4f07a6SMauro Carvalho Chehabpatch. Now other developers working with that tree will get the patch by 1290e4f07a6SMauro Carvalho Chehabdefault. Subsystem trees typically feed linux-next as well, making their 1300e4f07a6SMauro Carvalho Chehabcontents visible to the development community as a whole. At this point, 1310e4f07a6SMauro Carvalho Chehabthere's a good chance that you will get more comments from a new set of 1320e4f07a6SMauro Carvalho Chehabreviewers; these comments need to be answered as in the previous round. 1330e4f07a6SMauro Carvalho Chehab 1340e4f07a6SMauro Carvalho ChehabWhat may also happen at this point, depending on the nature of your patch, 1350e4f07a6SMauro Carvalho Chehabis that conflicts with work being done by others turn up. In the worst 1360e4f07a6SMauro Carvalho Chehabcase, heavy patch conflicts can result in some work being put on the back 1370e4f07a6SMauro Carvalho Chehabburner so that the remaining patches can be worked into shape and merged. 1380e4f07a6SMauro Carvalho ChehabOther times, conflict resolution will involve working with the other 1390e4f07a6SMauro Carvalho Chehabdevelopers and, possibly, moving some patches between trees to ensure that 1400e4f07a6SMauro Carvalho Chehabeverything applies cleanly. This work can be a pain, but count your 1410e4f07a6SMauro Carvalho Chehabblessings: before the advent of the linux-next tree, these conflicts often 1420e4f07a6SMauro Carvalho Chehabonly turned up during the merge window and had to be addressed in a hurry. 1430e4f07a6SMauro Carvalho ChehabNow they can be resolved at leisure, before the merge window opens. 1440e4f07a6SMauro Carvalho Chehab 1450e4f07a6SMauro Carvalho ChehabSome day, if all goes well, you'll log on and see that your patch has been 1460e4f07a6SMauro Carvalho Chehabmerged into the mainline kernel. Congratulations! Once the celebration is 1470e4f07a6SMauro Carvalho Chehabcomplete (and you have added yourself to the MAINTAINERS file), though, it 1480e4f07a6SMauro Carvalho Chehabis worth remembering an important little fact: the job still is not done. 1490e4f07a6SMauro Carvalho ChehabMerging into the mainline brings its own challenges. 1500e4f07a6SMauro Carvalho Chehab 1510e4f07a6SMauro Carvalho ChehabTo begin with, the visibility of your patch has increased yet again. There 1520e4f07a6SMauro Carvalho Chehabmay be a new round of comments from developers who had not been aware of 1530e4f07a6SMauro Carvalho Chehabthe patch before. It may be tempting to ignore them, since there is no 1540e4f07a6SMauro Carvalho Chehablonger any question of your code being merged. Resist that temptation, 1550e4f07a6SMauro Carvalho Chehabthough; you still need to be responsive to developers who have questions or 1560e4f07a6SMauro Carvalho Chehabsuggestions. 1570e4f07a6SMauro Carvalho Chehab 1580e4f07a6SMauro Carvalho ChehabMore importantly, though: inclusion into the mainline puts your code into 1590e4f07a6SMauro Carvalho Chehabthe hands of a much larger group of testers. Even if you have contributed 1600e4f07a6SMauro Carvalho Chehaba driver for hardware which is not yet available, you will be surprised by 1610e4f07a6SMauro Carvalho Chehabhow many people will build your code into their kernels. And, of course, 1620e4f07a6SMauro Carvalho Chehabwhere there are testers, there will be bug reports. 1630e4f07a6SMauro Carvalho Chehab 1640e4f07a6SMauro Carvalho ChehabThe worst sort of bug reports are regressions. If your patch causes a 1650e4f07a6SMauro Carvalho Chehabregression, you'll find an uncomfortable number of eyes upon you; 1660e4f07a6SMauro Carvalho Chehabregressions need to be fixed as soon as possible. If you are unwilling or 1670e4f07a6SMauro Carvalho Chehabunable to fix the regression (and nobody else does it for you), your patch 1680e4f07a6SMauro Carvalho Chehabwill almost certainly be removed during the stabilization period. Beyond 1690e4f07a6SMauro Carvalho Chehabnegating all of the work you have done to get your patch into the mainline, 1700e4f07a6SMauro Carvalho Chehabhaving a patch pulled as the result of a failure to fix a regression could 1710e4f07a6SMauro Carvalho Chehabwell make it harder for you to get work merged in the future. 1720e4f07a6SMauro Carvalho Chehab 1730e4f07a6SMauro Carvalho ChehabAfter any regressions have been dealt with, there may be other, ordinary 1740e4f07a6SMauro Carvalho Chehabbugs to deal with. The stabilization period is your best opportunity to 1750e4f07a6SMauro Carvalho Chehabfix these bugs and ensure that your code's debut in a mainline kernel 1760e4f07a6SMauro Carvalho Chehabrelease is as solid as possible. So, please, answer bug reports, and fix 1770e4f07a6SMauro Carvalho Chehabthe problems if at all possible. That's what the stabilization period is 1780e4f07a6SMauro Carvalho Chehabfor; you can start creating cool new patches once any problems with the old 1790e4f07a6SMauro Carvalho Chehabones have been taken care of. 1800e4f07a6SMauro Carvalho Chehab 1810e4f07a6SMauro Carvalho ChehabAnd don't forget that there are other milestones which may also create bug 1820e4f07a6SMauro Carvalho Chehabreports: the next mainline stable release, when prominent distributors pick 1830e4f07a6SMauro Carvalho Chehabup a version of the kernel containing your patch, etc. Continuing to 1840e4f07a6SMauro Carvalho Chehabrespond to these reports is a matter of basic pride in your work. If that 1850e4f07a6SMauro Carvalho Chehabis insufficient motivation, though, it's also worth considering that the 1860e4f07a6SMauro Carvalho Chehabdevelopment community remembers developers who lose interest in their code 1870e4f07a6SMauro Carvalho Chehabafter it's merged. The next time you post a patch, they will be evaluating 1880e4f07a6SMauro Carvalho Chehabit with the assumption that you will not be around to maintain it 1890e4f07a6SMauro Carvalho Chehabafterward. 1900e4f07a6SMauro Carvalho Chehab 1910e4f07a6SMauro Carvalho Chehab 1920e4f07a6SMauro Carvalho ChehabOther things that can happen 1930e4f07a6SMauro Carvalho Chehab----------------------------- 1940e4f07a6SMauro Carvalho Chehab 1950e4f07a6SMauro Carvalho ChehabOne day, you may open your mail client and see that somebody has mailed you 1960e4f07a6SMauro Carvalho Chehaba patch to your code. That is one of the advantages of having your code 1970e4f07a6SMauro Carvalho Chehabout there in the open, after all. If you agree with the patch, you can 1980e4f07a6SMauro Carvalho Chehabeither forward it on to the subsystem maintainer (be sure to include a 1990e4f07a6SMauro Carvalho Chehabproper From: line so that the attribution is correct, and add a signoff of 2000e4f07a6SMauro Carvalho Chehabyour own), or send an Acked-by: response back and let the original poster 2010e4f07a6SMauro Carvalho Chehabsend it upward. 2020e4f07a6SMauro Carvalho Chehab 2030e4f07a6SMauro Carvalho ChehabIf you disagree with the patch, send a polite response explaining why. If 2040e4f07a6SMauro Carvalho Chehabpossible, tell the author what changes need to be made to make the patch 2050e4f07a6SMauro Carvalho Chehabacceptable to you. There is a certain resistance to merging patches which 2060e4f07a6SMauro Carvalho Chehabare opposed by the author and maintainer of the code, but it only goes so 2070e4f07a6SMauro Carvalho Chehabfar. If you are seen as needlessly blocking good work, those patches will 2080e4f07a6SMauro Carvalho Chehabeventually flow around you and get into the mainline anyway. In the Linux 2090e4f07a6SMauro Carvalho Chehabkernel, nobody has absolute veto power over any code. Except maybe Linus. 2100e4f07a6SMauro Carvalho Chehab 2110e4f07a6SMauro Carvalho ChehabOn very rare occasion, you may see something completely different: another 2120e4f07a6SMauro Carvalho Chehabdeveloper posts a different solution to your problem. At that point, 2130e4f07a6SMauro Carvalho Chehabchances are that one of the two patches will not be merged, and "mine was 2140e4f07a6SMauro Carvalho Chehabhere first" is not considered to be a compelling technical argument. If 2150e4f07a6SMauro Carvalho Chehabsomebody else's patch displaces yours and gets into the mainline, there is 2160e4f07a6SMauro Carvalho Chehabreally only one way to respond: be pleased that your problem got solved and 2170e4f07a6SMauro Carvalho Chehabget on with your work. Having one's work shoved aside in this manner can 2180e4f07a6SMauro Carvalho Chehabbe hurtful and discouraging, but the community will remember your reaction 2190e4f07a6SMauro Carvalho Chehablong after they have forgotten whose patch actually got merged. 220