Re: [PATCH 1/7] dynamic debug v2 - infrastructure

[Jason Baron <jbaron@redhat.com>]

On Fri, Aug 08, 2008 at 06:07:41PM -0700, Greg KH wrote:
> > Few notes...there is still one control file: <debugfs>/dynamic_printk/modules
> > We can split this up now or later, but I kind of like being able to see all
> > the controls in one file. Also, i've used a djb2 hash function in the code,
> > which i'm not sure is under the correct license/copyright, so i just wanted
> > to point that out as well. see: scripts/basic/hash.c. If its an issue, i can
> > find another hash function. 
> 
> Why use that hash function?  Actually, it looks like you are using 2
> different ones, right?  Why?
> 

The hash functions are used to map between the calling module name, 
KBUILD_MODNAME, and whether or not debugging is enabled. In the 'fast' case a
bitmask is used to represent each hash bucket. One bit per bucket. Thus, if
all the modules in a bucket are disabled the bitmask is simply 0. Two
hashes are used in order to reduce the probability of collisions, used in this
way it is similar to a bloom filter. The code that implements this is:

#define __dynamic_dbg_enabled(module, type, value, level, hash)  ({          \
        int ret = 0;                                                         \
        if (unlikely((dynamic_printk_enabled & (1LL << DEBUG_HASH)) &&       \
                        (dynamic_printk_enabled2 & (1LL << DEBUG_HASH2))))   \
                        ret = __dynamic_dbg_enabled_helper(module, type,     \
                                                                value, hash);\
        ret; })

where DEBUG_HASH and DEBUG_HASH2 are computed at compile time. Thus, in the
'fast' case where debugging is off, we have two extra intructions, a test and
a jmp. Further, if we enable debugging for a single module, we will at most
do two 'tests' and a jump for disabled modules. If the module is enabled, we
would call through to __dynamic_dbg_enabled_helper(), which verifies whether
or not debugging is enabled for the module. I believe if we are then calling
'printk()' the cost of this function call is 'in the noise'. Thus, i've tried
to optimize the disabled case.

> > --- /dev/null
> > +++ b/include/linux/dynamic_printk.h
> > @@ -0,0 +1,94 @@
> > +#ifndef _DYNAMIC_PRINTK_H
> > +#define _DYNAMIC_PRINTK_H
> > +
> > +#ifdef __KERNEL__
> 
> Shouldn't be needed.
> 

ok

> > +#include <linux/string.h>
> > +#include <linux/hash.h>
> > +
> > +#define DYNAMIC_DEBUG_HASH_BITS 6
> > +#define DEBUG_HASH_TABLE_SIZE (1 << DYNAMIC_DEBUG_HASH_BITS)
> > +
> > +#define TYPE_BOOLEAN 1
> 
> What's this for?
> 

This is really in anticipation of more 'involved' dynamic debugging. That is,
'TYPE_BOOLEAN', just means the debugging is either off/on. The other types
are 'TYPE_FLAG', where you can control debugging for a module via a bitmask,
or 'TYPE_LEVEL', where you control the debugging for a module via a level 
control like printk levels. These different types were implemented in the
orginal patchset.


> > +#define DYNAMIC_ENABLED_ALL 0
> > +#define DYNAMIC_ENABLED_NONE 1
> > +#define DYNAMIC_ENABLED_SOME 2
> > +
> > +extern int dynamic_enabled;
> > +extern long long dynamic_printk_enabled;
> > +extern long long dynamic_printk_enabled2;
> 
> What's up with the "2" versions?
> 

"2" here is the second bitmask as explained previously.

thanks,

-Jason