| 1 | .. _submittingpatches: |
| 2 | |
| 3 | Submitting patches: the essential guide to getting your code into the kernel |
| 4 | ============================================================================ |
| 5 | |
| 6 | For a person or company who wishes to submit a change to the Linux |
| 7 | kernel, the process can sometimes be daunting if you're not familiar |
| 8 | with "the system." This text is a collection of suggestions which |
| 9 | can greatly increase the chances of your change being accepted. |
| 10 | |
| 11 | This document contains a large number of suggestions in a relatively terse |
| 12 | format. For detailed information on how the kernel development process |
| 13 | works, see Documentation/process/development-process.rst. Also, read |
| 14 | Documentation/process/submit-checklist.rst |
| 15 | for a list of items to check before submitting code. |
| 16 | For device tree binding patches, read |
| 17 | Documentation/devicetree/bindings/submitting-patches.rst. |
| 18 | |
| 19 | This documentation assumes that you're using ``git`` to prepare your patches. |
| 20 | If you're unfamiliar with ``git``, you would be well-advised to learn how to |
| 21 | use it, it will make your life as a kernel developer and in general much |
| 22 | easier. |
| 23 | |
| 24 | Some subsystems and maintainer trees have additional information about |
| 25 | their workflow and expectations, see |
| 26 | :ref:`Documentation/process/maintainer-handbooks.rst <maintainer_handbooks_main>`. |
| 27 | |
| 28 | Obtain a current source tree |
| 29 | ---------------------------- |
| 30 | |
| 31 | If you do not have a repository with the current kernel source handy, use |
| 32 | ``git`` to obtain one. You'll want to start with the mainline repository, |
| 33 | which can be grabbed with:: |
| 34 | |
| 35 | git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git |
| 36 | |
| 37 | Note, however, that you may not want to develop against the mainline tree |
| 38 | directly. Most subsystem maintainers run their own trees and want to see |
| 39 | patches prepared against those trees. See the **T:** entry for the subsystem |
| 40 | in the MAINTAINERS file to find that tree, or simply ask the maintainer if |
| 41 | the tree is not listed there. |
| 42 | |
| 43 | .. _describe_changes: |
| 44 | |
| 45 | Describe your changes |
| 46 | --------------------- |
| 47 | |
| 48 | Describe your problem. Whether your patch is a one-line bug fix or |
| 49 | 5000 lines of a new feature, there must be an underlying problem that |
| 50 | motivated you to do this work. Convince the reviewer that there is a |
| 51 | problem worth fixing and that it makes sense for them to read past the |
| 52 | first paragraph. |
| 53 | |
| 54 | Describe user-visible impact. Straight up crashes and lockups are |
| 55 | pretty convincing, but not all bugs are that blatant. Even if the |
| 56 | problem was spotted during code review, describe the impact you think |
| 57 | it can have on users. Keep in mind that the majority of Linux |
| 58 | installations run kernels from secondary stable trees or |
| 59 | vendor/product-specific trees that cherry-pick only specific patches |
| 60 | from upstream, so include anything that could help route your change |
| 61 | downstream: provoking circumstances, excerpts from dmesg, crash |
| 62 | descriptions, performance regressions, latency spikes, lockups, etc. |
| 63 | |
| 64 | Quantify optimizations and trade-offs. If you claim improvements in |
| 65 | performance, memory consumption, stack footprint, or binary size, |
| 66 | include numbers that back them up. But also describe non-obvious |
| 67 | costs. Optimizations usually aren't free but trade-offs between CPU, |
| 68 | memory, and readability; or, when it comes to heuristics, between |
| 69 | different workloads. Describe the expected downsides of your |
| 70 | optimization so that the reviewer can weigh costs against benefits. |
| 71 | |
| 72 | Once the problem is established, describe what you are actually doing |
| 73 | about it in technical detail. It's important to describe the change |
| 74 | in plain English for the reviewer to verify that the code is behaving |
| 75 | as you intend it to. |
| 76 | |
| 77 | The maintainer will thank you if you write your patch description in a |
| 78 | form which can be easily pulled into Linux's source code management |
| 79 | system, ``git``, as a "commit log". See :ref:`the_canonical_patch_format`. |
| 80 | |
| 81 | Solve only one problem per patch. If your description starts to get |
| 82 | long, that's a sign that you probably need to split up your patch. |
| 83 | See :ref:`split_changes`. |
| 84 | |
| 85 | When you submit or resubmit a patch or patch series, include the |
| 86 | complete patch description and justification for it. Don't just |
| 87 | say that this is version N of the patch (series). Don't expect the |
| 88 | subsystem maintainer to refer back to earlier patch versions or referenced |
| 89 | URLs to find the patch description and put that into the patch. |
| 90 | I.e., the patch (series) and its description should be self-contained. |
| 91 | This benefits both the maintainers and reviewers. Some reviewers |
| 92 | probably didn't even receive earlier versions of the patch. |
| 93 | |
| 94 | Describe your changes in imperative mood, e.g. "make xyzzy do frotz" |
| 95 | instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy |
| 96 | to do frotz", as if you are giving orders to the codebase to change |
| 97 | its behaviour. |
| 98 | |
| 99 | If you want to refer to a specific commit, don't just refer to the |
| 100 | SHA-1 ID of the commit. Please also include the oneline summary of |
| 101 | the commit, to make it easier for reviewers to know what it is about. |
| 102 | Example:: |
| 103 | |
| 104 | Commit e21d2170f36602ae2708 ("video: remove unnecessary |
| 105 | platform_set_drvdata()") removed the unnecessary |
| 106 | platform_set_drvdata(), but left the variable "dev" unused, |
| 107 | delete it. |
| 108 | |
| 109 | You should also be sure to use at least the first twelve characters of the |
| 110 | SHA-1 ID. The kernel repository holds a *lot* of objects, making |
| 111 | collisions with shorter IDs a real possibility. Bear in mind that, even if |
| 112 | there is no collision with your six-character ID now, that condition may |
| 113 | change five years from now. |
| 114 | |
| 115 | If related discussions or any other background information behind the change |
| 116 | can be found on the web, add 'Link:' tags pointing to it. If the patch is a |
| 117 | result of some earlier mailing list discussions or something documented on the |
| 118 | web, point to it. |
| 119 | |
| 120 | When linking to mailing list archives, preferably use the lore.kernel.org |
| 121 | message archiver service. To create the link URL, use the contents of the |
| 122 | ``Message-ID`` header of the message without the surrounding angle brackets. |
| 123 | For example:: |
| 124 | |
| 125 | Link: https://lore.kernel.org/30th.anniversary.repost@klaava.Helsinki.FI |
| 126 | |
| 127 | Please check the link to make sure that it is actually working and points |
| 128 | to the relevant message. |
| 129 | |
| 130 | However, try to make your explanation understandable without external |
| 131 | resources. In addition to giving a URL to a mailing list archive or bug, |
| 132 | summarize the relevant points of the discussion that led to the |
| 133 | patch as submitted. |
| 134 | |
| 135 | In case your patch fixes a bug, use the 'Closes:' tag with a URL referencing |
| 136 | the report in the mailing list archives or a public bug tracker. For example:: |
| 137 | |
| 138 | Closes: https://example.com/issues/1234 |
| 139 | |
| 140 | Some bug trackers have the ability to close issues automatically when a |
| 141 | commit with such a tag is applied. Some bots monitoring mailing lists can |
| 142 | also track such tags and take certain actions. Private bug trackers and |
| 143 | invalid URLs are forbidden. |
| 144 | |
| 145 | If your patch fixes a bug in a specific commit, e.g. you found an issue using |
| 146 | ``git bisect``, please use the 'Fixes:' tag with at least the first 12 |
| 147 | characters of the SHA-1 ID, and the one line summary. Do not split the tag |
| 148 | across multiple lines, tags are exempt from the "wrap at 75 columns" rule in |
| 149 | order to simplify parsing scripts. For example:: |
| 150 | |
| 151 | Fixes: 54a4f0239f2e ("KVM: MMU: make kvm_mmu_zap_page() return the number of pages it actually freed") |
| 152 | |
| 153 | The following ``git config`` settings can be used to add a pretty format for |
| 154 | outputting the above style in the ``git log`` or ``git show`` commands:: |
| 155 | |
| 156 | [core] |
| 157 | abbrev = 12 |
| 158 | [pretty] |
| 159 | fixes = Fixes: %h (\"%s\") |
| 160 | |
| 161 | An example call:: |
| 162 | |
| 163 | $ git log -1 --pretty=fixes 54a4f0239f2e |
| 164 | Fixes: 54a4f0239f2e ("KVM: MMU: make kvm_mmu_zap_page() return the number of pages it actually freed") |
| 165 | |
| 166 | .. _split_changes: |
| 167 | |
| 168 | Separate your changes |
| 169 | --------------------- |
| 170 | |
| 171 | Separate each **logical change** into a separate patch. |
| 172 | |
| 173 | For example, if your changes include both bug fixes and performance |
| 174 | enhancements for a single driver, separate those changes into two |
| 175 | or more patches. If your changes include an API update, and a new |
| 176 | driver which uses that new API, separate those into two patches. |
| 177 | |
| 178 | On the other hand, if you make a single change to numerous files, |
| 179 | group those changes into a single patch. Thus a single logical change |
| 180 | is contained within a single patch. |
| 181 | |
| 182 | The point to remember is that each patch should make an easily understood |
| 183 | change that can be verified by reviewers. Each patch should be justifiable |
| 184 | on its own merits. |
| 185 | |
| 186 | If one patch depends on another patch in order for a change to be |
| 187 | complete, that is OK. Simply note **"this patch depends on patch X"** |
| 188 | in your patch description. |
| 189 | |
| 190 | When dividing your change into a series of patches, take special care to |
| 191 | ensure that the kernel builds and runs properly after each patch in the |
| 192 | series. Developers using ``git bisect`` to track down a problem can end up |
| 193 | splitting your patch series at any point; they will not thank you if you |
| 194 | introduce bugs in the middle. |
| 195 | |
| 196 | If you cannot condense your patch set into a smaller set of patches, |
| 197 | then only post say 15 or so at a time and wait for review and integration. |
| 198 | |
| 199 | |
| 200 | |
| 201 | Style-check your changes |
| 202 | ------------------------ |
| 203 | |
| 204 | Check your patch for basic style violations, details of which can be |
| 205 | found in Documentation/process/coding-style.rst. |
| 206 | Failure to do so simply wastes |
| 207 | the reviewers time and will get your patch rejected, probably |
| 208 | without even being read. |
| 209 | |
| 210 | One significant exception is when moving code from one file to |
| 211 | another -- in this case you should not modify the moved code at all in |
| 212 | the same patch which moves it. This clearly delineates the act of |
| 213 | moving the code and your changes. This greatly aids review of the |
| 214 | actual differences and allows tools to better track the history of |
| 215 | the code itself. |
| 216 | |
| 217 | Check your patches with the patch style checker prior to submission |
| 218 | (scripts/checkpatch.pl). Note, though, that the style checker should be |
| 219 | viewed as a guide, not as a replacement for human judgment. If your code |
| 220 | looks better with a violation then its probably best left alone. |
| 221 | |
| 222 | The checker reports at three levels: |
| 223 | - ERROR: things that are very likely to be wrong |
| 224 | - WARNING: things requiring careful review |
| 225 | - CHECK: things requiring thought |
| 226 | |
| 227 | You should be able to justify all violations that remain in your |
| 228 | patch. |
| 229 | |
| 230 | |
| 231 | Select the recipients for your patch |
| 232 | ------------------------------------ |
| 233 | |
| 234 | You should always copy the appropriate subsystem maintainer(s) and list(s) on |
| 235 | any patch to code that they maintain; look through the MAINTAINERS file and the |
| 236 | source code revision history to see who those maintainers are. The script |
| 237 | scripts/get_maintainer.pl can be very useful at this step (pass paths to your |
| 238 | patches as arguments to scripts/get_maintainer.pl). If you cannot find a |
| 239 | maintainer for the subsystem you are working on, Andrew Morton |
| 240 | (akpm@linux-foundation.org) serves as a maintainer of last resort. |
| 241 | |
| 242 | linux-kernel@vger.kernel.org should be used by default for all patches, but the |
| 243 | volume on that list has caused a number of developers to tune it out. Please |
| 244 | do not spam unrelated lists and unrelated people, though. |
| 245 | |
| 246 | Many kernel-related lists are hosted at kernel.org; you can find a list |
| 247 | of them at https://subspace.kernel.org. There are kernel-related lists |
| 248 | hosted elsewhere as well, though. |
| 249 | |
| 250 | Linus Torvalds is the final arbiter of all changes accepted into the |
| 251 | Linux kernel. His e-mail address is <torvalds@linux-foundation.org>. |
| 252 | He gets a lot of e-mail, and, at this point, very few patches go through |
| 253 | Linus directly, so typically you should do your best to -avoid- |
| 254 | sending him e-mail. |
| 255 | |
| 256 | If you have a patch that fixes an exploitable security bug, send that patch |
| 257 | to security@kernel.org. For severe bugs, a short embargo may be considered |
| 258 | to allow distributors to get the patch out to users; in such cases, |
| 259 | obviously, the patch should not be sent to any public lists. See also |
| 260 | Documentation/process/security-bugs.rst. |
| 261 | |
| 262 | Patches that fix a severe bug in a released kernel should be directed |
| 263 | toward the stable maintainers by putting a line like this:: |
| 264 | |
| 265 | Cc: stable@vger.kernel.org |
| 266 | |
| 267 | into the sign-off area of your patch (note, NOT an email recipient). You |
| 268 | should also read Documentation/process/stable-kernel-rules.rst |
| 269 | in addition to this document. |
| 270 | |
| 271 | If changes affect userland-kernel interfaces, please send the MAN-PAGES |
| 272 | maintainer (as listed in the MAINTAINERS file) a man-pages patch, or at |
| 273 | least a notification of the change, so that some information makes its way |
| 274 | into the manual pages. User-space API changes should also be copied to |
| 275 | linux-api@vger.kernel.org. |
| 276 | |
| 277 | |
| 278 | No MIME, no links, no compression, no attachments. Just plain text |
| 279 | ------------------------------------------------------------------- |
| 280 | |
| 281 | 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 |
| 283 | developer to be able to "quote" your changes, using standard e-mail |
| 284 | tools, so that they may comment on specific portions of your code. |
| 285 | |
| 286 | For this reason, all patches should be submitted by e-mail "inline". The |
| 287 | easiest way to do this is with ``git send-email``, which is strongly |
| 288 | recommended. An interactive tutorial for ``git send-email`` is available at |
| 289 | https://git-send-email.io. |
| 290 | |
| 291 | If you choose not to use ``git send-email``: |
| 292 | |
| 293 | .. warning:: |
| 294 | |
| 295 | Be wary of your editor's word-wrap corrupting your patch, |
| 296 | if you choose to cut-n-paste your patch. |
| 297 | |
| 298 | Do not attach the patch as a MIME attachment, compressed or not. |
| 299 | Many popular e-mail applications will not always transmit a MIME |
| 300 | attachment as plain text, making it impossible to comment on your |
| 301 | code. A MIME attachment also takes Linus a bit more time to process, |
| 302 | decreasing the likelihood of your MIME-attached change being accepted. |
| 303 | |
| 304 | Exception: If your mailer is mangling patches then someone may ask |
| 305 | you to re-send them using MIME. |
| 306 | |
| 307 | See Documentation/process/email-clients.rst for hints about configuring |
| 308 | your e-mail client so that it sends your patches untouched. |
| 309 | |
| 310 | Respond to review comments |
| 311 | -------------------------- |
| 312 | |
| 313 | Your patch will almost certainly get comments from reviewers on ways in |
| 314 | which the patch can be improved, in the form of a reply to your email. You must |
| 315 | respond to those comments; ignoring reviewers is a good way to get ignored in |
| 316 | return. You can simply reply to their emails to answer their comments. Review |
| 317 | comments or questions that do not lead to a code change should almost certainly |
| 318 | bring about a comment or changelog entry so that the next reviewer better |
| 319 | understands what is going on. |
| 320 | |
| 321 | Be sure to tell the reviewers what changes you are making and to thank them |
| 322 | for their time. Code review is a tiring and time-consuming process, and |
| 323 | reviewers sometimes get grumpy. Even in that case, though, respond |
| 324 | politely and address the problems they have pointed out. When sending a next |
| 325 | version, add a ``patch changelog`` to the cover letter or to individual patches |
| 326 | explaining difference against previous submission (see |
| 327 | :ref:`the_canonical_patch_format`). |
| 328 | Notify people that commented on your patch about new versions by adding them to |
| 329 | the patches CC list. |
| 330 | |
| 331 | See Documentation/process/email-clients.rst for recommendations on email |
| 332 | clients and mailing list etiquette. |
| 333 | |
| 334 | .. _interleaved_replies: |
| 335 | |
| 336 | Use trimmed interleaved replies in email discussions |
| 337 | ---------------------------------------------------- |
| 338 | Top-posting is strongly discouraged in Linux kernel development |
| 339 | discussions. Interleaved (or "inline") replies make conversations much |
| 340 | easier to follow. For more details see: |
| 341 | https://en.wikipedia.org/wiki/Posting_style#Interleaved_style |
| 342 | |
| 343 | As is frequently quoted on the mailing list:: |
| 344 | |
| 345 | A: http://en.wikipedia.org/wiki/Top_post |
| 346 | Q: Were do I find info about this thing called top-posting? |
| 347 | A: Because it messes up the order in which people normally read text. |
| 348 | Q: Why is top-posting such a bad thing? |
| 349 | A: Top-posting. |
| 350 | Q: What is the most annoying thing in e-mail? |
| 351 | |
| 352 | Similarly, please trim all unneeded quotations that aren't relevant |
| 353 | to your reply. This makes responses easier to find, and saves time and |
| 354 | space. For more details see: http://daringfireball.net/2007/07/on_top :: |
| 355 | |
| 356 | A: No. |
| 357 | Q: Should I include quotations after my reply? |
| 358 | |
| 359 | .. _resend_reminders: |
| 360 | |
| 361 | Don't get discouraged - or impatient |
| 362 | ------------------------------------ |
| 363 | |
| 364 | After you have submitted your change, be patient and wait. Reviewers are |
| 365 | busy people and may not get to your patch right away. |
| 366 | |
| 367 | Once upon a time, patches used to disappear into the void without comment, |
| 368 | but the development process works more smoothly than that now. You should |
| 369 | receive comments within a few weeks (typically 2-3); if that does not |
| 370 | happen, make sure that you have sent your patches to the right place. |
| 371 | Wait for a minimum of one week before resubmitting or pinging reviewers |
| 372 | - possibly longer during busy times like merge windows. |
| 373 | |
| 374 | It's also ok to resend the patch or the patch series after a couple of |
| 375 | weeks with the word "RESEND" added to the subject line:: |
| 376 | |
| 377 | [PATCH Vx RESEND] sub/sys: Condensed patch summary |
| 378 | |
| 379 | Don't add "RESEND" when you are submitting a modified version of your |
| 380 | patch or patch series - "RESEND" only applies to resubmission of a |
| 381 | patch or patch series which have not been modified in any way from the |
| 382 | previous submission. |
| 383 | |
| 384 | |
| 385 | Include PATCH in the subject |
| 386 | ----------------------------- |
| 387 | |
| 388 | Due to high e-mail traffic to Linus, and to linux-kernel, it is common |
| 389 | convention to prefix your subject line with [PATCH]. This lets Linus |
| 390 | and other kernel developers more easily distinguish patches from other |
| 391 | e-mail discussions. |
| 392 | |
| 393 | ``git send-email`` will do this for you automatically. |
| 394 | |
| 395 | |
| 396 | Sign your work - the Developer's Certificate of Origin |
| 397 | ------------------------------------------------------ |
| 398 | |
| 399 | To improve tracking of who did what, especially with patches that can |
| 400 | percolate to their final resting place in the kernel through several |
| 401 | layers of maintainers, we've introduced a "sign-off" procedure on |
| 402 | patches that are being emailed around. |
| 403 | |
| 404 | The sign-off is a simple line at the end of the explanation for the |
| 405 | patch, which certifies that you wrote it or otherwise have the right to |
| 406 | pass it on as an open-source patch. The rules are pretty simple: if you |
| 407 | can certify the below: |
| 408 | |
| 409 | Developer's Certificate of Origin 1.1 |
| 410 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ |
| 411 | |
| 412 | By making a contribution to this project, I certify that: |
| 413 | |
| 414 | (a) The contribution was created in whole or in part by me and I |
| 415 | have the right to submit it under the open source license |
| 416 | indicated in the file; or |
| 417 | |
| 418 | (b) The contribution is based upon previous work that, to the best |
| 419 | of my knowledge, is covered under an appropriate open source |
| 420 | license and I have the right under that license to submit that |
| 421 | work with modifications, whether created in whole or in part |
| 422 | by me, under the same open source license (unless I am |
| 423 | permitted to submit under a different license), as indicated |
| 424 | in the file; or |
| 425 | |
| 426 | (c) The contribution was provided directly to me by some other |
| 427 | person who certified (a), (b) or (c) and I have not modified |
| 428 | it. |
| 429 | |
| 430 | (d) I understand and agree that this project and the contribution |
| 431 | are public and that a record of the contribution (including all |
| 432 | personal information I submit with it, including my sign-off) is |
| 433 | maintained indefinitely and may be redistributed consistent with |
| 434 | this project or the open source license(s) involved. |
| 435 | |
| 436 | then you just add a line saying:: |
| 437 | |
| 438 | Signed-off-by: Random J Developer <random@developer.example.org> |
| 439 | |
| 440 | using a known identity (sorry, no anonymous contributions.) |
| 441 | This will be done for you automatically if you use ``git commit -s``. |
| 442 | Reverts should also include "Signed-off-by". ``git revert -s`` does that |
| 443 | for you. |
| 444 | |
| 445 | Some people also put extra tags at the end. They'll just be ignored for |
| 446 | now, but you can do this to mark internal company procedures or just |
| 447 | point out some special detail about the sign-off. |
| 448 | |
| 449 | Any further SoBs (Signed-off-by:'s) following the author's SoB are from |
| 450 | people handling and transporting the patch, but were not involved in its |
| 451 | development. SoB chains should reflect the **real** route a patch took |
| 452 | as it was propagated to the maintainers and ultimately to Linus, with |
| 453 | the first SoB entry signalling primary authorship of a single author. |
| 454 | |
| 455 | |
| 456 | When to use Acked-by:, Cc:, and Co-developed-by: |
| 457 | ------------------------------------------------ |
| 458 | |
| 459 | The Signed-off-by: tag indicates that the signer was involved in the |
| 460 | development of the patch, or that he/she was in the patch's delivery path. |
| 461 | |
| 462 | If a person was not directly involved in the preparation or handling of a |
| 463 | patch but wishes to signify and record their approval of it then they can |
| 464 | ask to have an Acked-by: line added to the patch's changelog. |
| 465 | |
| 466 | Acked-by: is meant to be used by those responsible for or involved with the |
| 467 | affected code in one way or another. Most commonly, the maintainer when that |
| 468 | maintainer neither contributed to nor forwarded the patch. |
| 469 | |
| 470 | Acked-by: may also be used by other stakeholders, such as people with domain |
| 471 | knowledge (e.g. the original author of the code being modified), userspace-side |
| 472 | reviewers for a kernel uAPI patch or key users of a feature. Optionally, in |
| 473 | these cases, it can be useful to add a "# Suffix" to clarify its meaning:: |
| 474 | |
| 475 | Acked-by: The Stakeholder <stakeholder@example.org> # As primary user |
| 476 | |
| 477 | Acked-by: is not as formal as Signed-off-by:. It is a record that the acker |
| 478 | has at least reviewed the patch and has indicated acceptance. Hence patch |
| 479 | mergers will sometimes manually convert an acker's "yep, looks good to me" |
| 480 | into an Acked-by: (but note that it is usually better to ask for an |
| 481 | explicit ack). |
| 482 | |
| 483 | Acked-by: is also less formal than Reviewed-by:. For instance, maintainers may |
| 484 | use it to signify that they are OK with a patch landing, but they may not have |
| 485 | reviewed it as thoroughly as if a Reviewed-by: was provided. Similarly, a key |
| 486 | user may not have carried out a technical review of the patch, yet they may be |
| 487 | satisfied with the general approach, the feature or the user-facing interface. |
| 488 | |
| 489 | Acked-by: does not necessarily indicate acknowledgement of the entire patch. |
| 490 | For example, if a patch affects multiple subsystems and has an Acked-by: from |
| 491 | one subsystem maintainer then this usually indicates acknowledgement of just |
| 492 | the part which affects that maintainer's code. Judgement should be used here. |
| 493 | When in doubt people should refer to the original discussion in the mailing |
| 494 | list archives. A "# Suffix" may also be used in this case to clarify. |
| 495 | |
| 496 | If a person has had the opportunity to comment on a patch, but has not |
| 497 | provided such comments, you may optionally add a ``Cc:`` tag to the patch. |
| 498 | This tag documents that potentially interested parties have been included in |
| 499 | the discussion. Note, this is one of only three tags you might be able to use |
| 500 | without explicit permission of the person named (see 'Tagging people requires |
| 501 | permission' below for details). |
| 502 | |
| 503 | Co-developed-by: states that the patch was co-created by multiple developers; |
| 504 | it is used to give attribution to co-authors (in addition to the author |
| 505 | attributed by the From: tag) when several people work on a single patch. Since |
| 506 | Co-developed-by: denotes authorship, every Co-developed-by: must be immediately |
| 507 | followed by a Signed-off-by: of the associated co-author. Standard sign-off |
| 508 | procedure applies, i.e. the ordering of Signed-off-by: tags should reflect the |
| 509 | chronological history of the patch insofar as possible, regardless of whether |
| 510 | the author is attributed via From: or Co-developed-by:. Notably, the last |
| 511 | Signed-off-by: must always be that of the developer submitting the patch. |
| 512 | |
| 513 | Note, the From: tag is optional when the From: author is also the person (and |
| 514 | email) listed in the From: line of the email header. |
| 515 | |
| 516 | Example of a patch submitted by the From: author:: |
| 517 | |
| 518 | <changelog> |
| 519 | |
| 520 | Co-developed-by: First Co-Author <first@coauthor.example.org> |
| 521 | Signed-off-by: First Co-Author <first@coauthor.example.org> |
| 522 | Co-developed-by: Second Co-Author <second@coauthor.example.org> |
| 523 | Signed-off-by: Second Co-Author <second@coauthor.example.org> |
| 524 | Signed-off-by: From Author <from@author.example.org> |
| 525 | |
| 526 | Example of a patch submitted by a Co-developed-by: author:: |
| 527 | |
| 528 | From: From Author <from@author.example.org> |
| 529 | |
| 530 | <changelog> |
| 531 | |
| 532 | Co-developed-by: Random Co-Author <random@coauthor.example.org> |
| 533 | Signed-off-by: Random Co-Author <random@coauthor.example.org> |
| 534 | Signed-off-by: From Author <from@author.example.org> |
| 535 | Co-developed-by: Submitting Co-Author <sub@coauthor.example.org> |
| 536 | Signed-off-by: Submitting Co-Author <sub@coauthor.example.org> |
| 537 | |
| 538 | |
| 539 | Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes: |
| 540 | ---------------------------------------------------------------------- |
| 541 | |
| 542 | The Reported-by tag gives credit to people who find bugs and report them and it |
| 543 | hopefully inspires them to help us again in the future. The tag is intended for |
| 544 | bugs; please do not use it to credit feature requests. The tag should be |
| 545 | followed by a Closes: tag pointing to the report, unless the report is not |
| 546 | available on the web. The Link: tag can be used instead of Closes: if the patch |
| 547 | fixes a part of the issue(s) being reported. Note, the Reported-by tag is one |
| 548 | of only three tags you might be able to use without explicit permission of the |
| 549 | person named (see 'Tagging people requires permission' below for details). |
| 550 | |
| 551 | A Tested-by: tag indicates that the patch has been successfully tested (in |
| 552 | some environment) by the person named. This tag informs maintainers that |
| 553 | some testing has been performed, provides a means to locate testers for |
| 554 | future patches, and ensures credit for the testers. |
| 555 | |
| 556 | Reviewed-by:, instead, indicates that the patch has been reviewed and found |
| 557 | acceptable according to the Reviewer's Statement: |
| 558 | |
| 559 | Reviewer's statement of oversight |
| 560 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ |
| 561 | |
| 562 | By offering my Reviewed-by: tag, I state that: |
| 563 | |
| 564 | (a) I have carried out a technical review of this patch to |
| 565 | evaluate its appropriateness and readiness for inclusion into |
| 566 | the mainline kernel. |
| 567 | |
| 568 | (b) Any problems, concerns, or questions relating to the patch |
| 569 | have been communicated back to the submitter. I am satisfied |
| 570 | with the submitter's response to my comments. |
| 571 | |
| 572 | (c) While there may be things that could be improved with this |
| 573 | submission, I believe that it is, at this time, (1) a |
| 574 | worthwhile modification to the kernel, and (2) free of known |
| 575 | issues which would argue against its inclusion. |
| 576 | |
| 577 | (d) While I have reviewed the patch and believe it to be sound, I |
| 578 | do not (unless explicitly stated elsewhere) make any |
| 579 | warranties or guarantees that it will achieve its stated |
| 580 | purpose or function properly in any given situation. |
| 581 | |
| 582 | A Reviewed-by tag is a statement of opinion that the patch is an |
| 583 | appropriate modification of the kernel without any remaining serious |
| 584 | technical issues. Any interested reviewer (who has done the work) can |
| 585 | offer a Reviewed-by tag for a patch. This tag serves to give credit to |
| 586 | reviewers and to inform maintainers of the degree of review which has been |
| 587 | done on the patch. Reviewed-by: tags, when supplied by reviewers known to |
| 588 | understand the subject area and to perform thorough reviews, will normally |
| 589 | increase the likelihood of your patch getting into the kernel. |
| 590 | |
| 591 | Both Tested-by and Reviewed-by tags, once received on mailing list from tester |
| 592 | or reviewer, should be added by author to the applicable patches when sending |
| 593 | next versions. However if the patch has changed substantially in following |
| 594 | version, these tags might not be applicable anymore and thus should be removed. |
| 595 | Usually removal of someone's Tested-by or Reviewed-by tags should be mentioned |
| 596 | in the patch changelog (after the '---' separator). |
| 597 | |
| 598 | A Suggested-by: tag indicates that the patch idea is suggested by the person |
| 599 | named and ensures credit to the person for the idea: if we diligently credit |
| 600 | our idea reporters, they will, hopefully, be inspired to help us again in the |
| 601 | future. Note, this is one of only three tags you might be able to use without |
| 602 | explicit permission of the person named (see 'Tagging people requires |
| 603 | permission' below for details). |
| 604 | |
| 605 | A Fixes: tag indicates that the patch fixes an issue in a previous commit. It |
| 606 | is used to make it easy to determine where a bug originated, which can help |
| 607 | review a bug fix. This tag also assists the stable kernel team in determining |
| 608 | which stable kernel versions should receive your fix. This is the preferred |
| 609 | method for indicating a bug fixed by the patch. See :ref:`describe_changes` |
| 610 | for more details. |
| 611 | |
| 612 | Note: Attaching a Fixes: tag does not subvert the stable kernel rules |
| 613 | process nor the requirement to Cc: stable@vger.kernel.org on all stable |
| 614 | patch candidates. For more information, please read |
| 615 | Documentation/process/stable-kernel-rules.rst. |
| 616 | |
| 617 | Finally, while providing tags is welcome and typically very appreciated, please |
| 618 | note that signers (i.e. submitters and maintainers) may use their discretion in |
| 619 | applying offered tags. |
| 620 | |
| 621 | .. _tagging_people: |
| 622 | |
| 623 | Tagging people requires permission |
| 624 | ---------------------------------- |
| 625 | |
| 626 | Be careful in the addition of the aforementioned tags to your patches, as all |
| 627 | except for Cc:, Reported-by:, and Suggested-by: need explicit permission of the |
| 628 | person named. For those three implicit permission is sufficient if the person |
| 629 | contributed to the Linux kernel using that name and email address according |
| 630 | to the lore archives or the commit history -- and in case of Reported-by: |
| 631 | and Suggested-by: did the reporting or suggestion in public. Note, |
| 632 | bugzilla.kernel.org is a public place in this sense, but email addresses |
| 633 | used there are private; so do not expose them in tags, unless the person |
| 634 | used them in earlier contributions. |
| 635 | |
| 636 | .. _the_canonical_patch_format: |
| 637 | |
| 638 | The canonical patch format |
| 639 | -------------------------- |
| 640 | |
| 641 | This section describes how the patch itself should be formatted. Note |
| 642 | that, if you have your patches stored in a ``git`` repository, proper patch |
| 643 | formatting can be had with ``git format-patch``. The tools cannot create |
| 644 | the necessary text, though, so read the instructions below anyway. |
| 645 | |
| 646 | Subject Line |
| 647 | ^^^^^^^^^^^^ |
| 648 | |
| 649 | The canonical patch subject line is:: |
| 650 | |
| 651 | Subject: [PATCH 001/123] subsystem: summary phrase |
| 652 | |
| 653 | The canonical patch message body contains the following: |
| 654 | |
| 655 | - A ``from`` line specifying the patch author, followed by an empty |
| 656 | line (only needed if the person sending the patch is not the author). |
| 657 | |
| 658 | - The body of the explanation, line wrapped at 75 columns, which will |
| 659 | be copied to the permanent changelog to describe this patch. |
| 660 | |
| 661 | - An empty line. |
| 662 | |
| 663 | - The ``Signed-off-by:`` lines, described above, which will |
| 664 | also go in the changelog. |
| 665 | |
| 666 | - A marker line containing simply ``---``. |
| 667 | |
| 668 | - Any additional comments not suitable for the changelog. |
| 669 | |
| 670 | - The actual patch (``diff`` output). |
| 671 | |
| 672 | The Subject line format makes it very easy to sort the emails |
| 673 | alphabetically by subject line - pretty much any email reader will |
| 674 | support that - since because the sequence number is zero-padded, |
| 675 | the numerical and alphabetic sort is the same. |
| 676 | |
| 677 | The ``subsystem`` in the email's Subject should identify which |
| 678 | area or subsystem of the kernel is being patched. |
| 679 | |
| 680 | The ``summary phrase`` in the email's Subject should concisely |
| 681 | describe the patch which that email contains. The ``summary |
| 682 | phrase`` should not be a filename. Do not use the same ``summary |
| 683 | phrase`` for every patch in a whole patch series (where a ``patch |
| 684 | series`` is an ordered sequence of multiple, related patches). |
| 685 | |
| 686 | Bear in mind that the ``summary phrase`` of your email becomes a |
| 687 | globally-unique identifier for that patch. It propagates all the way |
| 688 | into the ``git`` changelog. The ``summary phrase`` may later be used in |
| 689 | developer discussions which refer to the patch. People will want to |
| 690 | google for the ``summary phrase`` to read discussion regarding that |
| 691 | patch. It will also be the only thing that people may quickly see |
| 692 | when, two or three months later, they are going through perhaps |
| 693 | thousands of patches using tools such as ``gitk`` or ``git log |
| 694 | --oneline``. |
| 695 | |
| 696 | For these reasons, the ``summary`` must be no more than 70-75 |
| 697 | characters, and it must describe both what the patch changes, as well |
| 698 | as why the patch might be necessary. It is challenging to be both |
| 699 | succinct and descriptive, but that is what a well-written summary |
| 700 | should do. |
| 701 | |
| 702 | The ``summary phrase`` may be prefixed by tags enclosed in square |
| 703 | brackets: "Subject: [PATCH <tag>...] <summary phrase>". The tags are |
| 704 | not considered part of the summary phrase, but describe how the patch |
| 705 | should be treated. Common tags might include a version descriptor if |
| 706 | the multiple versions of the patch have been sent out in response to |
| 707 | comments (i.e., "v1, v2, v3"), or "RFC" to indicate a request for |
| 708 | comments. |
| 709 | |
| 710 | If there are four patches in a patch series the individual patches may |
| 711 | be numbered like this: 1/4, 2/4, 3/4, 4/4. This assures that developers |
| 712 | understand the order in which the patches should be applied and that |
| 713 | they have reviewed or applied all of the patches in the patch series. |
| 714 | |
| 715 | Here are some good example Subjects:: |
| 716 | |
| 717 | Subject: [PATCH 2/5] ext2: improve scalability of bitmap searching |
| 718 | Subject: [PATCH v2 01/27] x86: fix eflags tracking |
| 719 | Subject: [PATCH v2] sub/sys: Condensed patch summary |
| 720 | Subject: [PATCH v2 M/N] sub/sys: Condensed patch summary |
| 721 | |
| 722 | From Line |
| 723 | ^^^^^^^^^ |
| 724 | |
| 725 | The ``from`` line must be the very first line in the message body, |
| 726 | and has the form: |
| 727 | |
| 728 | From: Patch Author <author@example.com> |
| 729 | |
| 730 | The ``from`` line specifies who will be credited as the author of the |
| 731 | patch in the permanent changelog. If the ``from`` line is missing, |
| 732 | then the ``From:`` line from the email header will be used to determine |
| 733 | the patch author in the changelog. |
| 734 | |
| 735 | The author may indicate their affiliation or the sponsor of the work |
| 736 | by adding the name of an organization to the ``from`` and ``SoB`` lines, |
| 737 | e.g.: |
| 738 | |
| 739 | From: Patch Author (Company) <author@example.com> |
| 740 | |
| 741 | Explanation Body |
| 742 | ^^^^^^^^^^^^^^^^ |
| 743 | |
| 744 | The explanation body will be committed to the permanent source |
| 745 | changelog, so should make sense to a competent reader who has long since |
| 746 | forgotten the immediate details of the discussion that might have led to |
| 747 | this patch. Including symptoms of the failure which the patch addresses |
| 748 | (kernel log messages, oops messages, etc.) are especially useful for |
| 749 | people who might be searching the commit logs looking for the applicable |
| 750 | patch. The text should be written in such detail so that when read |
| 751 | weeks, months or even years later, it can give the reader the needed |
| 752 | details to grasp the reasoning for **why** the patch was created. |
| 753 | |
| 754 | If a patch fixes a compile failure, it may not be necessary to include |
| 755 | _all_ of the compile failures; just enough that it is likely that |
| 756 | someone searching for the patch can find it. As in the ``summary |
| 757 | phrase``, it is important to be both succinct as well as descriptive. |
| 758 | |
| 759 | .. _backtraces: |
| 760 | |
| 761 | Backtraces in commit messages |
| 762 | """"""""""""""""""""""""""""" |
| 763 | |
| 764 | Backtraces help document the call chain leading to a problem. However, |
| 765 | not all backtraces are helpful. For example, early boot call chains are |
| 766 | unique and obvious. Copying the full dmesg output verbatim, however, |
| 767 | adds distracting information like timestamps, module lists, register and |
| 768 | stack dumps. |
| 769 | |
| 770 | Therefore, the most useful backtraces should distill the relevant |
| 771 | information from the dump, which makes it easier to focus on the real |
| 772 | issue. Here is an example of a well-trimmed backtrace:: |
| 773 | |
| 774 | unchecked MSR access error: WRMSR to 0xd51 (tried to write 0x0000000000000064) |
| 775 | at rIP: 0xffffffffae059994 (native_write_msr+0x4/0x20) |
| 776 | Call Trace: |
| 777 | mba_wrmsr |
| 778 | update_domains |
| 779 | rdtgroup_mkdir |
| 780 | |
| 781 | Commentary |
| 782 | ^^^^^^^^^^ |
| 783 | |
| 784 | The ``---`` marker line serves the essential purpose of marking for |
| 785 | patch handling tools where the changelog message ends. |
| 786 | |
| 787 | One good use for the additional comments after the ``---`` marker is |
| 788 | for a ``diffstat``, to show what files have changed, and the number of |
| 789 | inserted and deleted lines per file. A ``diffstat`` is especially useful |
| 790 | on bigger patches. If you are going to include a ``diffstat`` after the |
| 791 | ``---`` marker, please use ``diffstat`` options ``-p 1 -w 70`` so that |
| 792 | filenames are listed from the top of the kernel source tree and don't |
| 793 | use too much horizontal space (easily fit in 80 columns, maybe with some |
| 794 | indentation). (``git`` generates appropriate diffstats by default.) |
| 795 | |
| 796 | Other comments relevant only to the moment or the maintainer, not |
| 797 | suitable for the permanent changelog, should also go here. A good |
| 798 | example of such comments might be ``patch changelogs`` which describe |
| 799 | what has changed between the v1 and v2 version of the patch. |
| 800 | |
| 801 | Please put this information **after** the ``---`` line which separates |
| 802 | the changelog from the rest of the patch. The version information is |
| 803 | not part of the changelog which gets committed to the git tree. It is |
| 804 | additional information for the reviewers. If it's placed above the |
| 805 | commit tags, it needs manual interaction to remove it. If it is below |
| 806 | the separator line, it gets automatically stripped off when applying the |
| 807 | patch:: |
| 808 | |
| 809 | <commit message> |
| 810 | ... |
| 811 | Signed-off-by: Author <author@mail> |
| 812 | --- |
| 813 | V2 -> V3: Removed redundant helper function |
| 814 | V1 -> V2: Cleaned up coding style and addressed review comments |
| 815 | |
| 816 | path/to/file | 5+++-- |
| 817 | ... |
| 818 | |
| 819 | See more details on the proper patch format in the following |
| 820 | references. |
| 821 | |
| 822 | .. _explicit_in_reply_to: |
| 823 | |
| 824 | Explicit In-Reply-To headers |
| 825 | ---------------------------- |
| 826 | |
| 827 | It can be helpful to manually add In-Reply-To: headers to a patch |
| 828 | (e.g., when using ``git send-email``) to associate the patch with |
| 829 | previous relevant discussion, e.g. to link a bug fix to the email with |
| 830 | the bug report. However, for a multi-patch series, it is generally |
| 831 | best to avoid using In-Reply-To: to link to older versions of the |
| 832 | series. This way multiple versions of the patch don't become an |
| 833 | unmanageable forest of references in email clients. If a link is |
| 834 | helpful, you can use the https://lore.kernel.org/ redirector (e.g., in |
| 835 | the cover email text) to link to an earlier version of the patch series. |
| 836 | |
| 837 | |
| 838 | Providing base tree information |
| 839 | ------------------------------- |
| 840 | |
| 841 | When other developers receive your patches and start the review process, |
| 842 | it is absolutely necessary for them to know what is the base |
| 843 | commit/branch your work applies on, considering the sheer amount of |
| 844 | maintainer trees present nowadays. Note again the **T:** entry in the |
| 845 | MAINTAINERS file explained above. |
| 846 | |
| 847 | This is even more important for automated CI processes that attempt to |
| 848 | run a series of tests in order to establish the quality of your |
| 849 | submission before the maintainer starts the review. |
| 850 | |
| 851 | If you are using ``git format-patch`` to generate your patches, you can |
| 852 | automatically include the base tree information in your submission by |
| 853 | using the ``--base`` flag. The easiest and most convenient way to use |
| 854 | this option is with topical branches:: |
| 855 | |
| 856 | $ git checkout -t -b my-topical-branch master |
| 857 | Branch 'my-topical-branch' set up to track local branch 'master'. |
| 858 | Switched to a new branch 'my-topical-branch' |
| 859 | |
| 860 | [perform your edits and commits] |
| 861 | |
| 862 | $ git format-patch --base=auto --cover-letter -o outgoing/ master |
| 863 | outgoing/0000-cover-letter.patch |
| 864 | outgoing/0001-First-Commit.patch |
| 865 | outgoing/... |
| 866 | |
| 867 | When you open ``outgoing/0000-cover-letter.patch`` for editing, you will |
| 868 | notice that it will have the ``base-commit:`` trailer at the very |
| 869 | bottom, which provides the reviewer and the CI tools enough information |
| 870 | to properly perform ``git am`` without worrying about conflicts:: |
| 871 | |
| 872 | $ git checkout -b patch-review [base-commit-id] |
| 873 | Switched to a new branch 'patch-review' |
| 874 | $ git am patches.mbox |
| 875 | Applying: First Commit |
| 876 | Applying: ... |
| 877 | |
| 878 | Please see ``man git-format-patch`` for more information about this |
| 879 | option. |
| 880 | |
| 881 | .. note:: |
| 882 | |
| 883 | The ``--base`` feature was introduced in git version 2.9.0. |
| 884 | |
| 885 | If you are not using git to format your patches, you can still include |
| 886 | the same ``base-commit`` trailer to indicate the commit hash of the tree |
| 887 | on which your work is based. You should add it either in the cover |
| 888 | letter or in the first patch of the series and it should be placed |
| 889 | either below the ``---`` line or at the very bottom of all other |
| 890 | content, right before your email signature. |
| 891 | |
| 892 | Make sure that base commit is in an official maintainer/mainline tree |
| 893 | and not in some internal, accessible only to you tree - otherwise it |
| 894 | would be worthless. |
| 895 | |
| 896 | Tooling |
| 897 | ------- |
| 898 | |
| 899 | Many of the technical aspects of this process can be automated using |
| 900 | b4, documented at <https://b4.docs.kernel.org/en/latest/>. This can |
| 901 | help with things like tracking dependencies, running checkpatch and |
| 902 | with formatting and sending mails. |
| 903 | |
| 904 | References |
| 905 | ---------- |
| 906 | |
| 907 | Andrew Morton, "The perfect patch" (tpp). |
| 908 | <https://www.ozlabs.org/~akpm/stuff/tpp.txt> |
| 909 | |
| 910 | Jeff Garzik, "Linux kernel patch submission format". |
| 911 | <https://web.archive.org/web/20180829112450/http://linux.yyz.us/patch-format.html> |
| 912 | |
| 913 | Greg Kroah-Hartman, "How to piss off a kernel subsystem maintainer". |
| 914 | <http://www.kroah.com/log/linux/maintainer.html> |
| 915 | |
| 916 | <http://www.kroah.com/log/linux/maintainer-02.html> |
| 917 | |
| 918 | <http://www.kroah.com/log/linux/maintainer-03.html> |
| 919 | |
| 920 | <http://www.kroah.com/log/linux/maintainer-04.html> |
| 921 | |
| 922 | <http://www.kroah.com/log/linux/maintainer-05.html> |
| 923 | |
| 924 | <http://www.kroah.com/log/linux/maintainer-06.html> |
| 925 | |
| 926 | Kernel Documentation/process/coding-style.rst |
| 927 | |
| 928 | Linus Torvalds's mail on the canonical patch format: |
| 929 | <https://lore.kernel.org/r/Pine.LNX.4.58.0504071023190.28951@ppc970.osdl.org> |
| 930 | |
| 931 | Andi Kleen, "On submitting kernel patches" |
| 932 | Some strategies to get difficult or controversial changes in. |
| 933 | |
| 934 | http://halobates.de/on-submitting-patches.pdf |