2010-06-29 11:25:38

by Roman Fietze

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

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


2010-07-01 20:43:29

by Jason Baron

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

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

2010-07-02 08:16:22

by Roman Fietze

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

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 <[email protected]>
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 <[email protected]>
---
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


Attachments:
signature.asc (198.00 B)
This is a digitally signed message part.