[PATCH] dynamic_debug: allow to set dynamic debug flags right at module load time

[PATCH] dynamic_debug: allow to set dynamic debug flags right at module load time

[Roman Fietze]

Hello Jason, hello list,

If I'm not wrong one could only enable any dynamic debugging flag
after a module had been completely loaded, using debugfs. This makes
it impossible to use dev_dbg or pr_debug e.g. inside the module init
function or any function called by it.

My patch works by replacing _DPRINTK_FLAGS_DEFAULT after including all
kernel headers in my module source file and some small patch inside
dynamic_debug.c setting up the internal variables already when loading
a module with flags unequal to zero. This patch can of course be
optimized somewhat by reusing existing variables.

Subject: [PATCH] dynamic_debug: allow to set dynamic debug flags right at module load time

This allows to use e.g. pr_debug right from the beginning, e.g. in the
module init function.

- the module must redefine _DPRINTK_FLAGS_DEFAULT, e.g.

  #undef _DPRINTK_FLAGS_DEFAULT
  #define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT

- when a module is loaded and the flags are not zero, the enabled
  count and hash masks are enabled right away

Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
---
 lib/dynamic_debug.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index d6b8b9b..d10466e 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -656,6 +656,8 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n,
 {
 	struct ddebug_table *dt;
 	char *new_name;
+	size_t i;
+	char flagbuf[8];
 
 	dt = kzalloc(sizeof(*dt), GFP_KERNEL);
 	if (dt == NULL)
@@ -671,6 +673,22 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n,
 	dt->ddebugs = tab;
 
 	mutex_lock(&ddebug_lock);
+	for (i = 0 ; i < dt->num_ddebugs ; i++) {
+		struct _ddebug *dp = &dt->ddebugs[i];
+
+		if (dp->flags) {
+			dt->num_enabled++;
+			dynamic_debug_enabled |= (1LL << dp->primary_hash);
+			dynamic_debug_enabled2 |= (1LL << dp->secondary_hash);
+			if (verbose)
+				printk(KERN_INFO
+					"ddebug: added %s:%d [%s]%s %s\n",
+					dp->filename, dp->lineno,
+					dt->mod_name, dp->function,
+					ddebug_describe_flags(dp, flagbuf,
+							sizeof(flagbuf)));
+		}
+	}
 	list_add_tail(&dt->link, &ddebug_tables);
 	mutex_unlock(&ddebug_lock);
 
-- 
1.7.1


-- 
Roman Fietze                Telemotive AG Büro Mühlhausen
Breitwiesen                              73347 Mühlhausen
Tel.: +49(0)7335/18493-45        http://www.telemotive.de

Re: [PATCH] dynamic_debug: allow to set dynamic debug flags right at module load time

[Jason Baron]

On Wed, May 26, 2010 at 02:25:38PM +0200, Roman Fietze wrote:
> Hello Jason, hello list,
> 
> If I'm not wrong one could only enable any dynamic debugging flag
> after a module had been completely loaded, using debugfs. This makes
> it impossible to use dev_dbg or pr_debug e.g. inside the module init
> function or any function called by it.
> 

yes, that's correct.

> My patch works by replacing _DPRINTK_FLAGS_DEFAULT after including all
> kernel headers in my module source file and some small patch inside
> dynamic_debug.c setting up the internal variables already when loading
> a module with flags unequal to zero. This patch can of course be
> optimized somewhat by reusing existing variables.
> 
> Subject: [PATCH] dynamic_debug: allow to set dynamic debug flags right at module load time
> 
> This allows to use e.g. pr_debug right from the beginning, e.g. in the
> module init function.
> 
> - the module must redefine _DPRINTK_FLAGS_DEFAULT, e.g.
> 
>   #undef _DPRINTK_FLAGS_DEFAULT
>   #define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
> 
> - when a module is loaded and the flags are not zero, the enabled
>   count and hash masks are enabled right away
> 

it's a good idea, but i think we want this to be runtime configurable.
That is, we probably want this implemented as a module parameter, not as
a compile time thing. something like: modprobe module verbose=1

thanks,

-Jason

Re: [PATCH] dynamic_debug: allow to set dynamic debug flags right at module load time

[Roman Fietze]

[-- Attachment #1: Type: Text/Plain, Size: 994 bytes --]

Hello Jason,

On Wednesday 26 May 2010 20:35:59 Jason Baron wrote:

> ... we want this to be runtime configurable.
> That is, we probably want this implemented as a module parameter, not as
> a compile time thing. something like: modprobe module verbose=1

Kind of

#define dynamic_pr_debug(fmt, ...) do {					\
	...
		DEBUG_HASH2, __LINE__,					\
		verbose ? _DPRINTK_FLAGS_PRINT : _DPRINTK_FLAGS_DEFAULT}; \
	...

But what if verbose isn't there?

Or something smarter inside dynamic_debug_setup() or
ddebug_add_module() looking for a module symbol or parameter with that
name?

If you'll give me a hint I could probably implement a proposal.


Roman

-- 
Roman Fietze                Telemotive AG Büro Mühlhausen
Breitwiesen                              73347 Mühlhausen
Tel.: +49(0)7335/18493-45        http://www.telemotive.de

Amtsgericht Ulm                                HRB 541321
Vorstand:
Peter Kersten, Markus Fischer, Franz Diller, Markus Stolz

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] dynamic_debug: allow to set dynamic debug flags right at module load time

[Jason Baron]

On Thu, May 27, 2010 at 07:05:43AM +0200, Roman Fietze wrote:
> Hello Jason,
> 
> On Wednesday 26 May 2010 20:35:59 Jason Baron wrote:
> 
> > ... we want this to be runtime configurable.
> > That is, we probably want this implemented as a module parameter, not as
> > a compile time thing. something like: modprobe module verbose=1
> 
> Kind of
> 
> #define dynamic_pr_debug(fmt, ...) do {					\
> 	...
> 		DEBUG_HASH2, __LINE__,					\
> 		verbose ? _DPRINTK_FLAGS_PRINT : _DPRINTK_FLAGS_DEFAULT}; \
> 	...
> 
> But what if verbose isn't there?
> 
> Or something smarter inside dynamic_debug_setup() or
> ddebug_add_module() looking for a module symbol or parameter with that
> name?
> 

right, i think we want to add something inside ddebug_add_module() that
recognizes if the module was loaded with verbose=1. I think you can get
at the parameters via module->kp, which we need to pass in as well.

There is also a naming issue, in that if we "reserve" the param
"verbose", how do we make sure no other module wants to use that as a
module parameter name. Or maybe it doesn't matter if we don't consume
the parameter. That is, the parameter can mean 2 things. not sure.

thanks,

-Jason

Re: [PATCH] dynamic_debug: allow to set dynamic debug flags right at module load time

[Roman Fietze]

Hello Jason, hello LKML,

On Friday 28 May 2010 15:55:49 Jason Baron wrote:

> right, i think we want to add something inside ddebug_add_module()
> that recognizes if the module was loaded with verbose=1. I think you
> can get at the parameters via module->kp, which we need to pass in
> as well.

Yes, I would first check if there's a section named "__verbose" as it
is right now. If yes, I would search the already setup module->kp for
the used parameter.


Proposals, not being sure how to implement that right now:

Default is to search e.g. for param "dprintk".

Provide a macro to override that default, e.g.
DPRINTK_PARAM("verbose")

If the default or defined bool or other integer parameter is unequal
to 0 enable the p-flag on module load for all debug statements of this
module.


Questions just in case the proposal is kind of ok:

Prepare the code to allow the setting of different future flags
unequal to 'p'?

Use a charp param instead of a bool to allow that?


Roman

-- 
Roman Fietze              Telemotive AG Buero Muehlhausen
Breitwiesen                             73347 Muehlhausen
Tel.: +49(0)7335/18493-45        http://www.telemotive.de

Re: [PATCH] dynamic_debug: allow to set dynamic debug flags right at module load time

[Jason Baron]

On Tue, Jun 29, 2010 at 01:25:29PM +0200, Roman Fietze wrote:
> Hello Jason, hello LKML,
> 
> On Friday 28 May 2010 15:55:49 Jason Baron wrote:
> 
> > right, i think we want to add something inside ddebug_add_module()
> > that recognizes if the module was loaded with verbose=1. I think you
> > can get at the parameters via module->kp, which we need to pass in
> > as well.
> 
> Yes, I would first check if there's a section named "__verbose" as it
> is right now. If yes, I would search the already setup module->kp for
> the used parameter.
>
> 
> Proposals, not being sure how to implement that right now:
> 
> Default is to search e.g. for param "dprintk".
> 

make sense.

> Provide a macro to override that default, e.g.
> DPRINTK_PARAM("verbose")
>

why would we want to override it?
 
> If the default or defined bool or other integer parameter is unequal
> to 0 enable the p-flag on module load for all debug statements of this
> module.
> 
> 
> Questions just in case the proposal is kind of ok:
> 
> Prepare the code to allow the setting of different future flags
> unequal to 'p'?
> 
> Use a charp param instead of a bool to allow that?
> 

yes, we might eventually want more than a bool, but unless you have a
specific case in mind, I would keep as simple as possible for now.

thanks,

-Jason

Re: [PATCH] dynamic_debug: parse module parameters to enable dynamic printk at load time

[Roman Fietze]

[-- Attachment #1: Type: Text/Plain, Size: 6140 bytes --]

Hello Jason, hello LKML,

On Thursday 01 July 2010 22:43:19 Jason Baron wrote:

> make sense.
> ...
> I would keep as simple as possible for now.

So here we go with a first proposal. I think this can be just a base
for discussions, because I'm not yet used to tweak around in the guts
of the kernel itself, so there must be better solutions compared to
what I can offer here.

BTW, I had to move the ddebug setup below the parameter parsing and
setup, just in case somebody is wondering.

From 7ae21e5fc935e6aef4801e0ebce1886bdd2a7b74 Mon Sep 17 00:00:00 2001
From: Roman Fietze <roman.fietze@telemotive.de>
Date: Fri, 2 Jul 2010 10:06:20 +0200
Subject: [PATCH] dynamic_debug: parse module parameters to enable dynamic 
printk at load time

When a module is loaded do an additional check for a module parameter
named "dprintk" of type bool or int. If this paremeter is unequal to 0
then set the dynamic debug print flag right at load time.

Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
---
 include/linux/dynamic_debug.h |    2 +-
 kernel/module.c               |   50 +++++++++++++++++++++++++++++++---------
 lib/dynamic_debug.c           |   27 +++++++++++++++++++--
 3 files changed, 63 insertions(+), 16 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index f8c2e17..1b790a7 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -37,7 +37,7 @@ struct _ddebug {
 
 
 int ddebug_add_module(struct _ddebug *tab, unsigned int n,
-				const char *modname);
+				const char *modname, bool p_flag);
 
 #if defined(CONFIG_DYNAMIC_DEBUG)
 extern int ddebug_remove_module(char *mod_name);
diff --git a/kernel/module.c b/kernel/module.c
index 1016b75..58b4713 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1954,10 +1954,35 @@ static inline void add_kallsyms(struct module *mod,
 }
 #endif /* CONFIG_KALLSYMS */
 
-static void dynamic_debug_setup(struct _ddebug *debug, unsigned int num)
+static void dynamic_debug_setup(struct _ddebug *debug,
+				unsigned int num_debug,
+				struct kernel_param *params,
+				unsigned int num_kp)
 {
 #ifdef CONFIG_DYNAMIC_DEBUG
-	if (ddebug_add_module(debug, num, debug->modname))
+	bool p_flag = 0;
+
+	while (num_kp--) {
+		if (!strcmp("dprintk", params->name)) {
+			if (params->get == param_get_bool) {
+				if (params->flags & KPARAM_ISBOOL)
+					p_flag = *(bool *)params->arg;
+				else
+					p_flag = *(int *)params->arg;
+			}
+			else if (params->get == param_get_int) {
+				p_flag = *((int *)params->arg);
+			}
+			else {
+				pr_err("invalid type of dprintk module "
+				       "parameter adding module: %s\n",
+				       debug->modname);
+			}
+			break;
+		}
+		params++;
+	}
+	if (ddebug_add_module(debug, num_debug, debug->modname, p_flag))
 		printk(KERN_ERR "dynamic debug error adding module: %s\n",
 					debug->modname);
 #endif
@@ -2387,16 +2412,6 @@ static noinline struct module *load_module(void __user 
*umod,
 	kfree(strmap);
 	strmap = NULL;
 
-	if (!mod->taints) {
-		struct _ddebug *debug;
-		unsigned int num_debug;
-
-		debug = section_objs(hdr, sechdrs, secstrings, "__verbose",
-				     sizeof(*debug), &num_debug);
-		if (debug)
-			dynamic_debug_setup(debug, num_debug);
-	}
-
 	err = module_finalize(hdr, sechdrs, mod);
 	if (err < 0)
 		goto cleanup;
@@ -2443,6 +2458,17 @@ static noinline struct module *load_module(void __user 
*umod,
 	add_sect_attrs(mod, hdr->e_shnum, secstrings, sechdrs);
 	add_notes_attrs(mod, hdr->e_shnum, secstrings, sechdrs);
 
+	if (!mod->taints) {
+		struct _ddebug *debug;
+		unsigned int num_debug;
+
+		debug = section_objs(hdr, sechdrs, secstrings, "__verbose",
+				     sizeof(*debug), &num_debug);
+		if (debug)
+			dynamic_debug_setup(debug, num_debug,
+					    mod->kp, mod->num_kp);
+	}
+
 	/* Get rid of temporary copy */
 	vfree(hdr);
 
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index d6b8b9b..2ea876b 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -652,7 +652,7 @@ static const struct file_operations ddebug_proc_fops = {
  * and add it to the global list.
  */
 int ddebug_add_module(struct _ddebug *tab, unsigned int n,
-			     const char *name)
+		      const char *name, bool p_flag)
 {
 	struct ddebug_table *dt;
 	char *new_name;
@@ -671,6 +671,26 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int 
n,
 	dt->ddebugs = tab;
 
 	mutex_lock(&ddebug_lock);
+	if (p_flag) {
+		struct _ddebug *dp = dt->ddebugs;
+		size_t i = dt->num_ddebugs;
+		while (i--) {
+			dp->flags |= _DPRINTK_FLAGS_PRINT;
+			dt->num_enabled++;
+			dynamic_debug_enabled |= (1LL << dp->primary_hash);
+			dynamic_debug_enabled2 |= (1LL << dp->secondary_hash);
+			if (verbose) {
+				char flagbuf[8];
+				printk(KERN_INFO
+				       "ddebug: added %s:%d [%s]%s %s\n",
+				       dp->filename, dp->lineno,
+				       dt->mod_name, dp->function,
+				       ddebug_describe_flags(dp, flagbuf,
+							     
sizeof(flagbuf)));
+			}
+			dp++;
+		}
+	}
 	list_add_tail(&dt->link, &ddebug_tables);
 	mutex_unlock(&ddebug_lock);
 
@@ -748,7 +768,8 @@ static int __init dynamic_debug_init(void)
 		iter_start = iter;
 		for (; iter < __stop___verbose; iter++) {
 			if (strcmp(modname, iter->modname)) {
-				ret = ddebug_add_module(iter_start, n, 
modname);
+				ret = ddebug_add_module(iter_start, n, 
modname,
+							0);
 				if (ret)
 					goto out_free;
 				n = 0;
@@ -757,7 +778,7 @@ static int __init dynamic_debug_init(void)
 			}
 			n++;
 		}
-		ret = ddebug_add_module(iter_start, n, modname);
+		ret = ddebug_add_module(iter_start, n, modname, 0);
 	}
 out_free:
 	if (ret) {
-- 
1.7.1




Roman

-- 
Roman Fietze                Telemotive AG Büro Mühlhausen
Breitwiesen                              73347 Mühlhausen
Tel.: +49(0)7335/18493-45        http://www.telemotive.de

Amtsgericht Ulm                                HRB 541321
Vorstand:
Peter Kersten, Markus Fischer, Franz Diller, Markus Stolz

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]