From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH] ALSA: get rid of CONFIG_SND_VERBOSE_PRINTK Date: Fri, 07 Jun 2013 07:53:23 +0200 Message-ID: References: Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id C7AE82655D1 for ; Fri, 7 Jun 2013 07:52:47 +0200 (CEST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Alan Stern Cc: Joe Perches , alsa-devel@alsa-project.org List-Id: alsa-devel@alsa-project.org 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 > enabled. 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 > 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. 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. thanks, Takashi