Re: [00/11] pr_debug during module initialization

[Rusty Russell]

On Mon, 19 Mar 2012 00:17:29 -0600, Jim Cromie <jim.cromie@xxxxxxxxx> wrote:
> On Sun, Mar 18, 2012 at 8:04 PM, Rusty Russell <rusty@xxxxxxxxxxxxxxx> wrote:
> > The module parts seem fine. ÂThe re-parsing of the commandline seems
> > weird: ÂI'd really rather see something in unknown_bootoption(), like:
> >
> > Â Â Â Â/* Unused module parameter. */
> > Â Â Â Âif (strchr(param, '.') && (!val || strchr(param, '.') < val)) {
> > + Â Â Â Â Â Â Â /* Check for <module>.dyndebug fake param */
> > + Â Â Â Â Â Â Â dyndebug_parse(param, val);
> > Â Â Â Â Â Â Â Âreturn 0;
> > Â Â Â Â}
> 
> That is too early - the dyndbg rules cant be activated
> until dynamic_debug_init() runs, which is currently an
> arch_initcall(dynamic_debug_init);
> 
> Patch 11 makes it core-initcall, (which Im not sure is ok
> for all cases, but works for me), I also tried it with
> early_initcall (and that worked too)
> but I think thats still after the Booting kernel parse.

Indeed.  OK, that's fine then.

> I *could* capture the dyndbg options during Booting kernel
> parse, and activate them once the tables are loaded,
> but this seems convoluted.

Agreed.

> a "private" parse-args. After your earlier suggestion to use a
> callback, it occurred to me that just reusing parse_args would
> work, be fairly minimal (only 2 lines in dynamic_debug_init())
> and its already called a bunch of times; Booting kernel,
> module-load, and do-initcall-level (in Pawel's patch).
> Pawels patch doesnt do anything to avoid reparsing
> boot-time params.
> 
> Does this change your weirdness assessment ?

It's still weird to re-parse, but with a comment explaining that we
need to do it again after dynamic_debug_init().  Unless we can call
that explicitly before parse_args().
> Lastly, my earlier rev handled  foo.dyndbg params
> for loadable modules.  I took that out cuz
> Documentation/kernel-parameters.txt says thats
> for builtins (only, as I read it), and treating foo.dyndbg
> differently should be done w/o discussion.

Yes, when a module 'foo' declares it has a parameter 'bar', it becomes
'foo.bar' if the module is built-in, and just 'bar' if the module is
loaded.

Furthermore, modprobe reads /proc/cmdline when loading foo, looking for
"foo.*" to add to the modprobe arguments.

> > Otherwise, all looks good!
> >
> > Acked-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
> 
> thanks.  I guess the Ack (vs SOB) means that Jason should
> forward it on to Greg as a single set ?
> (subject to his Ack of course)

I assumed it would go via Jason.  Otherwise, get his ack and resend to
me.

Thanks,
Rusty.
-- 
  How could I marry someone with more hair than me?  http://baldalex.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/