[PATCH 1/3] CodingStyle updates

1. Updates chapter 13 (printing kernel messages) to expand on the use of
pr_debug()/pr_info(), what to avoid, and how to hook your debug code with
kernel.h.

2. New chapter 19, branch prediction optimizations, discusses the whole
un/likely issue.

Cc: "Kok, Auke" <auke-jan.h.kok@xxxxxxxxx>
Cc: Kyle Moffett <mrmacman_g4@xxxxxxx>
Cc: Jan Engelhardt <jengelh@xxxxxxxxxxxxxxx>
Cc: Adrian Bunk <bunk@xxxxxxxxxx>
Cc: roel <12o3l@xxxxxxxxxx>

Signed-off-by: Erez Zadok <ezk@xxxxxxxxxxxxx>
---
Documentation/CodingStyle | 88 +++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 86 insertions(+), 2 deletions(-)

diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
index 7f1730f..00b29e4 100644
--- a/Documentation/CodingStyle
+++ b/Documentation/CodingStyle
@@ -643,8 +643,26 @@ Printing numbers in parentheses (%d) adds no value and should be avoided.
There are a number of driver model diagnostic macros in <linux/device.h>
which you should use to make sure messages are matched to the right device
and driver, and are tagged with the right level: dev_err(), dev_warn(),
-dev_info(), and so forth. For messages that aren't associated with a
-particular device, <linux/kernel.h> defines pr_debug() and pr_info().
+dev_info(), and so forth.
+
+A number of people often like to define their own debugging printf's,
+wrapping printk's in #ifdef's that get turned on only when subsystem
+debugging is compiled in (e.g., dprintk, Dprintk, DPRINTK, etc.). Please
+don't reinvent the wheel but use existing mechanisms. For messages that
+aren't associated with a particular device, <linux/kernel.h> defines
+pr_debug() and pr_info(); the latter two translate to printk(KERN_DEBUG) and
+printk(KERN_INFO), respectively. However, to get pr_debug() to actually
+emit the message, you'll need to turn on DEBUG in your code, which can be
+done as follows in your subsystem Makefile:
+
+ifeq ($(CONFIG_WHATEVER_DEBUG),y)
+EXTRA_CFLAGS += -DDEBUG
+endif
+
+In this way, you can create a Kconfig parameter to turn on debugging at
+compile time, which will also turn on DEBUG, to enable pr_debug() to emit
+actual messages; conversely, when CONFIG_WHATEVER_DEBUG is off, DEBUG is
+off, and pr_debug() will display nothing.

Coming up with good debugging messages can be quite a challenge; and once
you have them, they can be a huge help for remote troubleshooting. Such
@@ -779,6 +797,69 @@ includes markers for indentation and mode configuration. People may use their
own custom mode, or may have some other magic method for making indentation
work correctly.

+ Chapter 19: branch prediction optimizations
+
+The kernel includes macros called likely() and unlikely(), which can be used
+as hints to the compiler to optimize branch prediction. They operate by
+asking gcc to shuffle the code around so that the more favorable outcome
+executes linearly, avoiding a JMP instruction; this can improve cache
+pipeline efficiency. For technical details how these macros work, see the
+References section at the end of this document.
+
+An example use of this as as follows:
+
+ ptr = kmalloc(size, GFP_KERNEL);
+ if (unlikely(!ptr))
+ ...
+
+or
+ err = some_function(...);
+ if (likely(!err))
+ ...
+
+The main two problems with using un/likely() are that (a) programmers can
+easily be wrong about their code's likelihood to take one branch
+vs. another, and (b) on average, gcc will do a much better job optimizing
+branches that the programmer can. The benefit on some systems for
+predicting correctly can be in saving a few instructions. But the penalty
+for wrong use of un/likely() can be very significant (possibly dozens of
+instructions), as you may be doing a JMP instruction, hurting your pipeline
+cache, EACH time you get to the branch in question!
+
+Therefore, use un/likely() sparingly, consider it primarily for hot paths,
+use it only when you are certain that the condition in question rarely
+happens, be sure that it happens with roughly the same probability under
+most/all user conditions. One rule of thumb suggested is that the
+probability of the branch un/taken should exceed 99% (although some would
+consider 95% as well). Of course, beware of silly mistakes such as
+intending to use likely() and using unlikely() instead.
+
+A good example of this is the above kmalloc(GFP_KERNEL) call. The chances
+of kmalloc() returning NULL are rather small, because even if it doesn't
+have memory to return to you at the moment, with GFP_KERNEL/__GFP_WAIT
+passed, kmalloc() will wait and suspend your thread, while it goes off to
+find some free memory (purging caches, flushing buffers, etc.). In other
+words, kmalloc() tries very hard to give you the memory you asked for by the
+time it return.
+
+Consider the next, bad example. Suppose you're developing a file system
+which performs logically different actions on different types of entities:
+files, directories, symlinks, devices, etc. and you use this code:
+
+ if (unlikely(S_ISBLK(mode))
+ ...
+
+On first glance, the above use of unlikely() seems right. After all, most
+file system objects are files and directories, and very few of them tend to
+be block devices. So this optimization should work well, no? Although it's
+true that it'll work well for most users, what about some user who happens
+to have a file system with lots of block devices? Or what if the user has
+only one block device object on the file system, but the user has an
+application which causes the above conditional to be traversed very
+frequently (e.g., a shell script that deals with devices)? Such users will
+be penalized heavily for going [sic] down the wrong path... Therefore, you
+should consider also whether a seemingly-rare condition is indeed rare ALL
+the time.


Appendix I: References
@@ -804,6 +885,9 @@ language C, URL: http://www.open-std.org/JTC1/SC22/WG14/
Kernel CodingStyle, by greg@xxxxxxxxx at OLS 2002:
http://www.kroah.com/linux/talks/ols_2002_kernel_codingstyle_talk/html/

+FAQ/LikelyUnlikely:
+http://kernelnewbies.org/FAQ/LikelyUnlikely
+
--
Last updated on 2007-July-13.

--
1.5.2.2

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages

  • Re: how to make sure that kernel is up
    ... difference in my debug output before the following debug statement ... FS: initializing ROM/RAM file system ... Windows CE Kernel for ARM Built on Jun 24 2004 at ... MMC INTERRUPT END ...
    (microsoft.public.windowsce.platbuilder)
  • Re: [RESEND] [PATCH] readahead:add blk_run_backing_dev
    ... patched versions of the kernel or you forgot to apply the io_context ... Please recheck. ... I've also long ago noticed that reading data from block devices is slower than from files from mounted on those block devices file systems. ... Can you rerun the same 11 tests over a file on the file system, ...
    (Linux-Kernel)
  • PRB:wince.nls in CE5.0
    ... FileSystem Starting - starting with clean file system ... 0x83ce5024: FS: Registering file system StoreMgr, index 3, flags 0x00000001 ... 0x83ce5024: NK Kernel: DEBUGCHK failed in file ... The debug messages above will continue again and again, ...
    (microsoft.public.windowsce.platbuilder)
  • Kitl cannot connect
    ... I am trying to get kitl working in order to debug my platform. ... Old or invalid version stamp in kernel structures - starting clean! ... FileSystem Starting - starting with clean file system ...
    (microsoft.public.windowsce.platbuilder)
  • 2.6.27-rc7 no init found on the root partition?
    ... but the kernel is unable to boot. ... XFS file system but no init found. ... it complains that root file system not found and I have ... # Input Device Drivers ...
    (Linux-Kernel)