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

[Randy Dunlap]

On Fri, 8 Aug 2008 17:51:53 -0400 Jason Baron 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. 

What license does it have?  I don't see one.

> ---
> 
>  Documentation/kernel-parameters.txt |    5 
>  include/asm-generic/vmlinux.lds.h   |   10 +
>  include/linux/device.h              |    6 -
>  include/linux/dynamic_printk.h      |   94 ++++++++
>  include/linux/kernel.h              |    7 +
>  include/linux/module.h              |    4 
>  kernel/module.c                     |   22 ++
>  lib/Kconfig.debug                   |   56 +++++
>  lib/Makefile                        |    2 
>  lib/dynamic_printk.c                |  409 +++++++++++++++++++++++++++++++++++
>  net/netfilter/nf_conntrack_pptp.c   |    2 
>  scripts/Makefile.lib                |   11 +
>  scripts/basic/Makefile              |    2 
>  scripts/basic/hash.c                |   64 +++++
>  14 files changed, 688 insertions(+), 6 deletions(-)
>  create mode 100644 include/linux/dynamic_printk.h
>  create mode 100644 lib/dynamic_printk.c
>  create mode 100644 scripts/basic/hash.c



> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 623ef24..6664783 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -632,6 +632,62 @@ config FIREWIRE_OHCI_REMOTE_DMA
>  
>  	  If unsure, say N.
>  
> +config DYNAMIC_PRINTK_DEBUG
> +	bool "Enable dynamic printk() call support"
> +	default n
> +	depends on PRINTK
> +	select PRINTK_DEBUG
> +	help
> +	 
> +	  Compiles debug level messages into the kernel, which would not
> +	  otherwise be available at runtime. These messages can then be
> +	  enabled/disabled on a per module basis. This mechanism, implicitly 

no comma.  Don't end lines with whitespace.

> +	  enables all pr_debug() and dev_dbg() calls. The impact of this 
> +	  compile option is a larger kernel text size ~2%.

	                                         size of about 2%.

> +
> +	  Usage:
> +
> +	  Dynamic debugging is controlled by the debugfs file, 
> +	  dynamic_printk/modules. This file contains a list of the modules that
> +	  can be enabled. The format of the file is the module name, followed
> +	  by a set of flags that can be enabled. The first flags is always the

	                                             first flag is

> +	  'enabled' flags. For example:

	            flag.

> +
> +		<module_name> <enabled=0/1>
> +				.
> +				.
> +				.
> +
> +	  <module_name> : Name of the module in which the debug call resides
> +	  <enabled=0/1> : whether the the messages are enabled or not

	                          the

> +
> +	  From a live system:
> +
> +		snd_hda_intel enabled=0
> +
> +		fixup enabled=0
> +
> +		driver enabled=0

including the apparently extraneous blank lines??

> +
> +	  Enable a module:
> +
> +	  	$echo "set enabled=1 <module_name>" > dynamic_printk/modules
> +
> +	  Disable a module:
> +
> +	  	$echo "set enabled=0 <module_name>" > dynamic_printk/modules
> +
> +	  Enable all modules:
> +	
> +		$echo "set enabled=1 all" > dynamic_printk/modules
> +
> +	  Disable all modules:
> +
> +		$echo "set enabled=0 all" > dynamic_printk/modules
> +
> +	  Finally, passing "dynamic_printk" at the command line enables all 
> +	  modules. This mode can be turned off by disabling modules.

What does "can be turned off by disabling modules" mean, please?
CONFIG_MODULES=n or something else?

> +	
>  source "samples/Kconfig"
>  
>  source "lib/Kconfig.kgdb"
> diff --git a/lib/dynamic_printk.c b/lib/dynamic_printk.c
> new file mode 100644
> index 0000000..04ae7e2
> --- /dev/null
> +++ b/lib/dynamic_printk.c
> @@ -0,0 +1,409 @@
> +
> +extern struct mod_debug __start___verbose[];
> +extern struct mod_debug __stop___verbose[];

put these into a header file.

> +
> +struct debug_name {
> +	struct hlist_node hlist;
> +	struct hlist_node hlist2;
> +	int hash1;
> +	int hash2;
> +	char *name;
> +	int enable;
> +	int type;
> +};
> +
> +static int nr_entries;
> +static int num_enabled;
> +int dynamic_enabled = DYNAMIC_ENABLED_NONE;
> +static struct hlist_head module_table[DEBUG_HASH_TABLE_SIZE] =
> +	{ [0 ... DEBUG_HASH_TABLE_SIZE-1] = HLIST_HEAD_INIT };
> +static struct hlist_head module_table2[DEBUG_HASH_TABLE_SIZE] =
> +	{ [0 ... DEBUG_HASH_TABLE_SIZE-1] = HLIST_HEAD_INIT };
> +static DECLARE_MUTEX(debug_list_mutex);
> +
> +long long dynamic_printk_enabled;
> +EXPORT_SYMBOL_GPL(dynamic_printk_enabled);
> +long long dynamic_printk_enabled2;
> +EXPORT_SYMBOL_GPL(dynamic_printk_enabled2);
> +
> +/* returns the debug module pointer. caller must locking */

      caller must ___ locking ??

> +static struct debug_name *find_debug_module(char *module_name)
> +{
> +	int i;
> +	struct hlist_head *head;
> +	struct hlist_node *node;
> +	struct debug_name *element;
> +
> +	element = NULL;
> +	for (i = 0; i < DEBUG_HASH_TABLE_SIZE; i++) {
> +		head = &module_table[i];
> +		hlist_for_each_entry_rcu(element, node, head, hlist)
> +			if (!strcmp(element->name, module_name))
> +				return element;
> +	}
> +	return NULL;
> +}
> +
> +/* returns the debug module pointer. caller must locking */

ditto, check comment.

> +static struct debug_name *find_debug_module_hash(char *module_name, int hash)
> +{
> +	struct hlist_head *head;
> +	struct hlist_node *node;
> +	struct debug_name *element;
> +
> +	element = NULL;
> +	head = &module_table[hash];
> +	hlist_for_each_entry_rcu(element, node, head, hlist)
> +		if (!strcmp(element->name, module_name))
> +			return element;
> +	return NULL;
> +}
> +

Please add kernel-doc comments for the exported/public interfaces.

> +int unregister_dynamic_debug_module(char *mod_name)
> +{
> +	struct debug_name *element;
> +	int ret = 0;
> +
> +	down(&debug_list_mutex);
> +	element = find_debug_module(mod_name);
> +	if (!element) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +	hlist_del_rcu(&element->hlist);
> +	hlist_del_rcu(&element->hlist2);
> +	synchronize_rcu();
> +	if (element->name)
> +		kfree(element->name);
> +	if (element->enable)
> +		num_enabled--;
> +	kfree(element);
> +	nr_entries--;
> +out:
> +	up(&debug_list_mutex);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(unregister_dynamic_debug_module);
> +
> +int register_dynamic_debug_module(char *mod_name, int type, char *share_name,
> +					char *flags, int hash, int hash2)
> +{
> +	struct debug_name *elem;
> +	int ret = 0;
> +
> +	down(&debug_list_mutex);
> +	elem = find_debug_module(mod_name);
> +	if (!elem) {
> +		if (__add_debug_module(mod_name, hash, hash2))
> +			goto out;
> +		elem = find_debug_module(mod_name);
> +		if (dynamic_enabled == DYNAMIC_ENABLED_ALL &&
> +				!strcmp(mod_name, share_name)) {
> +			elem->enable = true;
> +			num_enabled++;
> +		}
> +	}
> +	elem->type |= type;
> +out:
> +	up(&debug_list_mutex);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(register_dynamic_debug_module);
> +

> diff --git a/scripts/basic/hash.c b/scripts/basic/hash.c
> new file mode 100644
> index 0000000..3299ad7
> --- /dev/null
> +++ b/scripts/basic/hash.c
> @@ -0,0 +1,64 @@
> +/*
> + * Copyright (C) 2008 Red Hat, Inc., Jason Baron <jbaron@xxxxxxxxxx>
> + *
> + */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#define DYNAMIC_DEBUG_HASH_BITS 6
> +
> +static const char *program;
> +
> +static void usage(void)
> +{
> +	printf("Usage: %s <djb2|r5> <modname>\n", program);
> +	exit(1);
> +}
> +
> +/* djb2 hashing algorithm by Dan Bernstein. From:
> + * http://www.cse.yorku.ca/~oz/hash.html
> + */
> +
> +unsigned int djb2_hash(char *str)
> +{
> +	unsigned long hash = 5381;
> +	int c;
> +
> +	c = *str;
> +	while (c) {
> +		hash = ((hash << 5) + hash) + c;
> +		c = *++str;
> +	}
> +	return (unsigned int)(hash & ((1 << DYNAMIC_DEBUG_HASH_BITS) - 1));
> +}
> +
> +unsigned int r5_hash(char *str)
> +{
> +	unsigned long hash = 0;
> +	int c;
> +
> +	c = *str;
> +	while (c) {
> +		hash = (hash + (c << 4) + (c >> 4)) * 11;
> +		c = *++str;
> +	}
> +	return (unsigned int)(hash & ((1 << DYNAMIC_DEBUG_HASH_BITS) - 1));
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	program = argv[0];
> +
> +	if (argc != 3)
> +		usage();
> +	if (!strcmp(argv[1], "djb2"))
> +		printf("%d\n", djb2_hash(argv[2]));
> +	else if (!strcmp(argv[1], "r5"))
> +		printf("%d\n", r5_hash(argv[2]));
> +	else
> +		usage();
> +	exit(0);
> +}


---
~Randy
Linux Plumbers Conference, 17-19 September 2008, Portland, Oregon USA
http://linuxplumbersconf.org/
--
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/