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