linux-kernel - Re: [PATCH 19/25] pnp: if CONFIG_DYNAMIC_DEBUG, use pnp.ddebug instead of pnp.debug

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJfuBxzF7ScsQrrh4PYo5p0VQ1h=1EYTyanoVkRmwcwWRFimkw@mail.gmail.com>
Date:	Sun, 4 Dec 2011 22:42:42 -0700
From:	Jim Cromie <jim.cromie@...il.com>
To:	Thomas Renninger <trenn@...e.de>
Cc:	jbaron@...hat.com, greg@...ah.com, joe@...ches.com,
	bart.vanassche@...il.com, linux-kernel@...r.kernel.org,
	bhelgaas@...gle.com
Subject: Re: [PATCH 19/25] pnp: if CONFIG_DYNAMIC_DEBUG, use pnp.ddebug
 instead of pnp.debug

2011/12/1 Thomas Renninger <trenn@...e.de>:
> On Wednesday 30 November 2011 20:56:48 jim.cromie@...il.com wrote:
>> From: Jim Cromie <jim.cromie@...il.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 the newer unknown-parameter approach that Jason, Rusty recommended
(now done), it is possible for a module to implement its own .dyndbg option
handler (using __setup only, not with nicer module_param_named() macro,
at least with patch 25 included), but that doesnt seem wise:

modname.dyndbg is a fake option,
it doesnt show up in /sys/module/pnp/parameters/debug.
Adding pnp.dyndbg using __setup would add the sys file, giving an
entirely different interface than the one implemented in
/dbg/dynamic_debug/control.
Explaining this special case sounds difficult to do clearly, a sign of trouble.



> The only misleading would be that pnp.ddebug has nothing to do with
> dynamic debug if not compiled in, but user would have one parameter
> to rely on.
>
> In Documentation/kernel-parameters:
>        pnp.debug       [PNP]
>                        Enable PNP debug messages.  This depends on the
>                        CONFIG_PNP_DEBUG_MESSAGES option.
>
> Would be wrong in defined(CONFIG_DYNAMIC_DEBUG) case with your patch,
> but would always work with:
>        pnp.ddebug
> with my above suggestions.

how about a something like this ?


	pnp.debug=1	[PNP]
			Enable PNP debug messages (depends on the
			CONFIG_PNP_DEBUG_MESSAGES option and
                       !CONFIG_DYNAMIC_DEBUG).   If
CONFIG_DYNAMIC_DEBUG use pnp.dyndbg instead.  ...

This approach doesnt add any new failures;
if ! CONFIG_DYNAMIC_DEBUG,
     pnp.dyndbg will fail/warn just like any.dyndbg would
else
     pnp.debug will warn, yndbg will fail/warn just like any.dyndbg

>
> It's not a big deal and not a perfect solution, just looks a bit confusing
> to have 2 different parameters for the same thing.

I think this is covered adequately by a doc update, and less confusing than
different behavior/usage of 1 parameter under 2 different configs.

>
> Bjorn should have a look and acknowledge or sign the pnp parts off.
>
>    Thomas
>

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