5.Posting 13 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267268269270271272273274275276277278
  1. 5: POSTING PATCHES
  2. Sooner or later, the time comes when your work is ready to be presented to
  3. the community for review and, eventually, inclusion into the mainline
  4. kernel. Unsurprisingly, the kernel development community has evolved a set
  5. of conventions and procedures which are used in the posting of patches;
  6. following them will make life much easier for everybody involved. This
  7. document will attempt to cover these expectations in reasonable detail;
  8. more information can also be found in the files SubmittingPatches,
  9. SubmittingDrivers, and SubmitChecklist in the kernel documentation
  10. directory.
  11. 5.1: WHEN TO POST
  12. There is a constant temptation to avoid posting patches before they are
  13. completely "ready." For simple patches, that is not a problem. If the
  14. work being done is complex, though, there is a lot to be gained by getting
  15. feedback from the community before the work is complete. So you should
  16. consider posting in-progress work, or even making a git tree available so
  17. that interested developers can catch up with your work at any time.
  18. When posting code which is not yet considered ready for inclusion, it is a
  19. good idea to say so in the posting itself. Also mention any major work
  20. which remains to be done and any known problems. Fewer people will look at
  21. patches which are known to be half-baked, but those who do will come in
  22. with the idea that they can help you drive the work in the right direction.
  23. 5.2: BEFORE CREATING PATCHES
  24. There are a number of things which should be done before you consider
  25. sending patches to the development community. These include:
  26. - Test the code to the extent that you can. Make use of the kernel's
  27. debugging tools, ensure that the kernel will build with all reasonable
  28. combinations of configuration options, use cross-compilers to build for
  29. different architectures, etc.
  30. - Make sure your code is compliant with the kernel coding style
  31. guidelines.
  32. - Does your change have performance implications? If so, you should run
  33. benchmarks showing what the impact (or benefit) of your change is; a
  34. summary of the results should be included with the patch.
  35. - Be sure that you have the right to post the code. If this work was done
  36. for an employer, the employer likely has a right to the work and must be
  37. agreeable with its release under the GPL.
  38. As a general rule, putting in some extra thought before posting code almost
  39. always pays back the effort in short order.
  40. 5.3: PATCH PREPARATION
  41. The preparation of patches for posting can be a surprising amount of work,
  42. but, once again, attempting to save time here is not generally advisable
  43. even in the short term.
  44. Patches must be prepared against a specific version of the kernel. As a
  45. general rule, a patch should be based on the current mainline as found in
  46. Linus's git tree. It may become necessary to make versions against -mm,
  47. linux-next, or a subsystem tree, though, to facilitate wider testing and
  48. review. Depending on the area of your patch and what is going on
  49. elsewhere, basing a patch against these other trees can require a
  50. significant amount of work resolving conflicts and dealing with API
  51. changes.
  52. Only the most simple changes should be formatted as a single patch;
  53. everything else should be made as a logical series of changes. Splitting
  54. up patches is a bit of an art; some developers spend a long time figuring
  55. out how to do it in the way that the community expects. There are a few
  56. rules of thumb, however, which can help considerably:
  57. - The patch series you post will almost certainly not be the series of
  58. changes found in your working revision control system. Instead, the
  59. changes you have made need to be considered in their final form, then
  60. split apart in ways which make sense. The developers are interested in
  61. discrete, self-contained changes, not the path you took to get to those
  62. changes.
  63. - Each logically independent change should be formatted as a separate
  64. patch. These changes can be small ("add a field to this structure") or
  65. large (adding a significant new driver, for example), but they should be
  66. conceptually small and amenable to a one-line description. Each patch
  67. should make a specific change which can be reviewed on its own and
  68. verified to do what it says it does.
  69. - As a way of restating the guideline above: do not mix different types of
  70. changes in the same patch. If a single patch fixes a critical security
  71. bug, rearranges a few structures, and reformats the code, there is a
  72. good chance that it will be passed over and the important fix will be
  73. lost.
  74. - Each patch should yield a kernel which builds and runs properly; if your
  75. patch series is interrupted in the middle, the result should still be a
  76. working kernel. Partial application of a patch series is a common
  77. scenario when the "git bisect" tool is used to find regressions; if the
  78. result is a broken kernel, you will make life harder for developers and
  79. users who are engaging in the noble work of tracking down problems.
  80. - Do not overdo it, though. One developer recently posted a set of edits
  81. to a single file as 500 separate patches - an act which did not make him
  82. the most popular person on the kernel mailing list. A single patch can
  83. be reasonably large as long as it still contains a single *logical*
  84. change.
  85. - It can be tempting to add a whole new infrastructure with a series of
  86. patches, but to leave that infrastructure unused until the final patch
  87. in the series enables the whole thing. This temptation should be
  88. avoided if possible; if that series adds regressions, bisection will
  89. finger the last patch as the one which caused the problem, even though
  90. the real bug is elsewhere. Whenever possible, a patch which adds new
  91. code should make that code active immediately.
  92. Working to create the perfect patch series can be a frustrating process
  93. which takes quite a bit of time and thought after the "real work" has been
  94. done. When done properly, though, it is time well spent.
  95. 5.4: PATCH FORMATTING
  96. So now you have a perfect series of patches for posting, but the work is
  97. not done quite yet. Each patch needs to be formatted into a message which
  98. quickly and clearly communicates its purpose to the rest of the world. To
  99. that end, each patch will be composed of the following:
  100. - An optional "From" line naming the author of the patch. This line is
  101. only necessary if you are passing on somebody else's patch via email,
  102. but it never hurts to add it when in doubt.
  103. - A one-line description of what the patch does. This message should be
  104. enough for a reader who sees it with no other context to figure out the
  105. scope of the patch; it is the line that will show up in the "short form"
  106. changelogs. This message is usually formatted with the relevant
  107. subsystem name first, followed by the purpose of the patch. For
  108. example:
  109. gpio: fix build on CONFIG_GPIO_SYSFS=n
  110. - A blank line followed by a detailed description of the contents of the
  111. patch. This description can be as long as is required; it should say
  112. what the patch does and why it should be applied to the kernel.
  113. - One or more tag lines, with, at a minimum, one Signed-off-by: line from
  114. the author of the patch. Tags will be described in more detail below.
  115. The above three items should, normally, be the text used when committing
  116. the change to a revision control system. They are followed by:
  117. - The patch itself, in the unified ("-u") patch format. Using the "-p"
  118. option to diff will associate function names with changes, making the
  119. resulting patch easier for others to read.
  120. You should avoid including changes to irrelevant files (those generated by
  121. the build process, for example, or editor backup files) in the patch. The
  122. file "dontdiff" in the Documentation directory can help in this regard;
  123. pass it to diff with the "-X" option.
  124. The tags mentioned above are used to describe how various developers have
  125. been associated with the development of this patch. They are described in
  126. detail in the SubmittingPatches document; what follows here is a brief
  127. summary. Each of these lines has the format:
  128. tag: Full Name <email address> optional-other-stuff
  129. The tags in common use are:
  130. - Signed-off-by: this is a developer's certification that he or she has
  131. the right to submit the patch for inclusion into the kernel. It is an
  132. agreement to the Developer's Certificate of Origin, the full text of
  133. which can be found in Documentation/SubmittingPatches. Code without a
  134. proper signoff cannot be merged into the mainline.
  135. - Acked-by: indicates an agreement by another developer (often a
  136. maintainer of the relevant code) that the patch is appropriate for
  137. inclusion into the kernel.
  138. - Tested-by: states that the named person has tested the patch and found
  139. it to work.
  140. - Reviewed-by: the named developer has reviewed the patch for correctness;
  141. see the reviewer's statement in Documentation/SubmittingPatches for more
  142. detail.
  143. - Reported-by: names a user who reported a problem which is fixed by this
  144. patch; this tag is used to give credit to the (often underappreciated)
  145. people who test our code and let us know when things do not work
  146. correctly.
  147. - Cc: the named person received a copy of the patch and had the
  148. opportunity to comment on it.
  149. Be careful in the addition of tags to your patches: only Cc: is appropriate
  150. for addition without the explicit permission of the person named.
  151. 5.5: SENDING THE PATCH
  152. Before you mail your patches, there are a couple of other things you should
  153. take care of:
  154. - Are you sure that your mailer will not corrupt the patches? Patches
  155. which have had gratuitous white-space changes or line wrapping performed
  156. by the mail client will not apply at the other end, and often will not
  157. be examined in any detail. If there is any doubt at all, mail the patch
  158. to yourself and convince yourself that it shows up intact.
  159. Documentation/email-clients.txt has some helpful hints on making
  160. specific mail clients work for sending patches.
  161. - Are you sure your patch is free of silly mistakes? You should always
  162. run patches through scripts/checkpatch.pl and address the complaints it
  163. comes up with. Please bear in mind that checkpatch.pl, while being the
  164. embodiment of a fair amount of thought about what kernel patches should
  165. look like, is not smarter than you. If fixing a checkpatch.pl complaint
  166. would make the code worse, don't do it.
  167. Patches should always be sent as plain text. Please do not send them as
  168. attachments; that makes it much harder for reviewers to quote sections of
  169. the patch in their replies. Instead, just put the patch directly into your
  170. message.
  171. When mailing patches, it is important to send copies to anybody who might
  172. be interested in it. Unlike some other projects, the kernel encourages
  173. people to err on the side of sending too many copies; don't assume that the
  174. relevant people will see your posting on the mailing lists. In particular,
  175. copies should go to:
  176. - The maintainer(s) of the affected subsystem(s). As described earlier,
  177. the MAINTAINERS file is the first place to look for these people.
  178. - Other developers who have been working in the same area - especially
  179. those who might be working there now. Using git to see who else has
  180. modified the files you are working on can be helpful.
  181. - If you are responding to a bug report or a feature request, copy the
  182. original poster as well.
  183. - Send a copy to the relevant mailing list, or, if nothing else applies,
  184. the linux-kernel list.
  185. - If you are fixing a bug, think about whether the fix should go into the
  186. next stable update. If so, stable@kernel.org should get a copy of the
  187. patch. Also add a "Cc: stable@kernel.org" to the tags within the patch
  188. itself; that will cause the stable team to get a notification when your
  189. fix goes into the mainline.
  190. When selecting recipients for a patch, it is good to have an idea of who
  191. you think will eventually accept the patch and get it merged. While it
  192. is possible to send patches directly to Linus Torvalds and have him merge
  193. them, things are not normally done that way. Linus is busy, and there are
  194. subsystem maintainers who watch over specific parts of the kernel. Usually
  195. you will be wanting that maintainer to merge your patches. If there is no
  196. obvious maintainer, Andrew Morton is often the patch target of last resort.
  197. Patches need good subject lines. The canonical format for a patch line is
  198. something like:
  199. [PATCH nn/mm] subsys: one-line description of the patch
  200. where "nn" is the ordinal number of the patch, "mm" is the total number of
  201. patches in the series, and "subsys" is the name of the affected subsystem.
  202. Clearly, nn/mm can be omitted for a single, standalone patch.
  203. If you have a significant series of patches, it is customary to send an
  204. introductory description as part zero. This convention is not universally
  205. followed though; if you use it, remember that information in the
  206. introduction does not make it into the kernel changelogs. So please ensure
  207. that the patches, themselves, have complete changelog information.
  208. In general, the second and following parts of a multi-part patch should be
  209. sent as a reply to the first part so that they all thread together at the
  210. receiving end. Tools like git and quilt have commands to mail out a set of
  211. patches with the proper threading. If you have a long series, though, and
  212. are using git, please provide the --no-chain-reply-to option to avoid
  213. creating exceptionally deep nesting.