lkml.org 
[lkml]   [2011]   [Dec]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 19/25] pnp: if CONFIG_DYNAMIC_DEBUG, use pnp.ddebug instead of pnp.debug
On Mon, Dec 5, 2011 at 7:47 AM, Thomas Renninger <trenn@suse.de> wrote:
> On Monday, December 05, 2011 06:42:42 AM Jim Cromie wrote:
>> 2011/12/1 Thomas Renninger <trenn@suse.de>:
>> > On Wednesday 30 November 2011 20:56:48 jim.cromie@gmail.com wrote:
>> >> From: Jim Cromie <jim.cromie@gmail.com>
>> >>
>> >> resubmit of https://lkml.org/lkml/2010/9/15/398
>> >>
>> >> This allows usage of generic pnp.ddebug debug parameter instead of
>> >> pnp.debug PNP specific parameter.
>>
>> > It depends on what you compile in which pnp debug parameter one has to use
>> > and both are doing more or less the same?
>> >
>> > We could add two pnp parameters in !defined(CONFIG_DYNAMIC_DEBUG) case:
>> >  - deprecate pnp.debug by a message:
>> >    "pnp.debug deprecated, use pnp.ddebug" instead
>>
>> Just to be clear, this patch (yours) does this deprecation.
>>
>> >  - pnp.ddebug doing what pnp.debug is doing currently
>>
>> FWIW, the patch after this changes the name .ddebug to .dyndbg.
>>
>> Why is this better than just fixing kernel-parameters to
>> advise using dyndbg directly, and skipping the indirection ?
> With this patch you'd have pnp.debug and pnp.dyndbg essentially doing
> the same (from what I can see),
> but you'd either have to use the one or the other, depending
> on what is compiled in.
>
> It's not a big deal, but imo it would be nice to have one pnp debug
> option which would always work.
>
> This could look like this (not even compile tested):
>
> #if !defined(CONFIG_DYNAMIC_DEBUG)
> static int __init pnp_debug_setup(char *__unused)
> {
>     pnp_debug = 1;
> }
> __setup("pnp.dyndbg", pnp_debug_setup);
>
> static int __init pnp_old_debug_setup(char *__unused)
> {
>     printk(KERN_INFO "pnp.debug is deprecated, use pnp.dyndbg instead\n");
> }
> __setup("pnp.debug", pnp_old_debug_setup);
> #endif
>
>
> As said, not a big deal. Maybe nicer, not sure.

I agree with this size assessment :-)

However, you trimmed my argument against your approach;

$modname.dyndbg is generally fake, __setup("pnp.dyndbg"...) is not.
This makes a completely different control interface for 2 kinds of builds.

echo $setting > /sys/modules/pnp/parameters/dyndbg
vs
echo "module pnp +/-p" > /dbg/dynamic/control

its certainly possible to translate $setting into
ddebug_exec_query( (setting ? "+p" : "-p"), "pnp");
but this still leaves the issue of the different interface,
hidden below the skin-deep appearance of a single parameter.
It also doesnt address what this means:
echo "-pmf" > /sys/modules/pnp/parameters/dyndbg
I think its a rabbit-hole of special cases, which we shouldnt go down.

Summarizing, I think the choices are:

1- 2 different module-options for yes/no CONFIG_DYNAMIC_DEBUG builds
2- 1 module-option with 2 different interfaces
3- deeper but still imperfect translations of 1 to other.

Ive coded 1, and will submit it shortly,
but am willing to reconsider, or to just follow marching orders.

>
>    Thomas

thanks
Jim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2011-12-05 20:19    [W:0.099 / U:0.136 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site