[PATCH 1/3] CodingStyle updates

[Erez Zadok <ezk@cs.sunysb.edu>]

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@intel.com>
Cc: Kyle Moffett <mrmacman_g4@mac.com>
Cc: Jan Engelhardt <jengelh@computergmbh.de>
Cc: Adrian Bunk <bunk@kernel.org>
Cc: roel <12o3l@tiscali.nl>

Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
---
 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@kroah.com 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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/