public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jason Baron <jbaron@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, Greg KH <greg@kroah.com>
Subject: Re: [patch 0/3] dynamic_printk: new feature
Date: Wed, 30 Apr 2008 17:01:50 -0400	[thread overview]
Message-ID: <20080430210147.GA5616@redhat.com> (raw)
In-Reply-To: <20080430124506.0dd2a473.akpm@linux-foundation.org>

On Wed, Apr 30, 2008 at 12:45:06PM -0700, Andrew Morton wrote:
> We're now in the situation where numerous different subsystems have
> implemented private mechnisms for tuning their printk verbosity levels.
> 
> Have you taken a look across the tree with a view to converting some of
> them?  If so, how sizeable/messy/feasible would that task be?
> 
> 

i really only focused on pr_debug()/dev_dbg(), with an eye towards
widening the scope as we go...but I agree that it would be nice to
understand the scope for the start...i find ~5000 call sites to
dprintk(), which would be ideal candidates for this type of
infrastructure.

> 
> The situation is far, far worse with compile-time debugging selection.  We
> have over two hundred different implementations of dprintk!
> 
> Have you considered the feasibility of ploddingly converting each of those
> drivers, one at a time over to the new infrastructure?  Because that's what
> we should do, I'm afraid.
> 
> An implication of this is that once a dprintk-using driver has been
> converted over to use your new infrastructure, it should still be possible
> to fully disable the debugging at compile time.  Do you handle that?
> 

that's correct. the way i've handled this in the patch is:

if DEBUG 
	you get the current compiled in behavior per .c file
elif DYNAMIC_PRINTK
	you get the dynamic runtime configurable debugging
else
	its compiled out

> > If this patch is accepted, i'd like to convert the myriad 'debug' printks -
> > DEBUGP(), dprintk(), to a standard format, either pr_debug() or dev_dbg(), to
> > hook into this mechanism.
> 
> ah, so you have looked.  How nasty will it be?
> 
> 
> A couple of things:
> 
> - Your design handles a boolean on/off control.  But some code implements
>   a verbosity-level control.  Thoughts on this?
> 

right, i think though it could easily be extended to level control.
Basically the patch associates the on/off per KBUILD_MODNAME, however we
could also associate a level per KBUILD_MODNAME. This level could be set
either by the generic debugfs interface, via module parameters at module
load time, or in the the module __init sections as appropriate.


> - I expect that other code implements a field-selector control, for the
>   lack of a better term: an greater-than-one number of separate boolean
>   controls.  How to handle this?
> 
> 

hmmm...i think this is handled by having the driver call the conditions
in its scope and then call out to the generic infrastructure if the
conditions are met.

> Thanks for working on this.  If we can get this underway and get a decent
> amount of conversion done, it will be a huuuuuuuuuuuuge cleanup to the
> kernel.  But we will need to design it carefully first.
> 
> I guess one good testcase would be ALSA.  It has pretty fancy debugging
> control (which I apparently have never been smart enough to understand). 
> Did you take a look at what they're doing, with a view to
> can-we-switch-ALSA-to-use-this?
> 
> 

ok. i'll take a more detailed look at the pontentially wider scope of this change.

thanks,

-Jason


  parent reply	other threads:[~2008-04-30 21:02 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-29 18:39 [patch 0/3] dynamic_printk: new feature Jason Baron
2008-04-30 19:45 ` Andrew Morton
2008-04-30 20:54   ` Joe Perches
2008-04-30 21:01   ` Jason Baron [this message]
2008-05-01  3:44     ` Greg KH
2008-05-01  0:23   ` Nick Andrew

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080430210147.GA5616@redhat.com \
    --to=jbaron@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=greg@kroah.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox