At Tue, 4 Jun 2013 16:54:12 -0400 (EDT), Alan Stern wrote:
On Tue, 4 Jun 2013, Joe Perches wrote:
A somewhat common convention is to use
prefix_dbg(level, fmt, ...)
where level is either a numeric value or bitmask, and also is either a #define or a MODULE_PARAM
today sound/misc.c has:
#ifdef CONFIG_SND_DEBUG
#ifdef CONFIG_SND_DEBUG_VERBOSE #define DEFAULT_DEBUG_LEVEL 2 #else #define DEFAULT_DEBUG_LEVEL 1 #endif
static int debug = DEFAULT_DEBUG_LEVEL; module_param(debug, int, 0644); MODULE_PARM_DESC(debug, "Debug level (0 = disable)");
#endif /* CONFIG_SND_DEBUG */
Yes, I have read this part of the code.
I suggest converting all the remaining
snd_printd(...) to snd_dbg(1, ...) and snd_printdd(...) to snd_dbg(2, ...)
so that debug module param control can be used for all these and if the DEFAULT_DEBUG_LEVEL isn't high enough, the various snd_dbg(2, ...) can be completely optimized away if appropriate.
I don't see how DEFAULT_DEBUG_LEVEL can be used to optimize away anything. The user can always change the value of the "debug" module parameter while the system is running. The only valid opportunity for optimization would be if CONFIG_SND_DEBUG was disabled; then all these messages would disappear.
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().
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.
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().
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...
thanks,
Takashi