brintos

brintos / linux-shallow public Read only

0
0
Text · 17.4 KiB · de4edd4 Raw
361 lines · plain
1.. _development_posting:2 3Posting patches4===============5 6Sooner or later, the time comes when your work is ready to be presented to7the community for review and, eventually, inclusion into the mainline8kernel.  Unsurprisingly, the kernel development community has evolved a set9of conventions and procedures which are used in the posting of patches;10following them will make life much easier for everybody involved.  This11document will attempt to cover these expectations in reasonable detail;12more information can also be found in the files13:ref:`Documentation/process/submitting-patches.rst <submittingpatches>`14and :ref:`Documentation/process/submit-checklist.rst <submitchecklist>`.15 16 17When to post18------------19 20There is a constant temptation to avoid posting patches before they are21completely "ready."  For simple patches, that is not a problem.  If the22work being done is complex, though, there is a lot to be gained by getting23feedback from the community before the work is complete.  So you should24consider posting in-progress work, or even making a git tree available so25that interested developers can catch up with your work at any time.26 27When posting code which is not yet considered ready for inclusion, it is a28good idea to say so in the posting itself.  Also mention any major work29which remains to be done and any known problems.  Fewer people will look at30patches which are known to be half-baked, but those who do will come in31with the idea that they can help you drive the work in the right direction.32 33 34Before creating patches35-----------------------36 37There are a number of things which should be done before you consider38sending patches to the development community.  These include:39 40 - Test the code to the extent that you can.  Make use of the kernel's41   debugging tools, ensure that the kernel will build with all reasonable42   combinations of configuration options, use cross-compilers to build for43   different architectures, etc.44 45 - Make sure your code is compliant with the kernel coding style46   guidelines.47 48 - Does your change have performance implications?  If so, you should run49   benchmarks showing what the impact (or benefit) of your change is; a50   summary of the results should be included with the patch.51 52 - Be sure that you have the right to post the code.  If this work was done53   for an employer, the employer likely has a right to the work and must be54   agreeable with its release under the GPL.55 56As a general rule, putting in some extra thought before posting code almost57always pays back the effort in short order.58 59 60Patch preparation61-----------------62 63The preparation of patches for posting can be a surprising amount of work,64but, once again, attempting to save time here is not generally advisable65even in the short term.66 67Patches must be prepared against a specific version of the kernel.  As a68general rule, a patch should be based on the current mainline as found in69Linus's git tree.  When basing on mainline, start with a well-known release70point - a stable or -rc release - rather than branching off the mainline at71an arbitrary spot.72 73It may become necessary to make versions against -mm, linux-next, or a74subsystem tree, though, to facilitate wider testing and review.  Depending75on the area of your patch and what is going on elsewhere, basing a patch76against these other trees can require a significant amount of work77resolving conflicts and dealing with API changes.78 79Only the most simple changes should be formatted as a single patch;80everything else should be made as a logical series of changes.  Splitting81up patches is a bit of an art; some developers spend a long time figuring82out how to do it in the way that the community expects.  There are a few83rules of thumb, however, which can help considerably:84 85 - The patch series you post will almost certainly not be the series of86   changes found in your working revision control system.  Instead, the87   changes you have made need to be considered in their final form, then88   split apart in ways which make sense.  The developers are interested in89   discrete, self-contained changes, not the path you took to get to those90   changes.91 92 - Each logically independent change should be formatted as a separate93   patch.  These changes can be small ("add a field to this structure") or94   large (adding a significant new driver, for example), but they should be95   conceptually small and amenable to a one-line description.  Each patch96   should make a specific change which can be reviewed on its own and97   verified to do what it says it does.98 99 - As a way of restating the guideline above: do not mix different types of100   changes in the same patch.  If a single patch fixes a critical security101   bug, rearranges a few structures, and reformats the code, there is a102   good chance that it will be passed over and the important fix will be103   lost.104 105 - Each patch should yield a kernel which builds and runs properly; if your106   patch series is interrupted in the middle, the result should still be a107   working kernel.  Partial application of a patch series is a common108   scenario when the "git bisect" tool is used to find regressions; if the109   result is a broken kernel, you will make life harder for developers and110   users who are engaging in the noble work of tracking down problems.111 112 - Do not overdo it, though.  One developer once posted a set of edits113   to a single file as 500 separate patches - an act which did not make him114   the most popular person on the kernel mailing list.  A single patch can115   be reasonably large as long as it still contains a single *logical*116   change.117 118 - It can be tempting to add a whole new infrastructure with a series of119   patches, but to leave that infrastructure unused until the final patch120   in the series enables the whole thing.  This temptation should be121   avoided if possible; if that series adds regressions, bisection will122   finger the last patch as the one which caused the problem, even though123   the real bug is elsewhere.  Whenever possible, a patch which adds new124   code should make that code active immediately.125 126Working to create the perfect patch series can be a frustrating process127which takes quite a bit of time and thought after the "real work" has been128done.  When done properly, though, it is time well spent.129 130 131Patch formatting and changelogs132-------------------------------133 134So now you have a perfect series of patches for posting, but the work is135not done quite yet.  Each patch needs to be formatted into a message which136quickly and clearly communicates its purpose to the rest of the world.  To137that end, each patch will be composed of the following:138 139 - An optional "From" line naming the author of the patch.  This line is140   only necessary if you are passing on somebody else's patch via email,141   but it never hurts to add it when in doubt.142 143 - A one-line description of what the patch does.  This message should be144   enough for a reader who sees it with no other context to figure out the145   scope of the patch; it is the line that will show up in the "short form"146   changelogs.  This message is usually formatted with the relevant147   subsystem name first, followed by the purpose of the patch.  For148   example:149 150   ::151 152	gpio: fix build on CONFIG_GPIO_SYSFS=n153 154 - A blank line followed by a detailed description of the contents of the155   patch.  This description can be as long as is required; it should say156   what the patch does and why it should be applied to the kernel.157 158 - One or more tag lines, with, at a minimum, one Signed-off-by: line from159   the author of the patch.  Tags will be described in more detail below.160 161The items above, together, form the changelog for the patch.  Writing good162changelogs is a crucial but often-neglected art; it's worth spending163another moment discussing this issue.  When writing a changelog, you should164bear in mind that a number of different people will be reading your words.165These include subsystem maintainers and reviewers who need to decide166whether the patch should be included, distributors and other maintainers167trying to decide whether a patch should be backported to other kernels, bug168hunters wondering whether the patch is responsible for a problem they are169chasing, users who want to know how the kernel has changed, and more.  A170good changelog conveys the needed information to all of these people in the171most direct and concise way possible.172 173To that end, the summary line should describe the effects of and motivation174for the change as well as possible given the one-line constraint.  The175detailed description can then amplify on those topics and provide any176needed additional information.  If the patch fixes a bug, cite the commit177which introduced the bug if possible (and please provide both the commit ID178and the title when citing commits).  If a problem is associated with179specific log or compiler output, include that output to help others180searching for a solution to the same problem.  If the change is meant to181support other changes coming in later patch, say so.  If internal APIs are182changed, detail those changes and how other developers should respond.  In183general, the more you can put yourself into the shoes of everybody who will184be reading your changelog, the better that changelog (and the kernel as a185whole) will be.186 187Needless to say, the changelog should be the text used when committing the188change to a revision control system.  It will be followed by:189 190 - The patch itself, in the unified ("-u") patch format.  Using the "-p"191   option to diff will associate function names with changes, making the192   resulting patch easier for others to read.193 194You should avoid including changes to irrelevant files (those generated by195the build process, for example, or editor backup files) in the patch.  The196file "dontdiff" in the Documentation directory can help in this regard;197pass it to diff with the "-X" option.198 199The tags already briefly mentioned above are used to provide insights how200the patch came into being. They are described in detail in the201:ref:`Documentation/process/submitting-patches.rst <submittingpatches>`202document; what follows here is a brief summary.203 204One tag is used to refer to earlier commits which introduced problems fixed by205the patch::206 207	Fixes: 1f2e3d4c5b6a ("The first line of the commit specified by the first 12 characters of its SHA-1 ID")208 209Another tag is used for linking web pages with additional backgrounds or210details, for example an earlier discussion which leads to the patch or a211document with a specification implemented by the patch::212 213	Link: https://example.com/somewhere.html  optional-other-stuff214 215Many maintainers when applying a patch also add this tag to link to the216latest public review posting of the patch; often this is automatically done217by tools like b4 or a git hook like the one described in218'Documentation/maintainer/configure-git.rst'.219 220If the URL points to a public bug report being fixed by the patch, use the221"Closes:" tag instead::222 223	Closes: https://example.com/issues/1234  optional-other-stuff224 225Some bug trackers have the ability to close issues automatically when a226commit with such a tag is applied. Some bots monitoring mailing lists can227also track such tags and take certain actions. Private bug trackers and228invalid URLs are forbidden.229 230Another kind of tag is used to document who was involved in the development of231the patch. Each of these uses this format::232 233	tag: Full Name <email address>  optional-other-stuff234 235The tags in common use are:236 237 - Signed-off-by: this is a developer's certification that he or she has238   the right to submit the patch for inclusion into the kernel.  It is an239   agreement to the Developer's Certificate of Origin, the full text of240   which can be found in :ref:`Documentation/process/submitting-patches.rst <submittingpatches>`241   Code without a proper signoff cannot be merged into the mainline.242 243 - Co-developed-by: states that the patch was co-created by several developers;244   it is a used to give attribution to co-authors (in addition to the author245   attributed by the From: tag) when multiple people work on a single patch.246   Every Co-developed-by: must be immediately followed by a Signed-off-by: of247   the associated co-author.  Details and examples can be found in248   :ref:`Documentation/process/submitting-patches.rst <submittingpatches>`.249 250 - Acked-by: indicates an agreement by another developer (often a251   maintainer of the relevant code) that the patch is appropriate for252   inclusion into the kernel.253 254 - Tested-by: states that the named person has tested the patch and found255   it to work.256 257 - Reviewed-by: the named developer has reviewed the patch for correctness;258   see the reviewer's statement in :ref:`Documentation/process/submitting-patches.rst <submittingpatches>`259   for more detail.260 261 - Reported-by: names a user who reported a problem which is fixed by this262   patch; this tag is used to give credit to the (often underappreciated)263   people who test our code and let us know when things do not work264   correctly. Note, this tag should be followed by a Closes: tag pointing to265   the report, unless the report is not available on the web. The Link: tag266   can be used instead of Closes: if the patch fixes a part of the issue(s)267   being reported.268 269 - Cc: the named person received a copy of the patch and had the270   opportunity to comment on it.271 272Be careful in the addition of tags to your patches, as only Cc: is appropriate273for addition without the explicit permission of the person named; using274Reported-by: is fine most of the time as well, but ask for permission if275the bug was reported in private.276 277 278Sending the patch279-----------------280 281Before you mail your patches, there are a couple of other things you should282take care of:283 284 - Are you sure that your mailer will not corrupt the patches?  Patches285   which have had gratuitous white-space changes or line wrapping performed286   by the mail client will not apply at the other end, and often will not287   be examined in any detail.  If there is any doubt at all, mail the patch288   to yourself and convince yourself that it shows up intact.289 290   :ref:`Documentation/process/email-clients.rst <email_clients>` has some291   helpful hints on making specific mail clients work for sending patches.292 293 - Are you sure your patch is free of silly mistakes?  You should always294   run patches through scripts/checkpatch.pl and address the complaints it295   comes up with.  Please bear in mind that checkpatch.pl, while being the296   embodiment of a fair amount of thought about what kernel patches should297   look like, is not smarter than you.  If fixing a checkpatch.pl complaint298   would make the code worse, don't do it.299 300Patches should always be sent as plain text.  Please do not send them as301attachments; that makes it much harder for reviewers to quote sections of302the patch in their replies.  Instead, just put the patch directly into your303message.304 305When mailing patches, it is important to send copies to anybody who might306be interested in it.  Unlike some other projects, the kernel encourages307people to err on the side of sending too many copies; don't assume that the308relevant people will see your posting on the mailing lists.  In particular,309copies should go to:310 311 - The maintainer(s) of the affected subsystem(s).  As described earlier,312   the MAINTAINERS file is the first place to look for these people.313 314 - Other developers who have been working in the same area - especially315   those who might be working there now.  Using git to see who else has316   modified the files you are working on can be helpful.317 318 - If you are responding to a bug report or a feature request, copy the319   original poster as well.320 321 - Send a copy to the relevant mailing list, or, if nothing else applies,322   the linux-kernel list.323 324 - If you are fixing a bug, think about whether the fix should go into the325   next stable update.  If so, stable@vger.kernel.org should get a copy of326   the patch.  Also add a "Cc: stable@vger.kernel.org" to the tags within327   the patch itself; that will cause the stable team to get a notification328   when your fix goes into the mainline.329 330When selecting recipients for a patch, it is good to have an idea of who331you think will eventually accept the patch and get it merged.  While it332is possible to send patches directly to Linus Torvalds and have him merge333them, things are not normally done that way.  Linus is busy, and there are334subsystem maintainers who watch over specific parts of the kernel.  Usually335you will be wanting that maintainer to merge your patches.  If there is no336obvious maintainer, Andrew Morton is often the patch target of last resort.337 338Patches need good subject lines.  The canonical format for a patch line is339something like:340 341::342 343	[PATCH nn/mm] subsys: one-line description of the patch344 345where "nn" is the ordinal number of the patch, "mm" is the total number of346patches in the series, and "subsys" is the name of the affected subsystem.347Clearly, nn/mm can be omitted for a single, standalone patch.348 349If you have a significant series of patches, it is customary to send an350introductory description as part zero.  This convention is not universally351followed though; if you use it, remember that information in the352introduction does not make it into the kernel changelogs.  So please ensure353that the patches, themselves, have complete changelog information.354 355In general, the second and following parts of a multi-part patch should be356sent as a reply to the first part so that they all thread together at the357receiving end.  Tools like git and quilt have commands to mail out a set of358patches with the proper threading.  If you have a long series, though, and359are using git, please stay away from the --chain-reply-to option to avoid360creating exceptionally deep nesting.361