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