1 .. _development_coding: 2 3 Getting the code right 4 ====================== 5 6 While there is much to be said for a solid and community-oriented design 7 process, the proof of any kernel development project is in the resulting 8 code. It is the code which will be examined by other developers and merged 9 (or not) into the mainline tree. So it is the quality of this code which 10 will determine the ultimate success of the project. 11 12 This section will examine the coding process. We'll start with a look at a 13 number of ways in which kernel developers can go wrong. Then the focus 14 will shift toward doing things right and the tools which can help in that 15 quest. 16 17 18 Pitfalls 19 --------- 20 21 Coding style 22 ************ 23 24 The kernel has long had a standard coding style, described in 25 :ref:`Documentation/process/coding-style.rst <codingstyle>`. For much of 26 that time, the policies described in that file were taken as being, at most, 27 advisory. As a result, there is a substantial amount of code in the kernel 28 which does not meet the coding style guidelines. The presence of that code 29 leads to two independent hazards for kernel developers. 30 31 The first of these is to believe that the kernel coding standards do not 32 matter and are not enforced. The truth of the matter is that adding new 33 code to the kernel is very difficult if that code is not coded according to 34 the standard; many developers will request that the code be reformatted 35 before they will even review it. A code base as large as the kernel 36 requires some uniformity of code to make it possible for developers to 37 quickly understand any part of it. So there is no longer room for 38 strangely-formatted code. 39 40 Occasionally, the kernel's coding style will run into conflict with an 41 employer's mandated style. In such cases, the kernel's style will have to 42 win before the code can be merged. Putting code into the kernel means 43 giving up a degree of control in a number of ways - including control over 44 how the code is formatted. 45 46 The other trap is to assume that code which is already in the kernel is 47 urgently in need of coding style fixes. Developers may start to generate 48 reformatting patches as a way of gaining familiarity with the process, or 49 as a way of getting their name into the kernel changelogs - or both. But 50 pure coding style fixes are seen as noise by the development community; 51 they tend to get a chilly reception. So this type of patch is best 52 avoided. It is natural to fix the style of a piece of code while working 53 on it for other reasons, but coding style changes should not be made for 54 their own sake. 55 56 The coding style document also should not be read as an absolute law which 57 can never be transgressed. If there is a good reason to go against the 58 style (a line which becomes far less readable if split to fit within the 59 80-column limit, for example), just do it. 60 61 Note that you can also use the ``clang-format`` tool to help you with 62 these rules, to quickly re-format parts of your code automatically, 63 and to review full files in order to spot coding style mistakes, 64 typos and possible improvements. It is also handy for sorting ``#includes``, 65 for aligning variables/macros, for reflowing text and other similar tasks. 66 See the file :ref:`Documentation/dev-tools/clang-format.rst <clangformat>` 67 for more details. 68 69 Some basic editor settings, such as indentation and line endings, will be 70 set automatically if you are using an editor that is compatible with 71 EditorConfig. See the official EditorConfig website for more information: 72 https://editorconfig.org/ 73 74 Abstraction layers 75 ****************** 76 77 Computer Science professors teach students to make extensive use of 78 abstraction layers in the name of flexibility and information hiding. 79 Certainly the kernel makes extensive use of abstraction; no project 80 involving several million lines of code could do otherwise and survive. 81 But experience has shown that excessive or premature abstraction can be 82 just as harmful as premature optimization. Abstraction should be used to 83 the level required and no further. 84 85 At a simple level, consider a function which has an argument which is 86 always passed as zero by all callers. One could retain that argument just 87 in case somebody eventually needs to use the extra flexibility that it 88 provides. By that time, though, chances are good that the code which 89 implements this extra argument has been broken in some subtle way which was 90 never noticed - because it has never been used. Or, when the need for 91 extra flexibility arises, it does not do so in a way which matches the 92 programmer's early expectation. Kernel developers will routinely submit 93 patches to remove unused arguments; they should, in general, not be added 94 in the first place. 95 96 Abstraction layers which hide access to hardware - often to allow the bulk 97 of a driver to be used with multiple operating systems - are especially 98 frowned upon. Such layers obscure the code and may impose a performance 99 penalty; they do not belong in the Linux kernel. 100 101 On the other hand, if you find yourself copying significant amounts of code 102 from another kernel subsystem, it is time to ask whether it would, in fact, 103 make sense to pull out some of that code into a separate library or to 104 implement that functionality at a higher level. There is no value in 105 replicating the same code throughout the kernel. 106 107 108 #ifdef and preprocessor use in general 109 ************************************** 110 111 The C preprocessor seems to present a powerful temptation to some C 112 programmers, who see it as a way to efficiently encode a great deal of 113 flexibility into a source file. But the preprocessor is not C, and heavy 114 use of it results in code which is much harder for others to read and 115 harder for the compiler to check for correctness. Heavy preprocessor use 116 is almost always a sign of code which needs some cleanup work. 117 118 Conditional compilation with #ifdef is, indeed, a powerful feature, and it 119 is used within the kernel. But there is little desire to see code which is 120 sprinkled liberally with #ifdef blocks. As a general rule, #ifdef use 121 should be confined to header files whenever possible. 122 Conditionally-compiled code can be confined to functions which, if the code 123 is not to be present, simply become empty. The compiler will then quietly 124 optimize out the call to the empty function. The result is far cleaner 125 code which is easier to follow. 126 127 C preprocessor macros present a number of hazards, including possible 128 multiple evaluation of expressions with side effects and no type safety. 129 If you are tempted to define a macro, consider creating an inline function 130 instead. The code which results will be the same, but inline functions are 131 easier to read, do not evaluate their arguments multiple times, and allow 132 the compiler to perform type checking on the arguments and return value. 133 134 135 Inline functions 136 **************** 137 138 Inline functions present a hazard of their own, though. Programmers can 139 become enamored of the perceived efficiency inherent in avoiding a function 140 call and fill a source file with inline functions. Those functions, 141 however, can actually reduce performance. Since their code is replicated 142 at each call site, they end up bloating the size of the compiled kernel. 143 That, in turn, creates pressure on the processor's memory caches, which can 144 slow execution dramatically. Inline functions, as a rule, should be quite 145 small and relatively rare. The cost of a function call, after all, is not 146 that high; the creation of large numbers of inline functions is a classic 147 example of premature optimization. 148 149 In general, kernel programmers ignore cache effects at their peril. The 150 classic time/space tradeoff taught in beginning data structures classes 151 often does not apply to contemporary hardware. Space *is* time, in that a 152 larger program will run slower than one which is more compact. 153 154 More recent compilers take an increasingly active role in deciding whether 155 a given function should actually be inlined or not. So the liberal 156 placement of "inline" keywords may not just be excessive; it could also be 157 irrelevant. 158 159 160 Locking 161 ******* 162 163 In May, 2006, the "Devicescape" networking stack was, with great 164 fanfare, released under the GPL and made available for inclusion in the 165 mainline kernel. This donation was welcome news; support for wireless 166 networking in Linux was considered substandard at best, and the Devicescape 167 stack offered the promise of fixing that situation. Yet, this code did not 168 actually make it into the mainline until June, 2007 (2.6.22). What 169 happened? 170 171 This code showed a number of signs of having been developed behind 172 corporate doors. But one large problem in particular was that it was not 173 designed to work on multiprocessor systems. Before this networking stack 174 (now called mac80211) could be merged, a locking scheme needed to be 175 retrofitted onto it. 176 177 Once upon a time, Linux kernel code could be developed without thinking 178 about the concurrency issues presented by multiprocessor systems. Now, 179 however, this document is being written on a dual-core laptop. Even on 180 single-processor systems, work being done to improve responsiveness will 181 raise the level of concurrency within the kernel. The days when kernel 182 code could be written without thinking about locking are long past. 183 184 Any resource (data structures, hardware registers, etc.) which could be 185 accessed concurrently by more than one thread must be protected by a lock. 186 New code should be written with this requirement in mind; retrofitting 187 locking after the fact is a rather more difficult task. Kernel developers 188 should take the time to understand the available locking primitives well 189 enough to pick the right tool for the job. Code which shows a lack of 190 attention to concurrency will have a difficult path into the mainline. 191 192 193 Regressions 194 *********** 195 196 One final hazard worth mentioning is this: it can be tempting to make a 197 change (which may bring big improvements) which causes something to break 198 for existing users. This kind of change is called a "regression," and 199 regressions have become most unwelcome in the mainline kernel. With few 200 exceptions, changes which cause regressions will be backed out if the 201 regression cannot be fixed in a timely manner. Far better to avoid the 202 regression in the first place. 203 204 It is often argued that a regression can be justified if it causes things 205 to work for more people than it creates problems for. Why not make a 206 change if it brings new functionality to ten systems for each one it 207 breaks? The best answer to this question was expressed by Linus in July, 208 2007: 209 210 :: 211 212 So we don't fix bugs by introducing new problems. That way lies 213 madness, and nobody ever knows if you actually make any real 214 progress at all. Is it two steps forwards, one step back, or one 215 step forward and two steps back? 216 217 (https://lwn.net/Articles/243460/). 218 219 An especially unwelcome type of regression is any sort of change to the 220 user-space ABI. Once an interface has been exported to user space, it must 221 be supported indefinitely. This fact makes the creation of user-space 222 interfaces particularly challenging: since they cannot be changed in 223 incompatible ways, they must be done right the first time. For this 224 reason, a great deal of thought, clear documentation, and wide review for 225 user-space interfaces is always required. 226 227 228 Code checking tools 229 ------------------- 230 231 For now, at least, the writing of error-free code remains an ideal that few 232 of us can reach. What we can hope to do, though, is to catch and fix as 233 many of those errors as possible before our code goes into the mainline 234 kernel. To that end, the kernel developers have put together an impressive 235 array of tools which can catch a wide variety of obscure problems in an 236 automated way. Any problem caught by the computer is a problem which will 237 not afflict a user later on, so it stands to reason that the automated 238 tools should be used whenever possible. 239 240 The first step is simply to heed the warnings produced by the compiler. 241 Contemporary versions of gcc can detect (and warn about) a large number of 242 potential errors. Quite often, these warnings point to real problems. 243 Code submitted for review should, as a rule, not produce any compiler 244 warnings. When silencing warnings, take care to understand the real cause 245 and try to avoid "fixes" which make the warning go away without addressing 246 its cause. 247 248 Note that not all compiler warnings are enabled by default. Build the 249 kernel with "make KCFLAGS=-W" to get the full set. 250 251 The kernel provides several configuration options which turn on debugging 252 features; most of these are found in the "kernel hacking" submenu. Several 253 of these options should be turned on for any kernel used for development or 254 testing purposes. In particular, you should turn on: 255 256 - FRAME_WARN to get warnings for stack frames larger than a given amount. 257 The output generated can be verbose, but one need not worry about 258 warnings from other parts of the kernel. 259 260 - DEBUG_OBJECTS will add code to track the lifetime of various objects 261 created by the kernel and warn when things are done out of order. If 262 you are adding a subsystem which creates (and exports) complex objects 263 of its own, consider adding support for the object debugging 264 infrastructure. 265 266 - DEBUG_SLAB can find a variety of memory allocation and use errors; it 267 should be used on most development kernels. 268 269 - DEBUG_SPINLOCK, DEBUG_ATOMIC_SLEEP, and DEBUG_MUTEXES will find a 270 number of common locking errors. 271 272 There are quite a few other debugging options, some of which will be 273 discussed below. Some of them have a significant performance impact and 274 should not be used all of the time. But some time spent learning the 275 available options will likely be paid back many times over in short order. 276 277 One of the heavier debugging tools is the locking checker, or "lockdep." 278 This tool will track the acquisition and release of every lock (spinlock or 279 mutex) in the system, the order in which locks are acquired relative to 280 each other, the current interrupt environment, and more. It can then 281 ensure that locks are always acquired in the same order, that the same 282 interrupt assumptions apply in all situations, and so on. In other words, 283 lockdep can find a number of scenarios in which the system could, on rare 284 occasion, deadlock. This kind of problem can be painful (for both 285 developers and users) in a deployed system; lockdep allows them to be found 286 in an automated manner ahead of time. Code with any sort of non-trivial 287 locking should be run with lockdep enabled before being submitted for 288 inclusion. 289 290 As a diligent kernel programmer, you will, beyond doubt, check the return 291 status of any operation (such as a memory allocation) which can fail. The 292 fact of the matter, though, is that the resulting failure recovery paths 293 are, probably, completely untested. Untested code tends to be broken code; 294 you could be much more confident of your code if all those error-handling 295 paths had been exercised a few times. 296 297 The kernel provides a fault injection framework which can do exactly that, 298 especially where memory allocations are involved. With fault injection 299 enabled, a configurable percentage of memory allocations will be made to 300 fail; these failures can be restricted to a specific range of code. 301 Running with fault injection enabled allows the programmer to see how the 302 code responds when things go badly. See 303 Documentation/fault-injection/fault-injection.rst for more information on 304 how to use this facility. 305 306 Other kinds of errors can be found with the "sparse" static analysis tool. 307 With sparse, the programmer can be warned about confusion between 308 user-space and kernel-space addresses, mixture of big-endian and 309 small-endian quantities, the passing of integer values where a set of bit 310 flags is expected, and so on. Sparse must be installed separately (it can 311 be found at https://sparse.wiki.kernel.org/index.php/Main_Page if your 312 distributor does not package it); it can then be run on the code by adding 313 "C=1" to your make command. 314 315 The "Coccinelle" tool (http://coccinelle.lip6.fr/) is able to find a wide 316 variety of potential coding problems; it can also propose fixes for those 317 problems. Quite a few "semantic patches" for the kernel have been packaged 318 under the scripts/coccinelle directory; running "make coccicheck" will run 319 through those semantic patches and report on any problems found. See 320 :ref:`Documentation/dev-tools/coccinelle.rst <devtools_coccinelle>` 321 for more information. 322 323 Other kinds of portability errors are best found by compiling your code for 324 other architectures. If you do not happen to have an S/390 system or a 325 Blackfin development board handy, you can still perform the compilation 326 step. A large set of cross compilers for x86 systems can be found at 327 328 https://www.kernel.org/pub/tools/crosstool/ 329 330 Some time spent installing and using these compilers will help avoid 331 embarrassment later. 332 333 334 Documentation 335 ------------- 336 337 Documentation has often been more the exception than the rule with kernel 338 development. Even so, adequate documentation will help to ease the merging 339 of new code into the kernel, make life easier for other developers, and 340 will be helpful for your users. In many cases, the addition of 341 documentation has become essentially mandatory. 342 343 The first piece of documentation for any patch is its associated 344 changelog. Log entries should describe the problem being solved, the form 345 of the solution, the people who worked on the patch, any relevant 346 effects on performance, and anything else that might be needed to 347 understand the patch. Be sure that the changelog says *why* the patch is 348 worth applying; a surprising number of developers fail to provide that 349 information. 350 351 Any code which adds a new user-space interface - including new sysfs or 352 /proc files - should include documentation of that interface which enables 353 user-space developers to know what they are working with. See 354 Documentation/ABI/README for a description of how this documentation should 355 be formatted and what information needs to be provided. 356 357 The file :ref:`Documentation/admin-guide/kernel-parameters.rst 358 <kernelparameters>` describes all of the kernel's boot-time parameters. 359 Any patch which adds new parameters should add the appropriate entries to 360 this file. 361 362 Any new configuration options must be accompanied by help text which 363 clearly explains the options and when the user might want to select them. 364 365 Internal API information for many subsystems is documented by way of 366 specially-formatted comments; these comments can be extracted and formatted 367 in a number of ways by the "kernel-doc" script. If you are working within 368 a subsystem which has kerneldoc comments, you should maintain them and add 369 them, as appropriate, for externally-available functions. Even in areas 370 which have not been so documented, there is no harm in adding kerneldoc 371 comments for the future; indeed, this can be a useful activity for 372 beginning kernel developers. The format of these comments, along with some 373 information on how to create kerneldoc templates can be found at 374 :ref:`Documentation/doc-guide/ <doc_guide>`. 375 376 Anybody who reads through a significant amount of existing kernel code will 377 note that, often, comments are most notable by their absence. Once again, 378 the expectations for new code are higher than they were in the past; 379 merging uncommented code will be harder. That said, there is little desire 380 for verbosely-commented code. The code should, itself, be readable, with 381 comments explaining the more subtle aspects. 382 383 Certain things should always be commented. Uses of memory barriers should 384 be accompanied by a line explaining why the barrier is necessary. The 385 locking rules for data structures generally need to be explained somewhere. 386 Major data structures need comprehensive documentation in general. 387 Non-obvious dependencies between separate bits of code should be pointed 388 out. Anything which might tempt a code janitor to make an incorrect 389 "cleanup" needs a comment saying why it is done the way it is. And so on. 390 391 392 Internal API changes 393 -------------------- 394 395 The binary interface provided by the kernel to user space cannot be broken 396 except under the most severe circumstances. The kernel's internal 397 programming interfaces, instead, are highly fluid and can be changed when 398 the need arises. If you find yourself having to work around a kernel API, 399 or simply not using a specific functionality because it does not meet your 400 needs, that may be a sign that the API needs to change. As a kernel 401 developer, you are empowered to make such changes. 402 403 There are, of course, some catches. API changes can be made, but they need 404 to be well justified. So any patch making an internal API change should be 405 accompanied by a description of what the change is and why it is 406 necessary. This kind of change should also be broken out into a separate 407 patch, rather than buried within a larger patch. 408 409 The other catch is that a developer who changes an internal API is 410 generally charged with the task of fixing any code within the kernel tree 411 which is broken by the change. For a widely-used function, this duty can 412 lead to literally hundreds or thousands of changes - many of which are 413 likely to conflict with work being done by other developers. Needless to 414 say, this can be a large job, so it is best to be sure that the 415 justification is solid. Note that the Coccinelle tool can help with 416 wide-ranging API changes. 417 418 When making an incompatible API change, one should, whenever possible, 419 ensure that code which has not been updated is caught by the compiler. 420 This will help you to be sure that you have found all in-tree uses of that 421 interface. It will also alert developers of out-of-tree code that there is 422 a change that they need to respond to. Supporting out-of-tree code is not 423 something that kernel developers need to be worried about, but we also do 424 not have to make life harder for out-of-tree developers than it needs to 425 be.
Linux® is a registered trademark of Linus Torvalds in the United States and other countries.
TOMOYO® is a registered trademark of NTT DATA CORPORATION.