Messages in this thread Patch in this message | | | Date | Mon, 11 Aug 2008 10:12:34 -0400 | From | Jason Baron <> | Subject | Re: [PATCH 1/7] dynamic debug v2 - infrastructure |
| |
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
| |