[alsa-devel] [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK

[Alan Stern]

On Wed, 5 Jun 2013, Takashi Iwai wrote:

> > You didn't respond to the first point I raised.  Since these messages
> > are all meant for debugging, there's no point allowing them to have
> > prefixes like KERN_ERR or KERN_INFO.  They should always be printed at
> > the KERN_DEBUG level.  Or did you think this was so obviously true that
> > it didn't require any comment?
> 
> Unfortunately, it's not so straightforward.  Many messages are better
> with KERN_INFO indeed.  In such places, snd_printd() is used rather as
> snd_chattier_printk_with_prefix().

I don't understand the point of this.  The idea of using snd_printd() 
is to prevent these messages from being printed unless debugging is 
enabled.  Since the only time you are going to see these messages is 
when CONFIG_SND_DEBUG is on, why shouldn't they use KERN_DEBUG?

> This comes from the subsystem development history.  In the era of 2.4
> kernels, many systems did load/unload the module frequently on demand.
> Thus we didn't want to put a message from driver at each module probe
> or removal.  Otherwise dmesg will be flood of such messages at each
> time you play a WAV song or ring a beep via PCM.
> 
> Instead, we tended to put such informational messages as snd_printd(),
> while keeping the driver itself reticent as much as possible for
> "productive" systems.  This style was kept for a while even after
> merged to 2.5 kernels until recently.

Changing snd_printd() to use KERN_DEBUG always won't make the driver 
any less reticent.

> Also, some places use KERN_WARNING or KERN_ERR with snd_printd(),
> mostly because they are in the context with CONFIG_SND_DEBUG.
> They can be well pr_warning() or pr_err().

Okay.  Those can be changed separately.

> Last but not least, as mentioned in the reply to patch 1/2, many
> messages miss proper prefixes.
> 
> 
> These are the main reasons I stated that systematic replacements would
> be difficult.  We need to take a look through the whole snd_print*()
> and replace appropriately case by case...

I'm not sure if my original point behind all this is still clear.  I
don't really want to make a lot of small updates to the logging API for
the sound subsystem, or clean up messages that have the wrong logging
level.

The important point is that these files are _drivers_; what matters at
runtime is the device name.  On systems with more than one sound card
of the same type, the device name is vital, but even on others it still
helps a lot.

What I want to do is make the messages include the device and driver
names.  Basically this means calling dev_info() and friends instead of
"snd_printk(KERN_INFO" or pr_info().  As part of the fallout from this
change, it should no longer be necessary to print the filename or
module name.  If a driver contains multiple identical messages, they
can be made non-identical (assuming it really matters; messages like
"can't allocate memory" usually don't need to be pinned down exactly).  
Line numbers are even less useful because they can change from one
kernel version to the next.

I can't convert the entire sound subsystem at once -- it's way too big.  
But I can at least do the sound/usb subsystem.  If this means that two
different logging APIs are used by different sets of sound drivers, so
be it.

In theory, this would be more or less independent of the patches posted 
so far.  In fact, when taken to its logical extreme, _all_ the messages 
would use dev_*() or some variant.  There wouldn't be any snd_printk(), 
snd_printd(), pr_info(), ... messages left to worry about.

I don't expect that to happen any time soon.  Still, I think it makes
more sense to concentrate on converting the messages below sound/usb
rather than fiddling around with the existing API, which is inadequate
anyway since it doesn't include the device.

Alan Stern