279 lines
		
	
	
	
		
			13 KiB
			
		
	
	
	
		
			Text
		
	
	
	
	
	
		
		
			
		
	
	
			279 lines
		
	
	
	
		
			13 KiB
			
		
	
	
	
		
			Text
		
	
	
	
	
	
|   | 5: POSTING PATCHES | ||
|  | 
 | ||
|  | Sooner or later, the time comes when your work is ready to be presented to | ||
|  | the community for review and, eventually, inclusion into the mainline | ||
|  | kernel.  Unsurprisingly, the kernel development community has evolved a set | ||
|  | of conventions and procedures which are used in the posting of patches; | ||
|  | following them will make life much easier for everybody involved.  This | ||
|  | document will attempt to cover these expectations in reasonable detail; | ||
|  | more information can also be found in the files SubmittingPatches, | ||
|  | SubmittingDrivers, and SubmitChecklist in the kernel documentation | ||
|  | directory. | ||
|  | 
 | ||
|  | 
 | ||
|  | 5.1: WHEN TO POST | ||
|  | 
 | ||
|  | There is a constant temptation to avoid posting patches before they are | ||
|  | completely "ready."  For simple patches, that is not a problem.  If the | ||
|  | work being done is complex, though, there is a lot to be gained by getting | ||
|  | feedback from the community before the work is complete.  So you should | ||
|  | consider posting in-progress work, or even making a git tree available so | ||
|  | that interested developers can catch up with your work at any time. | ||
|  | 
 | ||
|  | When posting code which is not yet considered ready for inclusion, it is a | ||
|  | good idea to say so in the posting itself.  Also mention any major work | ||
|  | which remains to be done and any known problems.  Fewer people will look at | ||
|  | patches which are known to be half-baked, but those who do will come in | ||
|  | with the idea that they can help you drive the work in the right direction. | ||
|  | 
 | ||
|  | 
 | ||
|  | 5.2: BEFORE CREATING PATCHES | ||
|  | 
 | ||
|  | There are a number of things which should be done before you consider | ||
|  | sending patches to the development community.  These include: | ||
|  | 
 | ||
|  |  - Test the code to the extent that you can.  Make use of the kernel's | ||
|  |    debugging tools, ensure that the kernel will build with all reasonable | ||
|  |    combinations of configuration options, use cross-compilers to build for | ||
|  |    different architectures, etc. | ||
|  | 
 | ||
|  |  - Make sure your code is compliant with the kernel coding style | ||
|  |    guidelines. | ||
|  | 
 | ||
|  |  - Does your change have performance implications?  If so, you should run | ||
|  |    benchmarks showing what the impact (or benefit) of your change is; a | ||
|  |    summary of the results should be included with the patch. | ||
|  | 
 | ||
|  |  - Be sure that you have the right to post the code.  If this work was done | ||
|  |    for an employer, the employer likely has a right to the work and must be | ||
|  |    agreeable with its release under the GPL. | ||
|  | 
 | ||
|  | As a general rule, putting in some extra thought before posting code almost | ||
|  | always pays back the effort in short order. | ||
|  | 
 | ||
|  | 
 | ||
|  | 5.3: PATCH PREPARATION | ||
|  | 
 | ||
|  | The preparation of patches for posting can be a surprising amount of work, | ||
|  | but, once again, attempting to save time here is not generally advisable | ||
|  | even in the short term. | ||
|  | 
 | ||
|  | Patches must be prepared against a specific version of the kernel.  As a | ||
|  | general rule, a patch should be based on the current mainline as found in | ||
|  | Linus's git tree.  It may become necessary to make versions against -mm, | ||
|  | linux-next, or a subsystem tree, though, to facilitate wider testing and | ||
|  | review.  Depending on the area of your patch and what is going on | ||
|  | elsewhere, basing a patch against these other trees can require a | ||
|  | significant amount of work resolving conflicts and dealing with API | ||
|  | changes. | ||
|  | 
 | ||
|  | Only the most simple changes should be formatted as a single patch; | ||
|  | everything else should be made as a logical series of changes.  Splitting | ||
|  | up patches is a bit of an art; some developers spend a long time figuring | ||
|  | out how to do it in the way that the community expects.  There are a few | ||
|  | rules of thumb, however, which can help considerably: | ||
|  | 
 | ||
|  |  - The patch series you post will almost certainly not be the series of | ||
|  |    changes found in your working revision control system.  Instead, the | ||
|  |    changes you have made need to be considered in their final form, then | ||
|  |    split apart in ways which make sense.  The developers are interested in | ||
|  |    discrete, self-contained changes, not the path you took to get to those | ||
|  |    changes. | ||
|  | 
 | ||
|  |  - Each logically independent change should be formatted as a separate | ||
|  |    patch.  These changes can be small ("add a field to this structure") or | ||
|  |    large (adding a significant new driver, for example), but they should be | ||
|  |    conceptually small and amenable to a one-line description.  Each patch | ||
|  |    should make a specific change which can be reviewed on its own and | ||
|  |    verified to do what it says it does. | ||
|  | 
 | ||
|  |  - As a way of restating the guideline above: do not mix different types of | ||
|  |    changes in the same patch.  If a single patch fixes a critical security | ||
|  |    bug, rearranges a few structures, and reformats the code, there is a | ||
|  |    good chance that it will be passed over and the important fix will be | ||
|  |    lost. | ||
|  | 
 | ||
|  |  - Each patch should yield a kernel which builds and runs properly; if your | ||
|  |    patch series is interrupted in the middle, the result should still be a | ||
|  |    working kernel.  Partial application of a patch series is a common | ||
|  |    scenario when the "git bisect" tool is used to find regressions; if the | ||
|  |    result is a broken kernel, you will make life harder for developers and | ||
|  |    users who are engaging in the noble work of tracking down problems. | ||
|  | 
 | ||
|  |  - Do not overdo it, though.  One developer recently posted a set of edits | ||
|  |    to a single file as 500 separate patches - an act which did not make him | ||
|  |    the most popular person on the kernel mailing list.  A single patch can | ||
|  |    be reasonably large as long as it still contains a single *logical* | ||
|  |    change.  | ||
|  | 
 | ||
|  |  - It can be tempting to add a whole new infrastructure with a series of | ||
|  |    patches, but to leave that infrastructure unused until the final patch | ||
|  |    in the series enables the whole thing.  This temptation should be | ||
|  |    avoided if possible; if that series adds regressions, bisection will | ||
|  |    finger the last patch as the one which caused the problem, even though | ||
|  |    the real bug is elsewhere.  Whenever possible, a patch which adds new | ||
|  |    code should make that code active immediately. | ||
|  | 
 | ||
|  | Working to create the perfect patch series can be a frustrating process | ||
|  | which takes quite a bit of time and thought after the "real work" has been | ||
|  | done.  When done properly, though, it is time well spent. | ||
|  | 
 | ||
|  | 
 | ||
|  | 5.4: PATCH FORMATTING | ||
|  | 
 | ||
|  | So now you have a perfect series of patches for posting, but the work is | ||
|  | not done quite yet.  Each patch needs to be formatted into a message which | ||
|  | quickly and clearly communicates its purpose to the rest of the world.  To | ||
|  | that end, each patch will be composed of the following: | ||
|  | 
 | ||
|  |  - An optional "From" line naming the author of the patch.  This line is | ||
|  |    only necessary if you are passing on somebody else's patch via email, | ||
|  |    but it never hurts to add it when in doubt. | ||
|  | 
 | ||
|  |  - A one-line description of what the patch does.  This message should be | ||
|  |    enough for a reader who sees it with no other context to figure out the | ||
|  |    scope of the patch; it is the line that will show up in the "short form" | ||
|  |    changelogs.  This message is usually formatted with the relevant | ||
|  |    subsystem name first, followed by the purpose of the patch.  For | ||
|  |    example: | ||
|  | 
 | ||
|  | 	gpio: fix build on CONFIG_GPIO_SYSFS=n | ||
|  | 
 | ||
|  |  - A blank line followed by a detailed description of the contents of the | ||
|  |    patch.  This description can be as long as is required; it should say | ||
|  |    what the patch does and why it should be applied to the kernel. | ||
|  | 
 | ||
|  |  - One or more tag lines, with, at a minimum, one Signed-off-by: line from | ||
|  |    the author of the patch.  Tags will be described in more detail below. | ||
|  | 
 | ||
|  | The above three items should, normally, be the text used when committing | ||
|  | the change to a revision control system.  They are followed by: | ||
|  | 
 | ||
|  |  - The patch itself, in the unified ("-u") patch format.  Using the "-p" | ||
|  |    option to diff will associate function names with changes, making the | ||
|  |    resulting patch easier for others to read. | ||
|  | 
 | ||
|  | You should avoid including changes to irrelevant files (those generated by | ||
|  | the build process, for example, or editor backup files) in the patch.  The | ||
|  | file "dontdiff" in the Documentation directory can help in this regard; | ||
|  | pass it to diff with the "-X" option. | ||
|  | 
 | ||
|  | The tags mentioned above are used to describe how various developers have | ||
|  | been associated with the development of this patch.  They are described in | ||
|  | detail in the SubmittingPatches document; what follows here is a brief | ||
|  | summary.  Each of these lines has the format: | ||
|  | 
 | ||
|  | 	tag: Full Name <email address>  optional-other-stuff | ||
|  | 
 | ||
|  | The tags in common use are: | ||
|  | 
 | ||
|  |  - Signed-off-by: this is a developer's certification that he or she has | ||
|  |    the right to submit the patch for inclusion into the kernel.  It is an | ||
|  |    agreement to the Developer's Certificate of Origin, the full text of | ||
|  |    which can be found in Documentation/SubmittingPatches.  Code without a | ||
|  |    proper signoff cannot be merged into the mainline. | ||
|  | 
 | ||
|  |  - Acked-by: indicates an agreement by another developer (often a | ||
|  |    maintainer of the relevant code) that the patch is appropriate for | ||
|  |    inclusion into the kernel. | ||
|  | 
 | ||
|  |  - Tested-by: states that the named person has tested the patch and found | ||
|  |    it to work. | ||
|  | 
 | ||
|  |  - Reviewed-by: the named developer has reviewed the patch for correctness; | ||
|  |    see the reviewer's statement in Documentation/SubmittingPatches for more | ||
|  |    detail. | ||
|  | 
 | ||
|  |  - Reported-by: names a user who reported a problem which is fixed by this | ||
|  |    patch; this tag is used to give credit to the (often underappreciated) | ||
|  |    people who test our code and let us know when things do not work | ||
|  |    correctly. | ||
|  | 
 | ||
|  |  - Cc: the named person received a copy of the patch and had the | ||
|  |    opportunity to comment on it. | ||
|  | 
 | ||
|  | Be careful in the addition of tags to your patches: only Cc: is appropriate | ||
|  | for addition without the explicit permission of the person named. | ||
|  | 
 | ||
|  | 
 | ||
|  | 5.5: SENDING THE PATCH | ||
|  | 
 | ||
|  | Before you mail your patches, there are a couple of other things you should | ||
|  | take care of: | ||
|  | 
 | ||
|  |  - Are you sure that your mailer will not corrupt the patches?  Patches | ||
|  |    which have had gratuitous white-space changes or line wrapping performed | ||
|  |    by the mail client will not apply at the other end, and often will not | ||
|  |    be examined in any detail.  If there is any doubt at all, mail the patch | ||
|  |    to yourself and convince yourself that it shows up intact.   | ||
|  | 
 | ||
|  |    Documentation/email-clients.txt has some helpful hints on making | ||
|  |    specific mail clients work for sending patches. | ||
|  | 
 | ||
|  |  - Are you sure your patch is free of silly mistakes?  You should always | ||
|  |    run patches through scripts/checkpatch.pl and address the complaints it | ||
|  |    comes up with.  Please bear in mind that checkpatch.pl, while being the | ||
|  |    embodiment of a fair amount of thought about what kernel patches should | ||
|  |    look like, is not smarter than you.  If fixing a checkpatch.pl complaint | ||
|  |    would make the code worse, don't do it. | ||
|  | 
 | ||
|  | Patches should always be sent as plain text.  Please do not send them as | ||
|  | attachments; that makes it much harder for reviewers to quote sections of | ||
|  | the patch in their replies.  Instead, just put the patch directly into your | ||
|  | message. | ||
|  | 
 | ||
|  | When mailing patches, it is important to send copies to anybody who might | ||
|  | be interested in it.  Unlike some other projects, the kernel encourages | ||
|  | people to err on the side of sending too many copies; don't assume that the | ||
|  | relevant people will see your posting on the mailing lists.  In particular, | ||
|  | copies should go to: | ||
|  | 
 | ||
|  |  - The maintainer(s) of the affected subsystem(s).  As described earlier, | ||
|  |    the MAINTAINERS file is the first place to look for these people. | ||
|  | 
 | ||
|  |  - Other developers who have been working in the same area - especially | ||
|  |    those who might be working there now.  Using git to see who else has | ||
|  |    modified the files you are working on can be helpful. | ||
|  | 
 | ||
|  |  - If you are responding to a bug report or a feature request, copy the | ||
|  |    original poster as well. | ||
|  | 
 | ||
|  |  - Send a copy to the relevant mailing list, or, if nothing else applies, | ||
|  |    the linux-kernel list. | ||
|  | 
 | ||
|  |  - If you are fixing a bug, think about whether the fix should go into the | ||
|  |    next stable update.  If so, stable@kernel.org should get a copy of the | ||
|  |    patch.  Also add a "Cc: stable@kernel.org" to the tags within the patch | ||
|  |    itself; that will cause the stable team to get a notification when your | ||
|  |    fix goes into the mainline. | ||
|  | 
 | ||
|  | When selecting recipients for a patch, it is good to have an idea of who | ||
|  | you think will eventually accept the patch and get it merged.  While it | ||
|  | is possible to send patches directly to Linus Torvalds and have him merge | ||
|  | them, things are not normally done that way.  Linus is busy, and there are | ||
|  | subsystem maintainers who watch over specific parts of the kernel.  Usually | ||
|  | you will be wanting that maintainer to merge your patches.  If there is no | ||
|  | obvious maintainer, Andrew Morton is often the patch target of last resort. | ||
|  | 
 | ||
|  | Patches need good subject lines.  The canonical format for a patch line is | ||
|  | something like: | ||
|  | 
 | ||
|  | 	[PATCH nn/mm] subsys: one-line description of the patch | ||
|  | 
 | ||
|  | where "nn" is the ordinal number of the patch, "mm" is the total number of | ||
|  | patches in the series, and "subsys" is the name of the affected subsystem. | ||
|  | Clearly, nn/mm can be omitted for a single, standalone patch.   | ||
|  | 
 | ||
|  | If you have a significant series of patches, it is customary to send an | ||
|  | introductory description as part zero.  This convention is not universally | ||
|  | followed though; if you use it, remember that information in the | ||
|  | introduction does not make it into the kernel changelogs.  So please ensure | ||
|  | that the patches, themselves, have complete changelog information. | ||
|  | 
 | ||
|  | In general, the second and following parts of a multi-part patch should be | ||
|  | sent as a reply to the first part so that they all thread together at the | ||
|  | receiving end.  Tools like git and quilt have commands to mail out a set of | ||
|  | patches with the proper threading.  If you have a long series, though, and | ||
|  | are using git, please provide the --no-chain-reply-to option to avoid | ||
|  | creating exceptionally deep nesting. |