diff options
author | Jonathan Corbet <corbet@lwn.net> | 2015-01-29 15:08:28 -0500 |
---|---|---|
committer | Jonathan Corbet <corbet@lwn.net> | 2015-01-29 15:11:58 -0500 |
commit | fbd3a466123bf449b25c26e23d4d79cf722a76cc (patch) | |
tree | 4f9a46fad5a70a1aa9bb1ab5a400f609d55cd5e8 | |
parent | 5e857b667cc4b2778a1b4edb95177381d0b36243 (diff) | |
parent | b792ffe464f64c84c48d51e01c0fecabc4b39579 (diff) |
Merge branch 'doc/sp-update' into docs-next
Bring in the big SubmittingPatches thrashup.
Conflicts:
Documentation/SubmittingPatches
-rw-r--r-- | Documentation/SubmittingPatches | 436 |
1 files changed, 216 insertions, 220 deletions
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index 1671ce323a02..447671bd2927 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches | |||
@@ -10,27 +10,49 @@ kernel, the process can sometimes be daunting if you're not familiar | |||
10 | with "the system." This text is a collection of suggestions which | 10 | with "the system." This text is a collection of suggestions which |
11 | can greatly increase the chances of your change being accepted. | 11 | can greatly increase the chances of your change being accepted. |
12 | 12 | ||
13 | Read Documentation/SubmitChecklist for a list of items to check | 13 | This document contains a large number of suggestions in a relatively terse |
14 | before submitting code. If you are submitting a driver, also read | 14 | format. For detailed information on how the kernel development process |
15 | Documentation/SubmittingDrivers. | 15 | works, see Documentation/development-process. Also, read |
16 | Documentation/SubmitChecklist for a list of items to check before | ||
17 | submitting code. If you are submitting a driver, also read | ||
18 | Documentation/SubmittingDrivers; for device tree binding patches, read | ||
19 | Documentation/devicetree/bindings/submitting-patches.txt. | ||
16 | 20 | ||
17 | Many of these steps describe the default behavior of the git version | 21 | Many of these steps describe the default behavior of the git version |
18 | control system; if you use git to prepare your patches, you'll find much | 22 | control system; if you use git to prepare your patches, you'll find much |
19 | of the mechanical work done for you, though you'll still need to prepare | 23 | of the mechanical work done for you, though you'll still need to prepare |
20 | and document a sensible set of patches. | 24 | and document a sensible set of patches. In general, use of git will make |
25 | your life as a kernel developer easier. | ||
21 | 26 | ||
22 | -------------------------------------------- | 27 | -------------------------------------------- |
23 | SECTION 1 - CREATING AND SENDING YOUR CHANGE | 28 | SECTION 1 - CREATING AND SENDING YOUR CHANGE |
24 | -------------------------------------------- | 29 | -------------------------------------------- |
25 | 30 | ||
26 | 31 | ||
32 | 0) Obtain a current source tree | ||
33 | ------------------------------- | ||
34 | |||
35 | If you do not have a repository with the current kernel source handy, use | ||
36 | git to obtain one. You'll want to start with the mainline repository, | ||
37 | which can be grabbed with: | ||
38 | |||
39 | git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git | ||
40 | |||
41 | Note, however, that you may not want to develop against the mainline tree | ||
42 | directly. Most subsystem maintainers run their own trees and want to see | ||
43 | patches prepared against those trees. See the "T:" entry for the subsystem | ||
44 | in the MAINTAINERS file to find that tree, or simply ask the maintainer if | ||
45 | the tree is not listed there. | ||
46 | |||
47 | It is still possible to download kernel releases via tarballs (as described | ||
48 | in the next section), but that is the hard way to do kernel development. | ||
27 | 49 | ||
28 | 1) "diff -up" | 50 | 1) "diff -up" |
29 | ------------ | 51 | ------------ |
30 | 52 | ||
31 | Use "diff -up" or "diff -uprN" to create patches. git generates patches | 53 | If you must generate your patches by hand, use "diff -up" or "diff -uprN" |
32 | in this form by default; if you're using git, you can skip this section | 54 | to create patches. Git generates patches in this form by default; if |
33 | entirely. | 55 | you're using git, you can skip this section entirely. |
34 | 56 | ||
35 | All changes to the Linux kernel occur in the form of patches, as | 57 | All changes to the Linux kernel occur in the form of patches, as |
36 | generated by diff(1). When creating your patch, make sure to create it | 58 | generated by diff(1). When creating your patch, make sure to create it |
@@ -42,7 +64,7 @@ not in any lower subdirectory. | |||
42 | 64 | ||
43 | To create a patch for a single file, it is often sufficient to do: | 65 | To create a patch for a single file, it is often sufficient to do: |
44 | 66 | ||
45 | SRCTREE= linux-2.6 | 67 | SRCTREE= linux |
46 | MYFILE= drivers/net/mydriver.c | 68 | MYFILE= drivers/net/mydriver.c |
47 | 69 | ||
48 | cd $SRCTREE | 70 | cd $SRCTREE |
@@ -55,17 +77,16 @@ To create a patch for multiple files, you should unpack a "vanilla", | |||
55 | or unmodified kernel source tree, and generate a diff against your | 77 | or unmodified kernel source tree, and generate a diff against your |
56 | own source tree. For example: | 78 | own source tree. For example: |
57 | 79 | ||
58 | MYSRC= /devel/linux-2.6 | 80 | MYSRC= /devel/linux |
59 | 81 | ||
60 | tar xvfz linux-2.6.12.tar.gz | 82 | tar xvfz linux-3.19.tar.gz |
61 | mv linux-2.6.12 linux-2.6.12-vanilla | 83 | mv linux-3.19 linux-3.19-vanilla |
62 | diff -uprN -X linux-2.6.12-vanilla/Documentation/dontdiff \ | 84 | diff -uprN -X linux-3.19-vanilla/Documentation/dontdiff \ |
63 | linux-2.6.12-vanilla $MYSRC > /tmp/patch | 85 | linux-3.19-vanilla $MYSRC > /tmp/patch |
64 | 86 | ||
65 | "dontdiff" is a list of files which are generated by the kernel during | 87 | "dontdiff" is a list of files which are generated by the kernel during |
66 | the build process, and should be ignored in any diff(1)-generated | 88 | the build process, and should be ignored in any diff(1)-generated |
67 | patch. The "dontdiff" file is included in the kernel tree in | 89 | patch. |
68 | 2.6.12 and later. | ||
69 | 90 | ||
70 | Make sure your patch does not include any extra files which do not | 91 | Make sure your patch does not include any extra files which do not |
71 | belong in a patch submission. Make sure to review your patch -after- | 92 | belong in a patch submission. Make sure to review your patch -after- |
@@ -83,6 +104,7 @@ is another popular alternative. | |||
83 | 104 | ||
84 | 105 | ||
85 | 2) Describe your changes. | 106 | 2) Describe your changes. |
107 | ------------------------- | ||
86 | 108 | ||
87 | Describe your problem. Whether your patch is a one-line bug fix or | 109 | Describe your problem. Whether your patch is a one-line bug fix or |
88 | 5000 lines of a new feature, there must be an underlying problem that | 110 | 5000 lines of a new feature, there must be an underlying problem that |
@@ -124,10 +146,10 @@ See #3, next. | |||
124 | When you submit or resubmit a patch or patch series, include the | 146 | When you submit or resubmit a patch or patch series, include the |
125 | complete patch description and justification for it. Don't just | 147 | complete patch description and justification for it. Don't just |
126 | say that this is version N of the patch (series). Don't expect the | 148 | say that this is version N of the patch (series). Don't expect the |
127 | patch merger to refer back to earlier patch versions or referenced | 149 | subsystem maintainer to refer back to earlier patch versions or referenced |
128 | URLs to find the patch description and put that into the patch. | 150 | URLs to find the patch description and put that into the patch. |
129 | I.e., the patch (series) and its description should be self-contained. | 151 | I.e., the patch (series) and its description should be self-contained. |
130 | This benefits both the patch merger(s) and reviewers. Some reviewers | 152 | This benefits both the maintainers and reviewers. Some reviewers |
131 | probably didn't even receive earlier versions of the patch. | 153 | probably didn't even receive earlier versions of the patch. |
132 | 154 | ||
133 | Describe your changes in imperative mood, e.g. "make xyzzy do frotz" | 155 | Describe your changes in imperative mood, e.g. "make xyzzy do frotz" |
@@ -156,10 +178,15 @@ Example: | |||
156 | platform_set_drvdata(), but left the variable "dev" unused, | 178 | platform_set_drvdata(), but left the variable "dev" unused, |
157 | delete it. | 179 | delete it. |
158 | 180 | ||
181 | You should also be sure to use at least the first twelve characters of the | ||
182 | SHA-1 ID. The kernel repository holds a *lot* of objects, making | ||
183 | collisions with shorter IDs a real possibility. Bear in mind that, even if | ||
184 | there is no collision with your six-character ID now, that condition may | ||
185 | change five years from now. | ||
186 | |||
159 | If your patch fixes a bug in a specific commit, e.g. you found an issue using | 187 | If your patch fixes a bug in a specific commit, e.g. you found an issue using |
160 | git-bisect, please use the 'Fixes:' tag with the first 12 characters of the | 188 | git-bisect, please use the 'Fixes:' tag with the first 12 characters of the |
161 | SHA-1 ID, and the one line summary. | 189 | SHA-1 ID, and the one line summary. For example: |
162 | Example: | ||
163 | 190 | ||
164 | Fixes: e21d2170f366 ("video: remove unnecessary platform_set_drvdata()") | 191 | Fixes: e21d2170f366 ("video: remove unnecessary platform_set_drvdata()") |
165 | 192 | ||
@@ -172,8 +199,9 @@ outputting the above style in the git log or git show commands | |||
172 | fixes = Fixes: %h (\"%s\") | 199 | fixes = Fixes: %h (\"%s\") |
173 | 200 | ||
174 | 3) Separate your changes. | 201 | 3) Separate your changes. |
202 | ------------------------- | ||
175 | 203 | ||
176 | Separate _logical changes_ into a single patch file. | 204 | Separate each _logical change_ into a separate patch. |
177 | 205 | ||
178 | For example, if your changes include both bug fixes and performance | 206 | For example, if your changes include both bug fixes and performance |
179 | enhancements for a single driver, separate those changes into two | 207 | enhancements for a single driver, separate those changes into two |
@@ -184,90 +212,116 @@ On the other hand, if you make a single change to numerous files, | |||
184 | group those changes into a single patch. Thus a single logical change | 212 | group those changes into a single patch. Thus a single logical change |
185 | is contained within a single patch. | 213 | is contained within a single patch. |
186 | 214 | ||
215 | The point to remember is that each patch should make an easily understood | ||
216 | change that can be verified by reviewers. Each patch should be justifiable | ||
217 | on its own merits. | ||
218 | |||
187 | If one patch depends on another patch in order for a change to be | 219 | If one patch depends on another patch in order for a change to be |
188 | complete, that is OK. Simply note "this patch depends on patch X" | 220 | complete, that is OK. Simply note "this patch depends on patch X" |
189 | in your patch description. | 221 | in your patch description. |
190 | 222 | ||
223 | When dividing your change into a series of patches, take special care to | ||
224 | ensure that the kernel builds and runs properly after each patch in the | ||
225 | series. Developers using "git bisect" to track down a problem can end up | ||
226 | splitting your patch series at any point; they will not thank you if you | ||
227 | introduce bugs in the middle. | ||
228 | |||
191 | If you cannot condense your patch set into a smaller set of patches, | 229 | If you cannot condense your patch set into a smaller set of patches, |
192 | then only post say 15 or so at a time and wait for review and integration. | 230 | then only post say 15 or so at a time and wait for review and integration. |
193 | 231 | ||
194 | 232 | ||
195 | 233 | ||
196 | 4) Style check your changes. | 234 | 4) Style-check your changes. |
235 | ---------------------------- | ||
197 | 236 | ||
198 | Check your patch for basic style violations, details of which can be | 237 | Check your patch for basic style violations, details of which can be |
199 | found in Documentation/CodingStyle. Failure to do so simply wastes | 238 | found in Documentation/CodingStyle. Failure to do so simply wastes |
200 | the reviewers time and will get your patch rejected, probably | 239 | the reviewers time and will get your patch rejected, probably |
201 | without even being read. | 240 | without even being read. |
202 | 241 | ||
203 | At a minimum you should check your patches with the patch style | 242 | One significant exception is when moving code from one file to |
204 | checker prior to submission (scripts/checkpatch.pl). You should | 243 | another -- in this case you should not modify the moved code at all in |
205 | be able to justify all violations that remain in your patch. | 244 | the same patch which moves it. This clearly delineates the act of |
245 | moving the code and your changes. This greatly aids review of the | ||
246 | actual differences and allows tools to better track the history of | ||
247 | the code itself. | ||
248 | |||
249 | Check your patches with the patch style checker prior to submission | ||
250 | (scripts/checkpatch.pl). Note, though, that the style checker should be | ||
251 | viewed as a guide, not as a replacement for human judgment. If your code | ||
252 | looks better with a violation then its probably best left alone. | ||
253 | |||
254 | The checker reports at three levels: | ||
255 | - ERROR: things that are very likely to be wrong | ||
256 | - WARNING: things requiring careful review | ||
257 | - CHECK: things requiring thought | ||
206 | 258 | ||
259 | You should be able to justify all violations that remain in your | ||
260 | patch. | ||
207 | 261 | ||
208 | 262 | ||
209 | 5) Select e-mail destination. | 263 | 5) Select the recipients for your patch. |
264 | ---------------------------------------- | ||
210 | 265 | ||
211 | Look through the MAINTAINERS file and the source code, and determine | 266 | You should always copy the appropriate subsystem maintainer(s) on any patch |
212 | if your change applies to a specific subsystem of the kernel, with | 267 | to code that they maintain; look through the MAINTAINERS file and the |
213 | an assigned maintainer. If so, e-mail that person. The script | 268 | source code revision history to see who those maintainers are. The |
214 | scripts/get_maintainer.pl can be very useful at this step. | 269 | script scripts/get_maintainer.pl can be very useful at this step. If you |
270 | cannot find a maintainer for the subsystem your are working on, Andrew | ||
271 | Morton (akpm@linux-foundation.org) serves as a maintainer of last resort. | ||
215 | 272 | ||
216 | If no maintainer is listed, or the maintainer does not respond, send | 273 | You should also normally choose at least one mailing list to receive a copy |
217 | your patch to the primary Linux kernel developer's mailing list, | 274 | of your patch set. linux-kernel@vger.kernel.org functions as a list of |
218 | linux-kernel@vger.kernel.org. Most kernel developers monitor this | 275 | last resort, but the volume on that list has caused a number of developers |
219 | e-mail list, and can comment on your changes. | 276 | to tune it out. Look in the MAINTAINERS file for a subsystem-specific |
277 | list; your patch will probably get more attention there. Please do not | ||
278 | spam unrelated lists, though. | ||
220 | 279 | ||
280 | Many kernel-related lists are hosted on vger.kernel.org; you can find a | ||
281 | list of them at http://vger.kernel.org/vger-lists.html. There are | ||
282 | kernel-related lists hosted elsewhere as well, though. | ||
221 | 283 | ||
222 | Do not send more than 15 patches at once to the vger mailing lists!!! | 284 | Do not send more than 15 patches at once to the vger mailing lists!!! |
223 | 285 | ||
224 | |||
225 | Linus Torvalds is the final arbiter of all changes accepted into the | 286 | Linus Torvalds is the final arbiter of all changes accepted into the |
226 | Linux kernel. His e-mail address is <torvalds@linux-foundation.org>. | 287 | Linux kernel. His e-mail address is <torvalds@linux-foundation.org>. |
227 | He gets a lot of e-mail, so typically you should do your best to -avoid- | 288 | He gets a lot of e-mail, and, at this point, very few patches go through |
289 | Linus directly, so typically you should do your best to -avoid- | ||
228 | sending him e-mail. | 290 | sending him e-mail. |
229 | 291 | ||
230 | Patches which are bug fixes, are "obvious" changes, or similarly | 292 | If you have a patch that fixes an exploitable security bug, send that patch |
231 | require little discussion should be sent or CC'd to Linus. Patches | 293 | to security@kernel.org. For severe bugs, a short embargo may be considered |
232 | which require discussion or do not have a clear advantage should | 294 | to allow distrbutors to get the patch out to users; in such cases, |
233 | usually be sent first to linux-kernel. Only after the patch is | 295 | obviously, the patch should not be sent to any public lists. |
234 | discussed should the patch then be submitted to Linus. | ||
235 | |||
236 | |||
237 | 296 | ||
238 | 6) Select your CC (e-mail carbon copy) list. | 297 | Patches that fix a severe bug in a released kernel should be directed |
298 | toward the stable maintainers by putting a line like this: | ||
239 | 299 | ||
240 | Unless you have a reason NOT to do so, CC linux-kernel@vger.kernel.org. | 300 | Cc: stable@vger.kernel.org |
241 | 301 | ||
242 | Other kernel developers besides Linus need to be aware of your change, | 302 | into your patch. |
243 | so that they may comment on it and offer code review and suggestions. | ||
244 | linux-kernel is the primary Linux kernel developer mailing list. | ||
245 | Other mailing lists are available for specific subsystems, such as | ||
246 | USB, framebuffer devices, the VFS, the SCSI subsystem, etc. See the | ||
247 | MAINTAINERS file for a mailing list that relates specifically to | ||
248 | your change. | ||
249 | 303 | ||
250 | Majordomo lists of VGER.KERNEL.ORG at: | 304 | Note, however, that some subsystem maintainers want to come to their own |
251 | <http://vger.kernel.org/vger-lists.html> | 305 | conclusions on which patches should go to the stable trees. The networking |
306 | maintainer, in particular, would rather not see individual developers | ||
307 | adding lines like the above to their patches. | ||
252 | 308 | ||
253 | If changes affect userland-kernel interfaces, please send | 309 | If changes affect userland-kernel interfaces, please send the MAN-PAGES |
254 | the MAN-PAGES maintainer (as listed in the MAINTAINERS file) | 310 | maintainer (as listed in the MAINTAINERS file) a man-pages patch, or at |
255 | a man-pages patch, or at least a notification of the change, | 311 | least a notification of the change, so that some information makes its way |
256 | so that some information makes its way into the manual pages. | 312 | into the manual pages. User-space API changes should also be copied to |
257 | 313 | linux-api@vger.kernel.org. | |
258 | Even if the maintainer did not respond in step #5, make sure to ALWAYS | ||
259 | copy the maintainer when you change their code. | ||
260 | 314 | ||
261 | For small patches you may want to CC the Trivial Patch Monkey | 315 | For small patches you may want to CC the Trivial Patch Monkey |
262 | trivial@kernel.org which collects "trivial" patches. Have a look | 316 | trivial@kernel.org which collects "trivial" patches. Have a look |
263 | into the MAINTAINERS file for its current manager. | 317 | into the MAINTAINERS file for its current manager. |
264 | Trivial patches must qualify for one of the following rules: | 318 | Trivial patches must qualify for one of the following rules: |
265 | Spelling fixes in documentation | 319 | Spelling fixes in documentation |
266 | Spelling fixes which could break grep(1) | 320 | Spelling fixes for errors which could break grep(1) |
267 | Warning fixes (cluttering with useless warnings is bad) | 321 | Warning fixes (cluttering with useless warnings is bad) |
268 | Compilation fixes (only if they are actually correct) | 322 | Compilation fixes (only if they are actually correct) |
269 | Runtime fixes (only if they actually fix things) | 323 | Runtime fixes (only if they actually fix things) |
270 | Removing use of deprecated functions/macros (eg. check_region) | 324 | Removing use of deprecated functions/macros |
271 | Contact detail and documentation fixes | 325 | Contact detail and documentation fixes |
272 | Non-portable code replaced by portable code (even in arch-specific, | 326 | Non-portable code replaced by portable code (even in arch-specific, |
273 | since people copy, as long as it's trivial) | 327 | since people copy, as long as it's trivial) |
@@ -276,7 +330,8 @@ Trivial patches must qualify for one of the following rules: | |||
276 | 330 | ||
277 | 331 | ||
278 | 332 | ||
279 | 7) No MIME, no links, no compression, no attachments. Just plain text. | 333 | 6) No MIME, no links, no compression, no attachments. Just plain text. |
334 | ----------------------------------------------------------------------- | ||
280 | 335 | ||
281 | Linus and other kernel developers need to be able to read and comment | 336 | Linus and other kernel developers need to be able to read and comment |
282 | on the changes you are submitting. It is important for a kernel | 337 | on the changes you are submitting. It is important for a kernel |
@@ -299,54 +354,48 @@ you to re-send them using MIME. | |||
299 | See Documentation/email-clients.txt for hints about configuring | 354 | See Documentation/email-clients.txt for hints about configuring |
300 | your e-mail client so that it sends your patches untouched. | 355 | your e-mail client so that it sends your patches untouched. |
301 | 356 | ||
302 | 8) E-mail size. | 357 | 7) E-mail size. |
303 | 358 | --------------- | |
304 | When sending patches to Linus, always follow step #7. | ||
305 | 359 | ||
306 | Large changes are not appropriate for mailing lists, and some | 360 | Large changes are not appropriate for mailing lists, and some |
307 | maintainers. If your patch, uncompressed, exceeds 300 kB in size, | 361 | maintainers. If your patch, uncompressed, exceeds 300 kB in size, |
308 | it is preferred that you store your patch on an Internet-accessible | 362 | it is preferred that you store your patch on an Internet-accessible |
309 | server, and provide instead a URL (link) pointing to your patch. | 363 | server, and provide instead a URL (link) pointing to your patch. But note |
364 | that if your patch exceeds 300 kB, it almost certainly needs to be broken up | ||
365 | anyway. | ||
310 | 366 | ||
367 | 8) Respond to review comments. | ||
368 | ------------------------------ | ||
311 | 369 | ||
370 | Your patch will almost certainly get comments from reviewers on ways in | ||
371 | which the patch can be improved. You must respond to those comments; | ||
372 | ignoring reviewers is a good way to get ignored in return. Review comments | ||
373 | or questions that do not lead to a code change should almost certainly | ||
374 | bring about a comment or changelog entry so that the next reviewer better | ||
375 | understands what is going on. | ||
312 | 376 | ||
313 | 9) Name your kernel version. | 377 | Be sure to tell the reviewers what changes you are making and to thank them |
378 | for their time. Code review is a tiring and time-consuming process, and | ||
379 | reviewers sometimes get grumpy. Even in that case, though, respond | ||
380 | politely and address the problems they have pointed out. | ||
314 | 381 | ||
315 | It is important to note, either in the subject line or in the patch | ||
316 | description, the kernel version to which this patch applies. | ||
317 | 382 | ||
318 | If the patch does not apply cleanly to the latest kernel version, | 383 | 9) Don't get discouraged - or impatient. |
319 | Linus will not apply it. | 384 | ---------------------------------------- |
320 | 385 | ||
386 | After you have submitted your change, be patient and wait. Reviewers are | ||
387 | busy people and may not get to your patch right away. | ||
321 | 388 | ||
389 | Once upon a time, patches used to disappear into the void without comment, | ||
390 | but the development process works more smoothly than that now. You should | ||
391 | receive comments within a week or so; if that does not happen, make sure | ||
392 | that you have sent your patches to the right place. Wait for a minimum of | ||
393 | one week before resubmitting or pinging reviewers - possibly longer during | ||
394 | busy times like merge windows. | ||
322 | 395 | ||
323 | 10) Don't get discouraged. Re-submit. | ||
324 | 396 | ||
325 | After you have submitted your change, be patient and wait. If Linus | 397 | 10) Include PATCH in the subject |
326 | likes your change and applies it, it will appear in the next version | 398 | -------------------------------- |
327 | of the kernel that he releases. | ||
328 | |||
329 | However, if your change doesn't appear in the next version of the | ||
330 | kernel, there could be any number of reasons. It's YOUR job to | ||
331 | narrow down those reasons, correct what was wrong, and submit your | ||
332 | updated change. | ||
333 | |||
334 | It is quite common for Linus to "drop" your patch without comment. | ||
335 | That's the nature of the system. If he drops your patch, it could be | ||
336 | due to | ||
337 | * Your patch did not apply cleanly to the latest kernel version. | ||
338 | * Your patch was not sufficiently discussed on linux-kernel. | ||
339 | * A style issue (see section 2). | ||
340 | * An e-mail formatting issue (re-read this section). | ||
341 | * A technical problem with your change. | ||
342 | * He gets tons of e-mail, and yours got lost in the shuffle. | ||
343 | * You are being annoying. | ||
344 | |||
345 | When in doubt, solicit comments on linux-kernel mailing list. | ||
346 | |||
347 | |||
348 | |||
349 | 11) Include PATCH in the subject | ||
350 | 399 | ||
351 | Due to high e-mail traffic to Linus, and to linux-kernel, it is common | 400 | Due to high e-mail traffic to Linus, and to linux-kernel, it is common |
352 | convention to prefix your subject line with [PATCH]. This lets Linus | 401 | convention to prefix your subject line with [PATCH]. This lets Linus |
@@ -355,7 +404,8 @@ e-mail discussions. | |||
355 | 404 | ||
356 | 405 | ||
357 | 406 | ||
358 | 12) Sign your work | 407 | 11) Sign your work |
408 | ------------------ | ||
359 | 409 | ||
360 | To improve tracking of who did what, especially with patches that can | 410 | To improve tracking of who did what, especially with patches that can |
361 | percolate to their final resting place in the kernel through several | 411 | percolate to their final resting place in the kernel through several |
@@ -429,15 +479,15 @@ which appears in the changelog. | |||
429 | Special note to back-porters: It seems to be a common and useful practice | 479 | Special note to back-porters: It seems to be a common and useful practice |
430 | to insert an indication of the origin of a patch at the top of the commit | 480 | to insert an indication of the origin of a patch at the top of the commit |
431 | message (just after the subject line) to facilitate tracking. For instance, | 481 | message (just after the subject line) to facilitate tracking. For instance, |
432 | here's what we see in 2.6-stable : | 482 | here's what we see in a 3.x-stable release: |
433 | 483 | ||
434 | Date: Tue May 13 19:10:30 2008 +0000 | 484 | Date: Tue Oct 7 07:26:38 2014 -0400 |
435 | 485 | ||
436 | SCSI: libiscsi regression in 2.6.25: fix nop timer handling | 486 | libata: Un-break ATA blacklist |
437 | 487 | ||
438 | commit 4cf1043593db6a337f10e006c23c69e5fc93e722 upstream | 488 | commit 1c40279960bcd7d52dbdf1d466b20d24b99176c8 upstream. |
439 | 489 | ||
440 | And here's what appears in 2.4 : | 490 | And here's what might appear in an older kernel once a patch is backported: |
441 | 491 | ||
442 | Date: Tue May 13 22:12:27 2008 +0200 | 492 | Date: Tue May 13 22:12:27 2008 +0200 |
443 | 493 | ||
@@ -446,18 +496,19 @@ And here's what appears in 2.4 : | |||
446 | [backport of 2.6 commit b7acbdfbd1f277c1eb23f344f899cfa4cd0bf36a] | 496 | [backport of 2.6 commit b7acbdfbd1f277c1eb23f344f899cfa4cd0bf36a] |
447 | 497 | ||
448 | Whatever the format, this information provides a valuable help to people | 498 | Whatever the format, this information provides a valuable help to people |
449 | tracking your trees, and to people trying to trouble-shoot bugs in your | 499 | tracking your trees, and to people trying to troubleshoot bugs in your |
450 | tree. | 500 | tree. |
451 | 501 | ||
452 | 502 | ||
453 | 13) When to use Acked-by: and Cc: | 503 | 12) When to use Acked-by: and Cc: |
504 | --------------------------------- | ||
454 | 505 | ||
455 | The Signed-off-by: tag indicates that the signer was involved in the | 506 | The Signed-off-by: tag indicates that the signer was involved in the |
456 | development of the patch, or that he/she was in the patch's delivery path. | 507 | development of the patch, or that he/she was in the patch's delivery path. |
457 | 508 | ||
458 | If a person was not directly involved in the preparation or handling of a | 509 | If a person was not directly involved in the preparation or handling of a |
459 | patch but wishes to signify and record their approval of it then they can | 510 | patch but wishes to signify and record their approval of it then they can |
460 | arrange to have an Acked-by: line added to the patch's changelog. | 511 | ask to have an Acked-by: line added to the patch's changelog. |
461 | 512 | ||
462 | Acked-by: is often used by the maintainer of the affected code when that | 513 | Acked-by: is often used by the maintainer of the affected code when that |
463 | maintainer neither contributed to nor forwarded the patch. | 514 | maintainer neither contributed to nor forwarded the patch. |
@@ -465,7 +516,8 @@ maintainer neither contributed to nor forwarded the patch. | |||
465 | Acked-by: is not as formal as Signed-off-by:. It is a record that the acker | 516 | Acked-by: is not as formal as Signed-off-by:. It is a record that the acker |
466 | has at least reviewed the patch and has indicated acceptance. Hence patch | 517 | has at least reviewed the patch and has indicated acceptance. Hence patch |
467 | mergers will sometimes manually convert an acker's "yep, looks good to me" | 518 | mergers will sometimes manually convert an acker's "yep, looks good to me" |
468 | into an Acked-by:. | 519 | into an Acked-by: (but note that it is usually better to ask for an |
520 | explicit ack). | ||
469 | 521 | ||
470 | Acked-by: does not necessarily indicate acknowledgement of the entire patch. | 522 | Acked-by: does not necessarily indicate acknowledgement of the entire patch. |
471 | For example, if a patch affects multiple subsystems and has an Acked-by: from | 523 | For example, if a patch affects multiple subsystems and has an Acked-by: from |
@@ -477,11 +529,13 @@ list archives. | |||
477 | If a person has had the opportunity to comment on a patch, but has not | 529 | If a person has had the opportunity to comment on a patch, but has not |
478 | provided such comments, you may optionally add a "Cc:" tag to the patch. | 530 | provided such comments, you may optionally add a "Cc:" tag to the patch. |
479 | This is the only tag which might be added without an explicit action by the | 531 | This is the only tag which might be added without an explicit action by the |
480 | person it names. This tag documents that potentially interested parties | 532 | person it names - but it should indicate that this person was copied on the |
481 | have been included in the discussion | 533 | patch. This tag documents that potentially interested parties |
534 | have been included in the discussion. | ||
482 | 535 | ||
483 | 536 | ||
484 | 14) Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes: | 537 | 13) Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes: |
538 | -------------------------------------------------------------------------- | ||
485 | 539 | ||
486 | The Reported-by tag gives credit to people who find bugs and report them and it | 540 | The Reported-by tag gives credit to people who find bugs and report them and it |
487 | hopefully inspires them to help us again in the future. Please note that if | 541 | hopefully inspires them to help us again in the future. Please note that if |
@@ -541,7 +595,13 @@ which stable kernel versions should receive your fix. This is the preferred | |||
541 | method for indicating a bug fixed by the patch. See #2 above for more details. | 595 | method for indicating a bug fixed by the patch. See #2 above for more details. |
542 | 596 | ||
543 | 597 | ||
544 | 15) The canonical patch format | 598 | 14) The canonical patch format |
599 | ------------------------------ | ||
600 | |||
601 | This section describes how the patch itself should be formatted. Note | ||
602 | that, if you have your patches stored in a git repository, proper patch | ||
603 | formatting can be had with "git format-patch". The tools cannot create | ||
604 | the necessary text, though, so read the instructions below anyway. | ||
545 | 605 | ||
546 | The canonical patch subject line is: | 606 | The canonical patch subject line is: |
547 | 607 | ||
@@ -549,7 +609,8 @@ The canonical patch subject line is: | |||
549 | 609 | ||
550 | The canonical patch message body contains the following: | 610 | The canonical patch message body contains the following: |
551 | 611 | ||
552 | - A "from" line specifying the patch author. | 612 | - A "from" line specifying the patch author (only needed if the person |
613 | sending the patch is not the author). | ||
553 | 614 | ||
554 | - An empty line. | 615 | - An empty line. |
555 | 616 | ||
@@ -656,128 +717,63 @@ See more details on the proper patch format in the following | |||
656 | references. | 717 | references. |
657 | 718 | ||
658 | 719 | ||
659 | 16) Sending "git pull" requests (from Linus emails) | 720 | 15) Sending "git pull" requests |
660 | 721 | ------------------------------- | |
661 | Please write the git repo address and branch name alone on the same line | ||
662 | so that I can't even by mistake pull from the wrong branch, and so | ||
663 | that a triple-click just selects the whole thing. | ||
664 | |||
665 | So the proper format is something along the lines of: | ||
666 | |||
667 | "Please pull from | ||
668 | |||
669 | git://jdelvare.pck.nerim.net/jdelvare-2.6 i2c-for-linus | ||
670 | |||
671 | to get these changes:" | ||
672 | |||
673 | so that I don't have to hunt-and-peck for the address and inevitably | ||
674 | get it wrong (actually, I've only gotten it wrong a few times, and | ||
675 | checking against the diffstat tells me when I get it wrong, but I'm | ||
676 | just a lot more comfortable when I don't have to "look for" the right | ||
677 | thing to pull, and double-check that I have the right branch-name). | ||
678 | |||
679 | |||
680 | Please use "git diff -M --stat --summary" to generate the diffstat: | ||
681 | the -M enables rename detection, and the summary enables a summary of | ||
682 | new/deleted or renamed files. | ||
683 | |||
684 | With rename detection, the statistics are rather different [...] | ||
685 | because git will notice that a fair number of the changes are renames. | ||
686 | |||
687 | ----------------------------------- | ||
688 | SECTION 2 - HINTS, TIPS, AND TRICKS | ||
689 | ----------------------------------- | ||
690 | |||
691 | This section lists many of the common "rules" associated with code | ||
692 | submitted to the kernel. There are always exceptions... but you must | ||
693 | have a really good reason for doing so. You could probably call this | ||
694 | section Linus Computer Science 101. | ||
695 | |||
696 | |||
697 | |||
698 | 1) Read Documentation/CodingStyle | ||
699 | |||
700 | Nuff said. If your code deviates too much from this, it is likely | ||
701 | to be rejected without further review, and without comment. | ||
702 | |||
703 | One significant exception is when moving code from one file to | ||
704 | another -- in this case you should not modify the moved code at all in | ||
705 | the same patch which moves it. This clearly delineates the act of | ||
706 | moving the code and your changes. This greatly aids review of the | ||
707 | actual differences and allows tools to better track the history of | ||
708 | the code itself. | ||
709 | |||
710 | Check your patches with the patch style checker prior to submission | ||
711 | (scripts/checkpatch.pl). The style checker should be viewed as | ||
712 | a guide not as the final word. If your code looks better with | ||
713 | a violation then its probably best left alone. | ||
714 | |||
715 | The checker reports at three levels: | ||
716 | - ERROR: things that are very likely to be wrong | ||
717 | - WARNING: things requiring careful review | ||
718 | - CHECK: things requiring thought | ||
719 | |||
720 | You should be able to justify all violations that remain in your | ||
721 | patch. | ||
722 | |||
723 | |||
724 | |||
725 | 2) #ifdefs are ugly | ||
726 | |||
727 | Code cluttered with ifdefs is difficult to read and maintain. Don't do | ||
728 | it. Instead, put your ifdefs in a header, and conditionally define | ||
729 | 'static inline' functions, or macros, which are used in the code. | ||
730 | Let the compiler optimize away the "no-op" case. | ||
731 | |||
732 | Simple example, of poor code: | ||
733 | |||
734 | dev = alloc_etherdev (sizeof(struct funky_private)); | ||
735 | if (!dev) | ||
736 | return -ENODEV; | ||
737 | #ifdef CONFIG_NET_FUNKINESS | ||
738 | init_funky_net(dev); | ||
739 | #endif | ||
740 | |||
741 | Cleaned-up example: | ||
742 | |||
743 | (in header) | ||
744 | #ifndef CONFIG_NET_FUNKINESS | ||
745 | static inline void init_funky_net (struct net_device *d) {} | ||
746 | #endif | ||
747 | 722 | ||
748 | (in the code itself) | 723 | If you have a series of patches, it may be most convenient to have the |
749 | dev = alloc_etherdev (sizeof(struct funky_private)); | 724 | maintainer pull them directly into the subsystem repository with a |
750 | if (!dev) | 725 | "git pull" operation. Note, however, that pulling patches from a developer |
751 | return -ENODEV; | 726 | requires a higher degree of trust than taking patches from a mailing list. |
752 | init_funky_net(dev); | 727 | As a result, many subsystem maintainers are reluctant to take pull |
728 | requests, especially from new, unknown developers. If in doubt you can use | ||
729 | the pull request as the cover letter for a normal posting of the patch | ||
730 | series, giving the maintainer the option of using either. | ||
753 | 731 | ||
732 | A pull request should have [GIT] or [PULL] in the subject line. The | ||
733 | request itself should include the repository name and the branch of | ||
734 | interest on a single line; it should look something like: | ||
754 | 735 | ||
736 | Please pull from | ||
755 | 737 | ||
756 | 3) 'static inline' is better than a macro | 738 | git://jdelvare.pck.nerim.net/jdelvare-2.6 i2c-for-linus |
757 | 739 | ||
758 | Static inline functions are greatly preferred over macros. | 740 | to get these changes:" |
759 | They provide type safety, have no length limitations, no formatting | ||
760 | limitations, and under gcc they are as cheap as macros. | ||
761 | 741 | ||
762 | Macros should only be used for cases where a static inline is clearly | 742 | A pull request should also include an overall message saying what will be |
763 | suboptimal [there are a few, isolated cases of this in fast paths], | 743 | included in the request, a "git shortlog" listing of the patches |
764 | or where it is impossible to use a static inline function [such as | 744 | themselves, and a diffstat showing the overall effect of the patch series. |
765 | string-izing]. | 745 | The easiest way to get all this information together is, of course, to let |
746 | git do it for you with the "git request-pull" command. | ||
766 | 747 | ||
767 | 'static inline' is preferred over 'static __inline__', 'extern inline', | 748 | Some maintainers (including Linus) want to see pull requests from signed |
768 | and 'extern __inline__'. | 749 | commits; that increases their confidence that the request actually came |
750 | from you. Linus, in particular, will not pull from public hosting sites | ||
751 | like GitHub in the absence of a signed tag. | ||
769 | 752 | ||
753 | The first step toward creating such tags is to make a GNUPG key and get it | ||
754 | signed by one or more core kernel developers. This step can be hard for | ||
755 | new developers, but there is no way around it. Attending conferences can | ||
756 | be a good way to find developers who can sign your key. | ||
770 | 757 | ||
758 | Once you have prepared a patch series in git that you wish to have somebody | ||
759 | pull, create a signed tag with "git tag -s". This will create a new tag | ||
760 | identifying the last commit in the series and containing a signature | ||
761 | created with your private key. You will also have the opportunity to add a | ||
762 | changelog-style message to the tag; this is an ideal place to describe the | ||
763 | effects of the pull request as a whole. | ||
771 | 764 | ||
772 | 4) Don't over-design. | 765 | If the tree the maintainer will be pulling from is not the repository you |
766 | are working from, don't forget to push the signed tag explicitly to the | ||
767 | public tree. | ||
773 | 768 | ||
774 | Don't try to anticipate nebulous future cases which may or may not | 769 | When generating your pull request, use the signed tag as the target. A |
775 | be useful: "Make it as simple as you can, and no simpler." | 770 | command like this will do the trick: |
776 | 771 | ||
772 | git request-pull master git://my.public.tree/linux.git my-signed-tag | ||
777 | 773 | ||
778 | 774 | ||
779 | ---------------------- | 775 | ---------------------- |
780 | SECTION 3 - REFERENCES | 776 | SECTION 2 - REFERENCES |
781 | ---------------------- | 777 | ---------------------- |
782 | 778 | ||
783 | Andrew Morton, "The perfect patch" (tpp). | 779 | Andrew Morton, "The perfect patch" (tpp). |