linux-kernel - Re: [PATCH 1/3] drivers-core: make structured logging play nice with dynamic-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: <CAJfuBxxvR29xfzEhDu1WD6VjdsMJhZ5yhs0ExDiFpUpaCMPjTA@mail.gmail.com>
Date:	Mon, 23 Jul 2012 14:31:49 -0600
From:	Jim Cromie <jim.cromie@...il.com>
To:	Joe Perches <joe@...ches.com>
Cc:	Jason Baron <jbaron@...hat.com>, greg@...ah.com,
	linux-kernel@...r.kernel.org, kay@...y.com
Subject: Re: [PATCH 1/3] drivers-core: make structured logging play nice with dynamic-debug

On Mon, Jul 23, 2012 at 11:09 AM, Joe Perches <joe@...ches.com> wrote:
> On Mon, 2012-07-23 at 09:07 -0400, Jason Baron wrote:
>> On Thu, Jul 19, 2012 at 01:46:20PM -0600, Jim Cromie wrote:
>> > commit c4e00daaa96d3a0786f1f4fe6456281c60ef9a16 changed __dev_printk
>> > in a way that broke dynamic-debug's ability to control the dynamic
>> > prefix of dev_dbg(dev,..), but not dev_dbg(NULL,..) or pr_debug(..),
>> > which is why it wasnt noticed sooner.
>> >
>> > When dev==NULL, __dev_printk() just calls printk(), which just works.
>> > But otherwise, it assumed that level was always a string like "<L>"
>> > and just plucked out the 'L', ignoring the rest.  However,
>> > dynamic_emit_prefix() adds "[tid] module:func:line:" to the string,
>> > those additions all got lost.
>> >
>
> I think this is not a really good approach.
>
> 3 depths of %pV can consume quite a lot of stack
> and avoiding this is useful.
>
> I'd much rather improve/centralize the mechanism
> in dynamic_debug.c so that the extra recursion
> depth is avoided.
>
> Something like this:
>
> o Remove KERN_DEBUG from dynamic_emit_prefix
> o Make a function of create_syslog_header to format
>   the header for printk_emit.
> o Call printk_emit in dynamic_dev_dbg and dynamic_netdev_dbg
> o Call printk_emit in __dev_printk
> o Remove now unused EXPORT_SYMBOL(__netdev_printk)
> o Neatening
>
> Thoughts?
>

My patch was a minimal bug-fix aimed at rc-late, but it missed the window.
We now have room for a more comprehensive fix, such as yours.

OTOH, my patch does fix a bug, while yours is a bigger "optimization" patch.
Your commit message could identify the lost prefix as its impetus,
but I think it stands nicely on its own as a refactoring for less stack usage.

I'll try the patch soon

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/