Messages in this thread | | | From | Jim Cromie <> | Date | Mon, 23 Jul 2012 14:31:49 -0600 | 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@perches.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
| |