Mauro Carvalho Chehab | f7c9fe4 | 2016-09-19 08:07:36 -0300 | [diff] [blame] | 1 | .. _development_posting: |
| 2 | |
| 3 | Posting patches |
| 4 | =============== |
Jonathan Corbet | 75b0214 | 2008-09-30 15:15:56 -0600 | [diff] [blame] | 5 | |
| 6 | Sooner or later, the time comes when your work is ready to be presented to |
| 7 | the community for review and, eventually, inclusion into the mainline |
| 8 | kernel. Unsurprisingly, the kernel development community has evolved a set |
| 9 | of conventions and procedures which are used in the posting of patches; |
| 10 | following them will make life much easier for everybody involved. This |
| 11 | document will attempt to cover these expectations in reasonable detail; |
Mauro Carvalho Chehab | 8c27ceff3 | 2016-10-18 10:12:27 -0200 | [diff] [blame] | 12 | more information can also be found in the files process/submitting-patches.rst, |
| 13 | process/submitting-drivers.rst, and process/submit-checklist.rst in the kernel documentation |
Jonathan Corbet | 75b0214 | 2008-09-30 15:15:56 -0600 | [diff] [blame] | 14 | directory. |
| 15 | |
| 16 | |
Mauro Carvalho Chehab | f7c9fe4 | 2016-09-19 08:07:36 -0300 | [diff] [blame] | 17 | When to post |
| 18 | ------------ |
Jonathan Corbet | 75b0214 | 2008-09-30 15:15:56 -0600 | [diff] [blame] | 19 | |
| 20 | There is a constant temptation to avoid posting patches before they are |
| 21 | completely "ready." For simple patches, that is not a problem. If the |
| 22 | work being done is complex, though, there is a lot to be gained by getting |
| 23 | feedback from the community before the work is complete. So you should |
| 24 | consider posting in-progress work, or even making a git tree available so |
| 25 | that interested developers can catch up with your work at any time. |
| 26 | |
| 27 | When posting code which is not yet considered ready for inclusion, it is a |
| 28 | good idea to say so in the posting itself. Also mention any major work |
| 29 | which remains to be done and any known problems. Fewer people will look at |
| 30 | patches which are known to be half-baked, but those who do will come in |
| 31 | with the idea that they can help you drive the work in the right direction. |
| 32 | |
| 33 | |
Mauro Carvalho Chehab | f7c9fe4 | 2016-09-19 08:07:36 -0300 | [diff] [blame] | 34 | Before creating patches |
| 35 | ----------------------- |
Jonathan Corbet | 75b0214 | 2008-09-30 15:15:56 -0600 | [diff] [blame] | 36 | |
| 37 | There are a number of things which should be done before you consider |
| 38 | sending patches to the development community. These include: |
| 39 | |
| 40 | - Test the code to the extent that you can. Make use of the kernel's |
| 41 | debugging tools, ensure that the kernel will build with all reasonable |
| 42 | combinations of configuration options, use cross-compilers to build for |
| 43 | different architectures, etc. |
| 44 | |
| 45 | - Make sure your code is compliant with the kernel coding style |
| 46 | guidelines. |
| 47 | |
| 48 | - Does your change have performance implications? If so, you should run |
| 49 | benchmarks showing what the impact (or benefit) of your change is; a |
| 50 | 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 done |
| 53 | for an employer, the employer likely has a right to the work and must be |
| 54 | agreeable with its release under the GPL. |
| 55 | |
| 56 | As a general rule, putting in some extra thought before posting code almost |
| 57 | always pays back the effort in short order. |
| 58 | |
| 59 | |
Mauro Carvalho Chehab | f7c9fe4 | 2016-09-19 08:07:36 -0300 | [diff] [blame] | 60 | Patch preparation |
| 61 | ----------------- |
Jonathan Corbet | 75b0214 | 2008-09-30 15:15:56 -0600 | [diff] [blame] | 62 | |
| 63 | The preparation of patches for posting can be a surprising amount of work, |
| 64 | but, once again, attempting to save time here is not generally advisable |
| 65 | even in the short term. |
| 66 | |
| 67 | Patches must be prepared against a specific version of the kernel. As a |
| 68 | general rule, a patch should be based on the current mainline as found in |
Jonathan Corbet | 5c050fb | 2011-03-25 12:17:53 -0600 | [diff] [blame] | 69 | Linus's git tree. When basing on mainline, start with a well-known release |
| 70 | point - a stable or -rc release - rather than branching off the mainline at |
| 71 | an arbitrary spot. |
| 72 | |
| 73 | It may become necessary to make versions against -mm, linux-next, or a |
| 74 | subsystem tree, though, to facilitate wider testing and review. Depending |
| 75 | on the area of your patch and what is going on elsewhere, basing a patch |
| 76 | against these other trees can require a significant amount of work |
| 77 | resolving conflicts and dealing with API changes. |
Jonathan Corbet | 75b0214 | 2008-09-30 15:15:56 -0600 | [diff] [blame] | 78 | |
| 79 | Only the most simple changes should be formatted as a single patch; |
| 80 | everything else should be made as a logical series of changes. Splitting |
| 81 | up patches is a bit of an art; some developers spend a long time figuring |
| 82 | out how to do it in the way that the community expects. There are a few |
| 83 | rules of thumb, however, which can help considerably: |
| 84 | |
| 85 | - The patch series you post will almost certainly not be the series of |
| 86 | changes found in your working revision control system. Instead, the |
| 87 | changes you have made need to be considered in their final form, then |
| 88 | split apart in ways which make sense. The developers are interested in |
| 89 | discrete, self-contained changes, not the path you took to get to those |
| 90 | changes. |
| 91 | |
| 92 | - Each logically independent change should be formatted as a separate |
| 93 | patch. These changes can be small ("add a field to this structure") or |
| 94 | large (adding a significant new driver, for example), but they should be |
| 95 | conceptually small and amenable to a one-line description. Each patch |
| 96 | should make a specific change which can be reviewed on its own and |
| 97 | verified to do what it says it does. |
| 98 | |
| 99 | - As a way of restating the guideline above: do not mix different types of |
| 100 | changes in the same patch. If a single patch fixes a critical security |
| 101 | bug, rearranges a few structures, and reformats the code, there is a |
| 102 | good chance that it will be passed over and the important fix will be |
| 103 | lost. |
| 104 | |
| 105 | - Each patch should yield a kernel which builds and runs properly; if your |
| 106 | patch series is interrupted in the middle, the result should still be a |
| 107 | working kernel. Partial application of a patch series is a common |
| 108 | scenario when the "git bisect" tool is used to find regressions; if the |
| 109 | result is a broken kernel, you will make life harder for developers and |
| 110 | users who are engaging in the noble work of tracking down problems. |
| 111 | |
Jonathan Corbet | 5c050fb | 2011-03-25 12:17:53 -0600 | [diff] [blame] | 112 | - Do not overdo it, though. One developer once posted a set of edits |
Jonathan Corbet | 75b0214 | 2008-09-30 15:15:56 -0600 | [diff] [blame] | 113 | to a single file as 500 separate patches - an act which did not make him |
| 114 | the most popular person on the kernel mailing list. A single patch can |
| 115 | be reasonably large as long as it still contains a single *logical* |
Jonathan Corbet | 5c050fb | 2011-03-25 12:17:53 -0600 | [diff] [blame] | 116 | change. |
Jonathan Corbet | 75b0214 | 2008-09-30 15:15:56 -0600 | [diff] [blame] | 117 | |
| 118 | - It can be tempting to add a whole new infrastructure with a series of |
| 119 | patches, but to leave that infrastructure unused until the final patch |
| 120 | in the series enables the whole thing. This temptation should be |
| 121 | avoided if possible; if that series adds regressions, bisection will |
| 122 | finger the last patch as the one which caused the problem, even though |
| 123 | the real bug is elsewhere. Whenever possible, a patch which adds new |
| 124 | code should make that code active immediately. |
| 125 | |
| 126 | Working to create the perfect patch series can be a frustrating process |
| 127 | which takes quite a bit of time and thought after the "real work" has been |
| 128 | done. When done properly, though, it is time well spent. |
| 129 | |
| 130 | |
Mauro Carvalho Chehab | f7c9fe4 | 2016-09-19 08:07:36 -0300 | [diff] [blame] | 131 | Patch formatting and changelogs |
| 132 | ------------------------------- |
Jonathan Corbet | 75b0214 | 2008-09-30 15:15:56 -0600 | [diff] [blame] | 133 | |
| 134 | So now you have a perfect series of patches for posting, but the work is |
| 135 | not done quite yet. Each patch needs to be formatted into a message which |
| 136 | quickly and clearly communicates its purpose to the rest of the world. To |
| 137 | that end, each patch will be composed of the following: |
| 138 | |
| 139 | - An optional "From" line naming the author of the patch. This line is |
| 140 | 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 be |
| 144 | enough for a reader who sees it with no other context to figure out the |
| 145 | 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 relevant |
| 147 | subsystem name first, followed by the purpose of the patch. For |
| 148 | example: |
| 149 | |
Mauro Carvalho Chehab | f7c9fe4 | 2016-09-19 08:07:36 -0300 | [diff] [blame] | 150 | :: |
| 151 | |
Jonathan Corbet | 75b0214 | 2008-09-30 15:15:56 -0600 | [diff] [blame] | 152 | gpio: fix build on CONFIG_GPIO_SYSFS=n |
| 153 | |
| 154 | - A blank line followed by a detailed description of the contents of the |
| 155 | patch. This description can be as long as is required; it should say |
| 156 | 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 from |
| 159 | the author of the patch. Tags will be described in more detail below. |
| 160 | |
Jonathan Corbet | 5d98932 | 2009-04-21 13:33:06 -0600 | [diff] [blame] | 161 | The items above, together, form the changelog for the patch. Writing good |
| 162 | changelogs is a crucial but often-neglected art; it's worth spending |
| 163 | another moment discussing this issue. When writing a changelog, you should |
| 164 | bear in mind that a number of different people will be reading your words. |
| 165 | These include subsystem maintainers and reviewers who need to decide |
| 166 | whether the patch should be included, distributors and other maintainers |
| 167 | trying to decide whether a patch should be backported to other kernels, bug |
| 168 | hunters wondering whether the patch is responsible for a problem they are |
| 169 | chasing, users who want to know how the kernel has changed, and more. A |
| 170 | good changelog conveys the needed information to all of these people in the |
| 171 | most direct and concise way possible. |
| 172 | |
| 173 | To that end, the summary line should describe the effects of and motivation |
| 174 | for the change as well as possible given the one-line constraint. The |
| 175 | detailed description can then amplify on those topics and provide any |
| 176 | needed additional information. If the patch fixes a bug, cite the commit |
Jonathan Corbet | 5c050fb | 2011-03-25 12:17:53 -0600 | [diff] [blame] | 177 | which introduced the bug if possible (and please provide both the commit ID |
| 178 | and the title when citing commits). If a problem is associated with |
Jonathan Corbet | 5d98932 | 2009-04-21 13:33:06 -0600 | [diff] [blame] | 179 | specific log or compiler output, include that output to help others |
| 180 | searching for a solution to the same problem. If the change is meant to |
| 181 | support other changes coming in later patch, say so. If internal APIs are |
| 182 | changed, detail those changes and how other developers should respond. In |
| 183 | general, the more you can put yourself into the shoes of everybody who will |
| 184 | be reading your changelog, the better that changelog (and the kernel as a |
| 185 | whole) will be. |
| 186 | |
| 187 | Needless to say, the changelog should be the text used when committing the |
| 188 | change to a revision control system. It will be followed by: |
Jonathan Corbet | 75b0214 | 2008-09-30 15:15:56 -0600 | [diff] [blame] | 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 the |
| 192 | resulting patch easier for others to read. |
| 193 | |
| 194 | You should avoid including changes to irrelevant files (those generated by |
| 195 | the build process, for example, or editor backup files) in the patch. The |
| 196 | file "dontdiff" in the Documentation directory can help in this regard; |
| 197 | pass it to diff with the "-X" option. |
| 198 | |
| 199 | The tags mentioned above are used to describe how various developers have |
| 200 | been associated with the development of this patch. They are described in |
Mauro Carvalho Chehab | 8c27ceff3 | 2016-10-18 10:12:27 -0200 | [diff] [blame] | 201 | detail in the process/submitting-patches.rst document; what follows here is a brief |
Jonathan Corbet | 75b0214 | 2008-09-30 15:15:56 -0600 | [diff] [blame] | 202 | summary. Each of these lines has the format: |
| 203 | |
Mauro Carvalho Chehab | f7c9fe4 | 2016-09-19 08:07:36 -0300 | [diff] [blame] | 204 | :: |
| 205 | |
Jonathan Corbet | 75b0214 | 2008-09-30 15:15:56 -0600 | [diff] [blame] | 206 | tag: Full Name <email address> optional-other-stuff |
| 207 | |
| 208 | The tags in common use are: |
| 209 | |
| 210 | - Signed-off-by: this is a developer's certification that he or she has |
| 211 | the right to submit the patch for inclusion into the kernel. It is an |
| 212 | agreement to the Developer's Certificate of Origin, the full text of |
Mauro Carvalho Chehab | 8c27ceff3 | 2016-10-18 10:12:27 -0200 | [diff] [blame] | 213 | which can be found in Documentation/process/submitting-patches.rst. Code without a |
Jonathan Corbet | 75b0214 | 2008-09-30 15:15:56 -0600 | [diff] [blame] | 214 | proper signoff cannot be merged into the mainline. |
| 215 | |
| 216 | - Acked-by: indicates an agreement by another developer (often a |
| 217 | maintainer of the relevant code) that the patch is appropriate for |
| 218 | inclusion into the kernel. |
| 219 | |
| 220 | - Tested-by: states that the named person has tested the patch and found |
| 221 | it to work. |
| 222 | |
| 223 | - Reviewed-by: the named developer has reviewed the patch for correctness; |
Mauro Carvalho Chehab | 8c27ceff3 | 2016-10-18 10:12:27 -0200 | [diff] [blame] | 224 | see the reviewer's statement in Documentation/process/submitting-patches.rst for more |
Jonathan Corbet | 75b0214 | 2008-09-30 15:15:56 -0600 | [diff] [blame] | 225 | detail. |
| 226 | |
| 227 | - Reported-by: names a user who reported a problem which is fixed by this |
| 228 | patch; this tag is used to give credit to the (often underappreciated) |
| 229 | people who test our code and let us know when things do not work |
| 230 | correctly. |
| 231 | |
| 232 | - Cc: the named person received a copy of the patch and had the |
| 233 | opportunity to comment on it. |
| 234 | |
| 235 | Be careful in the addition of tags to your patches: only Cc: is appropriate |
| 236 | for addition without the explicit permission of the person named. |
| 237 | |
| 238 | |
Mauro Carvalho Chehab | f7c9fe4 | 2016-09-19 08:07:36 -0300 | [diff] [blame] | 239 | Sending the patch |
| 240 | ----------------- |
Jonathan Corbet | 75b0214 | 2008-09-30 15:15:56 -0600 | [diff] [blame] | 241 | |
| 242 | Before you mail your patches, there are a couple of other things you should |
| 243 | take care of: |
| 244 | |
| 245 | - Are you sure that your mailer will not corrupt the patches? Patches |
| 246 | which have had gratuitous white-space changes or line wrapping performed |
| 247 | by the mail client will not apply at the other end, and often will not |
| 248 | be examined in any detail. If there is any doubt at all, mail the patch |
Jonathan Corbet | 5c050fb | 2011-03-25 12:17:53 -0600 | [diff] [blame] | 249 | to yourself and convince yourself that it shows up intact. |
Jonathan Corbet | 75b0214 | 2008-09-30 15:15:56 -0600 | [diff] [blame] | 250 | |
Mauro Carvalho Chehab | 8c27ceff3 | 2016-10-18 10:12:27 -0200 | [diff] [blame] | 251 | Documentation/process/email-clients.rst has some helpful hints on making |
Jonathan Corbet | 75b0214 | 2008-09-30 15:15:56 -0600 | [diff] [blame] | 252 | specific mail clients work for sending patches. |
| 253 | |
| 254 | - Are you sure your patch is free of silly mistakes? You should always |
| 255 | run patches through scripts/checkpatch.pl and address the complaints it |
| 256 | comes up with. Please bear in mind that checkpatch.pl, while being the |
| 257 | embodiment of a fair amount of thought about what kernel patches should |
| 258 | look like, is not smarter than you. If fixing a checkpatch.pl complaint |
| 259 | would make the code worse, don't do it. |
| 260 | |
| 261 | Patches should always be sent as plain text. Please do not send them as |
| 262 | attachments; that makes it much harder for reviewers to quote sections of |
| 263 | the patch in their replies. Instead, just put the patch directly into your |
| 264 | message. |
| 265 | |
| 266 | When mailing patches, it is important to send copies to anybody who might |
| 267 | be interested in it. Unlike some other projects, the kernel encourages |
| 268 | people to err on the side of sending too many copies; don't assume that the |
| 269 | relevant people will see your posting on the mailing lists. In particular, |
| 270 | copies should go to: |
| 271 | |
| 272 | - The maintainer(s) of the affected subsystem(s). As described earlier, |
| 273 | the MAINTAINERS file is the first place to look for these people. |
| 274 | |
| 275 | - Other developers who have been working in the same area - especially |
| 276 | those who might be working there now. Using git to see who else has |
| 277 | modified the files you are working on can be helpful. |
| 278 | |
| 279 | - If you are responding to a bug report or a feature request, copy the |
| 280 | original poster as well. |
| 281 | |
| 282 | - Send a copy to the relevant mailing list, or, if nothing else applies, |
| 283 | the linux-kernel list. |
| 284 | |
| 285 | - If you are fixing a bug, think about whether the fix should go into the |
Joe Perches | 2eb7f20 | 2011-12-09 14:12:00 -0800 | [diff] [blame] | 286 | next stable update. If so, stable@vger.kernel.org should get a copy of |
| 287 | the patch. Also add a "Cc: stable@vger.kernel.org" to the tags within |
| 288 | the patch itself; that will cause the stable team to get a notification |
| 289 | when your fix goes into the mainline. |
Jonathan Corbet | 75b0214 | 2008-09-30 15:15:56 -0600 | [diff] [blame] | 290 | |
| 291 | When selecting recipients for a patch, it is good to have an idea of who |
| 292 | you think will eventually accept the patch and get it merged. While it |
| 293 | is possible to send patches directly to Linus Torvalds and have him merge |
| 294 | them, things are not normally done that way. Linus is busy, and there are |
| 295 | subsystem maintainers who watch over specific parts of the kernel. Usually |
| 296 | you will be wanting that maintainer to merge your patches. If there is no |
| 297 | obvious maintainer, Andrew Morton is often the patch target of last resort. |
| 298 | |
| 299 | Patches need good subject lines. The canonical format for a patch line is |
| 300 | something like: |
| 301 | |
Mauro Carvalho Chehab | f7c9fe4 | 2016-09-19 08:07:36 -0300 | [diff] [blame] | 302 | :: |
| 303 | |
Jonathan Corbet | 75b0214 | 2008-09-30 15:15:56 -0600 | [diff] [blame] | 304 | [PATCH nn/mm] subsys: one-line description of the patch |
| 305 | |
| 306 | where "nn" is the ordinal number of the patch, "mm" is the total number of |
| 307 | patches in the series, and "subsys" is the name of the affected subsystem. |
Jonathan Corbet | 5c050fb | 2011-03-25 12:17:53 -0600 | [diff] [blame] | 308 | Clearly, nn/mm can be omitted for a single, standalone patch. |
Jonathan Corbet | 75b0214 | 2008-09-30 15:15:56 -0600 | [diff] [blame] | 309 | |
| 310 | If you have a significant series of patches, it is customary to send an |
| 311 | introductory description as part zero. This convention is not universally |
| 312 | followed though; if you use it, remember that information in the |
| 313 | introduction does not make it into the kernel changelogs. So please ensure |
| 314 | that the patches, themselves, have complete changelog information. |
| 315 | |
| 316 | In general, the second and following parts of a multi-part patch should be |
| 317 | sent as a reply to the first part so that they all thread together at the |
| 318 | receiving end. Tools like git and quilt have commands to mail out a set of |
| 319 | patches with the proper threading. If you have a long series, though, and |
Jonathan Corbet | 5c050fb | 2011-03-25 12:17:53 -0600 | [diff] [blame] | 320 | are using git, please stay away from the --chain-reply-to option to avoid |
Jonathan Corbet | 75b0214 | 2008-09-30 15:15:56 -0600 | [diff] [blame] | 321 | creating exceptionally deep nesting. |