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