[alsa-devel] [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK
tiwai at suse.de
Fri Jun 7 07:53:23 CEST 2013
At Thu, 6 Jun 2013 17:28:52 -0400 (EDT),
Alan Stern wrote:
> 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
Originally, yes, but differently developed along time.
> 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?
KERN_DEBUG appears in dmesg normally, no?
> > 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
> 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.
Yeah, dev_*() is more appropriate in many places.
Feel free to work on replacement with dev_*() variants for
sound/usb/*. I prefer such bottom up cleanups, actually.
More information about the Alsa-devel