Commit | Line | Data |
---|---|---|
31c9d7c8 TG |
1 | .. SPDX-License-Identifier: GPL-2.0 |
2 | ||
3 | The tip tree handbook | |
4 | ===================== | |
5 | ||
6 | What is the tip tree? | |
7 | --------------------- | |
8 | ||
9 | The tip tree is a collection of several subsystems and areas of | |
10 | development. The tip tree is both a direct development tree and a | |
11 | aggregation tree for several sub-maintainer trees. The tip tree gitweb URL | |
12 | is: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git | |
13 | ||
14 | The tip tree contains the following subsystems: | |
15 | ||
16 | - **x86 architecture** | |
17 | ||
18 | The x86 architecture development takes place in the tip tree except | |
19 | for the x86 KVM and XEN specific parts which are maintained in the | |
20 | corresponding subsystems and routed directly to mainline from | |
21 | there. It's still good practice to Cc the x86 maintainers on | |
22 | x86-specific KVM and XEN patches. | |
23 | ||
24 | Some x86 subsystems have their own maintainers in addition to the | |
25 | overall x86 maintainers. Please Cc the overall x86 maintainers on | |
26 | patches touching files in arch/x86 even when they are not called out | |
27 | by the MAINTAINER file. | |
28 | ||
29 | Note, that ``x86@kernel.org`` is not a mailing list. It is merely a | |
30 | mail alias which distributes mails to the x86 top-level maintainer | |
31 | team. Please always Cc the Linux Kernel mailing list (LKML) | |
32 | ``linux-kernel@vger.kernel.org``, otherwise your mail ends up only in | |
33 | the private inboxes of the maintainers. | |
34 | ||
35 | - **Scheduler** | |
36 | ||
37 | Scheduler development takes place in the -tip tree, in the | |
38 | sched/core branch - with occasional sub-topic trees for | |
39 | work-in-progress patch-sets. | |
40 | ||
41 | - **Locking and atomics** | |
42 | ||
43 | Locking development (including atomics and other synchronization | |
44 | primitives that are connected to locking) takes place in the -tip | |
45 | tree, in the locking/core branch - with occasional sub-topic trees | |
46 | for work-in-progress patch-sets. | |
47 | ||
48 | - **Generic interrupt subsystem and interrupt chip drivers**: | |
49 | ||
50 | - interrupt core development happens in the irq/core branch | |
51 | ||
52 | - interrupt chip driver development also happens in the irq/core | |
53 | branch, but the patches are usually applied in a separate maintainer | |
54 | tree and then aggregated into irq/core | |
55 | ||
56 | - **Time, timers, timekeeping, NOHZ and related chip drivers**: | |
57 | ||
58 | - timekeeping, clocksource core, NTP and alarmtimer development | |
59 | happens in the timers/core branch, but patches are usually applied in | |
60 | a separate maintainer tree and then aggregated into timers/core | |
61 | ||
62 | - clocksource/event driver development happens in the timers/core | |
63 | branch, but patches are mostly applied in a separate maintainer tree | |
64 | and then aggregated into timers/core | |
65 | ||
66 | - **Performance counters core, architecture support and tooling**: | |
67 | ||
68 | - perf core and architecture support development happens in the | |
69 | perf/core branch | |
70 | ||
71 | - perf tooling development happens in the perf tools maintainer | |
72 | tree and is aggregated into the tip tree. | |
73 | ||
74 | - **CPU hotplug core** | |
75 | ||
76 | - **RAS core** | |
77 | ||
78 | Mostly x86-specific RAS patches are collected in the tip ras/core | |
79 | branch. | |
80 | ||
81 | - **EFI core** | |
82 | ||
83 | EFI development in the efi git tree. The collected patches are | |
84 | aggregated in the tip efi/core branch. | |
85 | ||
86 | - **RCU** | |
87 | ||
88 | RCU development happens in the linux-rcu tree. The resulting changes | |
89 | are aggregated into the tip core/rcu branch. | |
90 | ||
91 | - **Various core code components**: | |
92 | ||
93 | - debugobjects | |
94 | ||
95 | - objtool | |
96 | ||
97 | - random bits and pieces | |
98 | ||
99 | ||
100 | Patch submission notes | |
101 | ---------------------- | |
102 | ||
103 | Selecting the tree/branch | |
104 | ^^^^^^^^^^^^^^^^^^^^^^^^^ | |
105 | ||
106 | In general, development against the head of the tip tree master branch is | |
107 | fine, but for the subsystems which are maintained separately, have their | |
108 | own git tree and are only aggregated into the tip tree, development should | |
109 | take place against the relevant subsystem tree or branch. | |
110 | ||
111 | Bug fixes which target mainline should always be applicable against the | |
112 | mainline kernel tree. Potential conflicts against changes which are already | |
113 | queued in the tip tree are handled by the maintainers. | |
114 | ||
115 | Patch subject | |
116 | ^^^^^^^^^^^^^ | |
117 | ||
118 | The tip tree preferred format for patch subject prefixes is | |
119 | 'subsys/component:', e.g. 'x86/apic:', 'x86/mm/fault:', 'sched/fair:', | |
120 | 'genirq/core:'. Please do not use file names or complete file paths as | |
121 | prefix. 'git log path/to/file' should give you a reasonable hint in most | |
122 | cases. | |
123 | ||
124 | The condensed patch description in the subject line should start with a | |
125 | uppercase letter and should be written in imperative tone. | |
126 | ||
127 | ||
128 | Changelog | |
129 | ^^^^^^^^^ | |
130 | ||
0c4ff6f6 BS |
131 | The general rules about changelogs in the :ref:`Submitting patches guide |
132 | <describe_changes>`, apply. | |
31c9d7c8 TG |
133 | |
134 | The tip tree maintainers set value on following these rules, especially on | |
135 | the request to write changelogs in imperative mood and not impersonating | |
136 | code or the execution of it. This is not just a whim of the | |
137 | maintainers. Changelogs written in abstract words are more precise and | |
138 | tend to be less confusing than those written in the form of novels. | |
139 | ||
140 | It's also useful to structure the changelog into several paragraphs and not | |
141 | lump everything together into a single one. A good structure is to explain | |
142 | the context, the problem and the solution in separate paragraphs and this | |
143 | order. | |
144 | ||
145 | Examples for illustration: | |
146 | ||
147 | Example 1:: | |
148 | ||
149 | x86/intel_rdt/mbm: Fix MBM overflow handler during hot cpu | |
150 | ||
151 | When a CPU is dying, we cancel the worker and schedule a new worker on a | |
152 | different CPU on the same domain. But if the timer is already about to | |
153 | expire (say 0.99s) then we essentially double the interval. | |
154 | ||
155 | We modify the hot cpu handling to cancel the delayed work on the dying | |
156 | cpu and run the worker immediately on a different cpu in same domain. We | |
157 | donot flush the worker because the MBM overflow worker reschedules the | |
158 | worker on same CPU and scans the domain->cpu_mask to get the domain | |
159 | pointer. | |
160 | ||
161 | Improved version:: | |
162 | ||
163 | x86/intel_rdt/mbm: Fix MBM overflow handler during CPU hotplug | |
164 | ||
165 | When a CPU is dying, the overflow worker is canceled and rescheduled on a | |
166 | different CPU in the same domain. But if the timer is already about to | |
167 | expire this essentially doubles the interval which might result in a non | |
168 | detected overflow. | |
169 | ||
170 | Cancel the overflow worker and reschedule it immediately on a different CPU | |
171 | in the same domain. The work could be flushed as well, but that would | |
172 | reschedule it on the same CPU. | |
173 | ||
174 | Example 2:: | |
175 | ||
176 | time: POSIX CPU timers: Ensure that variable is initialized | |
177 | ||
178 | If cpu_timer_sample_group returns -EINVAL, it will not have written into | |
179 | *sample. Checking for cpu_timer_sample_group's return value precludes the | |
180 | potential use of an uninitialized value of now in the following block. | |
181 | Given an invalid clock_idx, the previous code could otherwise overwrite | |
182 | *oldval in an undefined manner. This is now prevented. We also exploit | |
183 | short-circuiting of && to sample the timer only if the result will | |
184 | actually be used to update *oldval. | |
185 | ||
186 | Improved version:: | |
187 | ||
188 | posix-cpu-timers: Make set_process_cpu_timer() more robust | |
189 | ||
190 | Because the return value of cpu_timer_sample_group() is not checked, | |
191 | compilers and static checkers can legitimately warn about a potential use | |
192 | of the uninitialized variable 'now'. This is not a runtime issue as all | |
193 | call sites hand in valid clock ids. | |
194 | ||
195 | Also cpu_timer_sample_group() is invoked unconditionally even when the | |
196 | result is not used because *oldval is NULL. | |
197 | ||
198 | Make the invocation conditional and check the return value. | |
199 | ||
200 | Example 3:: | |
201 | ||
202 | The entity can also be used for other purposes. | |
203 | ||
204 | Let's rename it to be more generic. | |
205 | ||
206 | Improved version:: | |
207 | ||
208 | The entity can also be used for other purposes. | |
209 | ||
210 | Rename it to be more generic. | |
211 | ||
212 | ||
213 | For complex scenarios, especially race conditions and memory ordering | |
214 | issues, it is valuable to depict the scenario with a table which shows | |
215 | the parallelism and the temporal order of events. Here is an example:: | |
216 | ||
217 | CPU0 CPU1 | |
218 | free_irq(X) interrupt X | |
219 | spin_lock(desc->lock) | |
220 | wake irq thread() | |
221 | spin_unlock(desc->lock) | |
222 | spin_lock(desc->lock) | |
223 | remove action() | |
224 | shutdown_irq() | |
225 | release_resources() thread_handler() | |
226 | spin_unlock(desc->lock) access released resources. | |
227 | ^^^^^^^^^^^^^^^^^^^^^^^^^ | |
228 | synchronize_irq() | |
229 | ||
230 | Lockdep provides similar useful output to depict a possible deadlock | |
231 | scenario:: | |
232 | ||
233 | CPU0 CPU1 | |
234 | rtmutex_lock(&rcu->rt_mutex) | |
235 | spin_lock(&rcu->rt_mutex.wait_lock) | |
236 | local_irq_disable() | |
237 | spin_lock(&timer->it_lock) | |
238 | spin_lock(&rcu->mutex.wait_lock) | |
239 | --> Interrupt | |
240 | spin_lock(&timer->it_lock) | |
241 | ||
242 | ||
243 | Function references in changelogs | |
244 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
245 | ||
246 | When a function is mentioned in the changelog, either the text body or the | |
247 | subject line, please use the format 'function_name()'. Omitting the | |
248 | brackets after the function name can be ambiguous:: | |
249 | ||
250 | Subject: subsys/component: Make reservation_count static | |
251 | ||
252 | reservation_count is only used in reservation_stats. Make it static. | |
253 | ||
254 | The variant with brackets is more precise:: | |
255 | ||
256 | Subject: subsys/component: Make reservation_count() static | |
257 | ||
258 | reservation_count() is only called from reservation_stats(). Make it | |
259 | static. | |
260 | ||
261 | ||
262 | Backtraces in changelogs | |
263 | ^^^^^^^^^^^^^^^^^^^^^^^^ | |
264 | ||
265 | See :ref:`backtraces`. | |
266 | ||
267 | Ordering of commit tags | |
268 | ^^^^^^^^^^^^^^^^^^^^^^^ | |
269 | ||
270 | To have a uniform view of the commit tags, the tip maintainers use the | |
271 | following tag ordering scheme: | |
272 | ||
273 | - Fixes: 12char-SHA1 ("sub/sys: Original subject line") | |
274 | ||
275 | A Fixes tag should be added even for changes which do not need to be | |
276 | backported to stable kernels, i.e. when addressing a recently introduced | |
277 | issue which only affects tip or the current head of mainline. These tags | |
278 | are helpful to identify the original commit and are much more valuable | |
279 | than prominently mentioning the commit which introduced a problem in the | |
280 | text of the changelog itself because they can be automatically | |
281 | extracted. | |
282 | ||
283 | The following example illustrates the difference:: | |
284 | ||
285 | Commit | |
286 | ||
287 | abcdef012345678 ("x86/xxx: Replace foo with bar") | |
288 | ||
289 | left an unused instance of variable foo around. Remove it. | |
290 | ||
291 | Signed-off-by: J.Dev <j.dev@mail> | |
292 | ||
293 | Please say instead:: | |
294 | ||
295 | The recent replacement of foo with bar left an unused instance of | |
296 | variable foo around. Remove it. | |
297 | ||
298 | Fixes: abcdef012345678 ("x86/xxx: Replace foo with bar") | |
299 | Signed-off-by: J.Dev <j.dev@mail> | |
300 | ||
301 | The latter puts the information about the patch into the focus and | |
302 | amends it with the reference to the commit which introduced the issue | |
303 | rather than putting the focus on the original commit in the first place. | |
304 | ||
305 | - Reported-by: ``Reporter <reporter@mail>`` | |
306 | ||
307 | - Originally-by: ``Original author <original-author@mail>`` | |
308 | ||
309 | - Suggested-by: ``Suggester <suggester@mail>`` | |
310 | ||
311 | - Co-developed-by: ``Co-author <co-author@mail>`` | |
312 | ||
313 | Signed-off: ``Co-author <co-author@mail>`` | |
314 | ||
315 | Note, that Co-developed-by and Signed-off-by of the co-author(s) must | |
316 | come in pairs. | |
317 | ||
318 | - Signed-off-by: ``Author <author@mail>`` | |
319 | ||
320 | The first Signed-off-by (SOB) after the last Co-developed-by/SOB pair is the | |
321 | author SOB, i.e. the person flagged as author by git. | |
322 | ||
323 | - Signed-off-by: ``Patch handler <handler@mail>`` | |
324 | ||
325 | SOBs after the author SOB are from people handling and transporting | |
326 | the patch, but were not involved in development. SOB chains should | |
327 | reflect the **real** route a patch took as it was propagated to us, | |
328 | with the first SOB entry signalling primary authorship of a single | |
329 | author. Acks should be given as Acked-by lines and review approvals | |
330 | as Reviewed-by lines. | |
331 | ||
332 | If the handler made modifications to the patch or the changelog, then | |
333 | this should be mentioned **after** the changelog text and **above** | |
334 | all commit tags in the following format:: | |
335 | ||
336 | ... changelog text ends. | |
337 | ||
338 | [ handler: Replaced foo by bar and updated changelog ] | |
339 | ||
340 | First-tag: ..... | |
341 | ||
342 | Note the two empty new lines which separate the changelog text and the | |
343 | commit tags from that notice. | |
344 | ||
345 | If a patch is sent to the mailing list by a handler then the author has | |
346 | to be noted in the first line of the changelog with:: | |
347 | ||
348 | From: Author <author@mail> | |
349 | ||
350 | Changelog text starts here.... | |
351 | ||
352 | so the authorship is preserved. The 'From:' line has to be followed | |
353 | by a empty newline. If that 'From:' line is missing, then the patch | |
354 | would be attributed to the person who sent (transported, handled) it. | |
355 | The 'From:' line is automatically removed when the patch is applied | |
356 | and does not show up in the final git changelog. It merely affects | |
357 | the authorship information of the resulting Git commit. | |
358 | ||
359 | - Tested-by: ``Tester <tester@mail>`` | |
360 | ||
361 | - Reviewed-by: ``Reviewer <reviewer@mail>`` | |
362 | ||
363 | - Acked-by: ``Acker <acker@mail>`` | |
364 | ||
365 | - Cc: ``cc-ed-person <person@mail>`` | |
366 | ||
367 | If the patch should be backported to stable, then please add a '``Cc: | |
368 | stable@vger.kernel.org``' tag, but do not Cc stable when sending your | |
369 | mail. | |
370 | ||
371 | - Link: ``https://link/to/information`` | |
372 | ||
373 | For referring to an email on LKML or other kernel mailing lists, | |
a9d85efb | 374 | please use the lore.kernel.org redirector URL:: |
31c9d7c8 | 375 | |
a9d85efb | 376 | https://lore.kernel.org/r/email-message@id |
31c9d7c8 TG |
377 | |
378 | The kernel.org redirector is considered a stable URL, unlike other email | |
379 | archives. | |
380 | ||
381 | Maintainers will add a Link tag referencing the email of the patch | |
382 | submission when they apply a patch to the tip tree. This tag is useful | |
383 | for later reference and is also used for commit notifications. | |
384 | ||
385 | Please do not use combined tags, e.g. ``Reported-and-tested-by``, as | |
386 | they just complicate automated extraction of tags. | |
387 | ||
388 | ||
389 | Links to documentation | |
390 | ^^^^^^^^^^^^^^^^^^^^^^ | |
391 | ||
392 | Providing links to documentation in the changelog is a great help to later | |
393 | debugging and analysis. Unfortunately, URLs often break very quickly | |
394 | because companies restructure their websites frequently. Non-'volatile' | |
395 | exceptions include the Intel SDM and the AMD APM. | |
396 | ||
397 | Therefore, for 'volatile' documents, please create an entry in the kernel | |
398 | bugzilla https://bugzilla.kernel.org and attach a copy of these documents | |
399 | to the bugzilla entry. Finally, provide the URL of the bugzilla entry in | |
400 | the changelog. | |
401 | ||
402 | Patch resend or reminders | |
403 | ^^^^^^^^^^^^^^^^^^^^^^^^^ | |
404 | ||
405 | See :ref:`resend_reminders`. | |
406 | ||
407 | Merge window | |
408 | ^^^^^^^^^^^^ | |
409 | ||
410 | Please do not expect large patch series to be handled during the merge | |
411 | window or even during the week before. Such patches should be submitted in | |
412 | mergeable state *at* *least* a week before the merge window opens. | |
413 | Exceptions are made for bug fixes and *sometimes* for small standalone | |
414 | drivers for new hardware or minimally invasive patches for hardware | |
415 | enablement. | |
416 | ||
417 | During the merge window, the maintainers instead focus on following the | |
418 | upstream changes, fixing merge window fallout, collecting bug fixes, and | |
419 | allowing themselves a breath. Please respect that. | |
420 | ||
421 | The release candidate -rc1 is the starting point for new patches to be | |
422 | applied which are targeted for the next merge window. | |
423 | ||
4f119255 CK |
424 | So called _urgent_ branches will be merged into mainline during the |
425 | stabilization phase of each release. | |
426 | ||
31c9d7c8 TG |
427 | |
428 | Git | |
429 | ^^^ | |
430 | ||
431 | The tip maintainers accept git pull requests from maintainers who provide | |
432 | subsystem changes for aggregation in the tip tree. | |
433 | ||
434 | Pull requests for new patch submissions are usually not accepted and do not | |
435 | replace proper patch submission to the mailing list. The main reason for | |
436 | this is that the review workflow is email based. | |
437 | ||
438 | If you submit a larger patch series it is helpful to provide a git branch | |
439 | in a private repository which allows interested people to easily pull the | |
440 | series for testing. The usual way to offer this is a git URL in the cover | |
441 | letter of the patch series. | |
442 | ||
9b5a7f4a DH |
443 | Testing |
444 | ^^^^^^^ | |
445 | ||
446 | Code should be tested before submitting to the tip maintainers. Anything | |
447 | other than minor changes should be built, booted and tested with | |
448 | comprehensive (and heavyweight) kernel debugging options enabled. | |
449 | ||
450 | These debugging options can be found in kernel/configs/x86_debug.config | |
451 | and can be added to an existing kernel config by running: | |
452 | ||
453 | make x86_debug.config | |
454 | ||
455 | Some of these options are x86-specific and can be left out when testing | |
456 | on other architectures. | |
31c9d7c8 | 457 | |
b7dac767 SC |
458 | .. _maintainer-tip-coding-style: |
459 | ||
31c9d7c8 TG |
460 | Coding style notes |
461 | ------------------ | |
462 | ||
463 | Comment style | |
464 | ^^^^^^^^^^^^^ | |
465 | ||
466 | Sentences in comments start with an uppercase letter. | |
467 | ||
468 | Single line comments:: | |
469 | ||
470 | /* This is a single line comment */ | |
471 | ||
472 | Multi-line comments:: | |
473 | ||
474 | /* | |
475 | * This is a properly formatted | |
476 | * multi-line comment. | |
477 | * | |
478 | * Larger multi-line comments should be split into paragraphs. | |
479 | */ | |
480 | ||
481 | No tail comments: | |
482 | ||
483 | Please refrain from using tail comments. Tail comments disturb the | |
484 | reading flow in almost all contexts, but especially in code:: | |
485 | ||
486 | if (somecondition_is_true) /* Don't put a comment here */ | |
487 | dostuff(); /* Neither here */ | |
488 | ||
489 | seed = MAGIC_CONSTANT; /* Nor here */ | |
490 | ||
491 | Use freestanding comments instead:: | |
492 | ||
493 | /* This condition is not obvious without a comment */ | |
494 | if (somecondition_is_true) { | |
495 | /* This really needs to be documented */ | |
496 | dostuff(); | |
497 | } | |
498 | ||
499 | /* This magic initialization needs a comment. Maybe not? */ | |
500 | seed = MAGIC_CONSTANT; | |
501 | ||
502 | Comment the important things: | |
503 | ||
504 | Comments should be added where the operation is not obvious. Documenting | |
505 | the obvious is just a distraction:: | |
506 | ||
507 | /* Decrement refcount and check for zero */ | |
508 | if (refcount_dec_and_test(&p->refcnt)) { | |
509 | do; | |
510 | lots; | |
511 | of; | |
512 | magic; | |
513 | things; | |
514 | } | |
515 | ||
516 | Instead, comments should explain the non-obvious details and document | |
517 | constraints:: | |
518 | ||
519 | if (refcount_dec_and_test(&p->refcnt)) { | |
520 | /* | |
521 | * Really good explanation why the magic things below | |
522 | * need to be done, ordering and locking constraints, | |
523 | * etc.. | |
524 | */ | |
525 | do; | |
526 | lots; | |
527 | of; | |
528 | magic; | |
529 | /* Needs to be the last operation because ... */ | |
530 | things; | |
531 | } | |
532 | ||
533 | Function documentation comments: | |
534 | ||
535 | To document functions and their arguments please use kernel-doc format | |
536 | and not free form comments:: | |
537 | ||
538 | /** | |
539 | * magic_function - Do lots of magic stuff | |
540 | * @magic: Pointer to the magic data to operate on | |
541 | * @offset: Offset in the data array of @magic | |
542 | * | |
543 | * Deep explanation of mysterious things done with @magic along | |
544 | * with documentation of the return values. | |
545 | * | |
546 | * Note, that the argument descriptors above are arranged | |
547 | * in a tabular fashion. | |
548 | */ | |
549 | ||
550 | This applies especially to globally visible functions and inline | |
551 | functions in public header files. It might be overkill to use kernel-doc | |
552 | format for every (static) function which needs a tiny explanation. The | |
553 | usage of descriptive function names often replaces these tiny comments. | |
554 | Apply common sense as always. | |
555 | ||
556 | ||
557 | Documenting locking requirements | |
558 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
559 | Documenting locking requirements is a good thing, but comments are not | |
560 | necessarily the best choice. Instead of writing:: | |
561 | ||
562 | /* Caller must hold foo->lock */ | |
563 | void func(struct foo *foo) | |
564 | { | |
565 | ... | |
566 | } | |
567 | ||
568 | Please use:: | |
569 | ||
570 | void func(struct foo *foo) | |
571 | { | |
572 | lockdep_assert_held(&foo->lock); | |
573 | ... | |
574 | } | |
575 | ||
576 | In PROVE_LOCKING kernels, lockdep_assert_held() emits a warning | |
577 | if the caller doesn't hold the lock. Comments can't do that. | |
578 | ||
579 | Bracket rules | |
580 | ^^^^^^^^^^^^^ | |
581 | ||
582 | Brackets should be omitted only if the statement which follows 'if', 'for', | |
583 | 'while' etc. is truly a single line:: | |
584 | ||
585 | if (foo) | |
586 | do_something(); | |
587 | ||
588 | The following is not considered to be a single line statement even | |
589 | though C does not require brackets:: | |
590 | ||
591 | for (i = 0; i < end; i++) | |
592 | if (foo[i]) | |
593 | do_something(foo[i]); | |
594 | ||
595 | Adding brackets around the outer loop enhances the reading flow:: | |
596 | ||
597 | for (i = 0; i < end; i++) { | |
598 | if (foo[i]) | |
599 | do_something(foo[i]); | |
600 | } | |
601 | ||
602 | ||
603 | Variable declarations | |
604 | ^^^^^^^^^^^^^^^^^^^^^ | |
605 | ||
606 | The preferred ordering of variable declarations at the beginning of a | |
607 | function is reverse fir tree order:: | |
608 | ||
609 | struct long_struct_name *descriptive_name; | |
610 | unsigned long foo, bar; | |
611 | unsigned int tmp; | |
612 | int ret; | |
613 | ||
614 | The above is faster to parse than the reverse ordering:: | |
615 | ||
616 | int ret; | |
617 | unsigned int tmp; | |
618 | unsigned long foo, bar; | |
619 | struct long_struct_name *descriptive_name; | |
620 | ||
621 | And even more so than random ordering:: | |
622 | ||
623 | unsigned long foo, bar; | |
624 | int ret; | |
625 | struct long_struct_name *descriptive_name; | |
626 | unsigned int tmp; | |
627 | ||
628 | Also please try to aggregate variables of the same type into a single | |
629 | line. There is no point in wasting screen space:: | |
630 | ||
631 | unsigned long a; | |
632 | unsigned long b; | |
633 | unsigned long c; | |
634 | unsigned long d; | |
635 | ||
636 | It's really sufficient to do:: | |
637 | ||
638 | unsigned long a, b, c, d; | |
639 | ||
640 | Please also refrain from introducing line splits in variable declarations:: | |
641 | ||
642 | struct long_struct_name *descriptive_name = container_of(bar, | |
643 | struct long_struct_name, | |
644 | member); | |
645 | struct foobar foo; | |
646 | ||
647 | It's way better to move the initialization to a separate line after the | |
648 | declarations:: | |
649 | ||
650 | struct long_struct_name *descriptive_name; | |
651 | struct foobar foo; | |
652 | ||
653 | descriptive_name = container_of(bar, struct long_struct_name, member); | |
654 | ||
655 | ||
656 | Variable types | |
657 | ^^^^^^^^^^^^^^ | |
658 | ||
659 | Please use the proper u8, u16, u32, u64 types for variables which are meant | |
660 | to describe hardware or are used as arguments for functions which access | |
661 | hardware. These types are clearly defining the bit width and avoid | |
662 | truncation, expansion and 32/64-bit confusion. | |
663 | ||
664 | u64 is also recommended in code which would become ambiguous for 32-bit | |
665 | kernels when 'unsigned long' would be used instead. While in such | |
666 | situations 'unsigned long long' could be used as well, u64 is shorter | |
667 | and also clearly shows that the operation is required to be 64 bits wide | |
668 | independent of the target CPU. | |
669 | ||
670 | Please use 'unsigned int' instead of 'unsigned'. | |
671 | ||
672 | ||
673 | Constants | |
674 | ^^^^^^^^^ | |
675 | ||
676 | Please do not use literal (hexa)decimal numbers in code or initializers. | |
677 | Either use proper defines which have descriptive names or consider using | |
678 | an enum. | |
679 | ||
680 | ||
681 | Struct declarations and initializers | |
682 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |
683 | ||
684 | Struct declarations should align the struct member names in a tabular | |
685 | fashion:: | |
686 | ||
687 | struct bar_order { | |
688 | unsigned int guest_id; | |
689 | int ordered_item; | |
690 | struct menu *menu; | |
691 | }; | |
692 | ||
693 | Please avoid documenting struct members within the declaration, because | |
694 | this often results in strangely formatted comments and the struct members | |
695 | become obfuscated:: | |
696 | ||
697 | struct bar_order { | |
698 | unsigned int guest_id; /* Unique guest id */ | |
699 | int ordered_item; | |
700 | /* Pointer to a menu instance which contains all the drinks */ | |
701 | struct menu *menu; | |
702 | }; | |
703 | ||
704 | Instead, please consider using the kernel-doc format in a comment preceding | |
705 | the struct declaration, which is easier to read and has the added advantage | |
706 | of including the information in the kernel documentation, for example, as | |
707 | follows:: | |
708 | ||
709 | ||
710 | /** | |
711 | * struct bar_order - Description of a bar order | |
712 | * @guest_id: Unique guest id | |
713 | * @ordered_item: The item number from the menu | |
714 | * @menu: Pointer to the menu from which the item | |
715 | * was ordered | |
716 | * | |
717 | * Supplementary information for using the struct. | |
718 | * | |
719 | * Note, that the struct member descriptors above are arranged | |
720 | * in a tabular fashion. | |
721 | */ | |
722 | struct bar_order { | |
723 | unsigned int guest_id; | |
724 | int ordered_item; | |
725 | struct menu *menu; | |
726 | }; | |
727 | ||
728 | Static struct initializers must use C99 initializers and should also be | |
729 | aligned in a tabular fashion:: | |
730 | ||
731 | static struct foo statfoo = { | |
732 | .a = 0, | |
733 | .plain_integer = CONSTANT_DEFINE_OR_ENUM, | |
734 | .bar = &statbar, | |
735 | }; | |
736 | ||
737 | Note that while C99 syntax allows the omission of the final comma, | |
738 | we recommend the use of a comma on the last line because it makes | |
739 | reordering and addition of new lines easier, and makes such future | |
740 | patches slightly easier to read as well. | |
741 | ||
742 | Line breaks | |
743 | ^^^^^^^^^^^ | |
744 | ||
745 | Restricting line length to 80 characters makes deeply indented code hard to | |
746 | read. Consider breaking out code into helper functions to avoid excessive | |
747 | line breaking. | |
748 | ||
749 | The 80 character rule is not a strict rule, so please use common sense when | |
750 | breaking lines. Especially format strings should never be broken up. | |
751 | ||
752 | When splitting function declarations or function calls, then please align | |
753 | the first argument in the second line with the first argument in the first | |
754 | line:: | |
755 | ||
756 | static int long_function_name(struct foobar *barfoo, unsigned int id, | |
757 | unsigned int offset) | |
758 | { | |
759 | ||
760 | if (!id) { | |
761 | ret = longer_function_name(barfoo, DEFAULT_BARFOO_ID, | |
762 | offset); | |
763 | ... | |
764 | ||
765 | Namespaces | |
766 | ^^^^^^^^^^ | |
767 | ||
768 | Function/variable namespaces improve readability and allow easy | |
769 | grepping. These namespaces are string prefixes for globally visible | |
770 | function and variable names, including inlines. These prefixes should | |
771 | combine the subsystem and the component name such as 'x86_comp\_', | |
772 | 'sched\_', 'irq\_', and 'mutex\_'. | |
773 | ||
774 | This also includes static file scope functions that are immediately put | |
775 | into globally visible driver templates - it's useful for those symbols | |
776 | to carry a good prefix as well, for backtrace readability. | |
777 | ||
778 | Namespace prefixes may be omitted for local static functions and | |
779 | variables. Truly local functions, only called by other local functions, | |
780 | can have shorter descriptive names - our primary concern is greppability | |
781 | and backtrace readability. | |
782 | ||
783 | Please note that 'xxx_vendor\_' and 'vendor_xxx_` prefixes are not | |
784 | helpful for static functions in vendor-specific files. After all, it | |
785 | is already clear that the code is vendor-specific. In addition, vendor | |
786 | names should only be for truly vendor-specific functionality. | |
787 | ||
788 | As always apply common sense and aim for consistency and readability. | |
789 | ||
790 | ||
791 | Commit notifications | |
792 | -------------------- | |
793 | ||
794 | The tip tree is monitored by a bot for new commits. The bot sends an email | |
795 | for each new commit to a dedicated mailing list | |
796 | (``linux-tip-commits@vger.kernel.org``) and Cc's all people who are | |
797 | mentioned in one of the commit tags. It uses the email message ID from the | |
798 | Link tag at the end of the tag list to set the In-Reply-To email header so | |
799 | the message is properly threaded with the patch submission email. | |
800 | ||
801 | The tip maintainers and submaintainers try to reply to the submitter | |
802 | when merging a patch, but they sometimes forget or it does not fit the | |
803 | workflow of the moment. While the bot message is purely mechanical, it | |
804 | also implies a 'Thank you! Applied.'. |