Message-ID: <4F5DC8D9.9030608@lge.com>
Date:	Mon, 12 Mar 2012 18:58:49 +0900
From:	Namhyung Kim <namhyung.kim@....com>
To:	linux-kernel@...r.kernel.org
Cc:	Ingo Molnar <mingo@...e.hu>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Paul Mackerras <paulus@...ba.org>,
	David Ahern <dsahern@...il.com>,
	Namhyung Kim <namhyung@...il.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: [RFD] perf: Integrate logging facilities

Hi, all

This is my proposal for integrating various logging facilities used on perf 
tools. We now have a couple of ways to report something bad happened to user 
as follows:

  * fprintf(stderr, ...)
  * perror
  * die/error/warning/usage
  * pr_err/pr_warning/pr_info/pr_debug
  * ui__error/ui__warning

Also we use dump_printf() when -D given. Some of them are aware of TUI (based 
on the value of use_browser), but other's are not. In addition, we should 
consider the case of python binding. There also is a patch proposed by Pekka 
Enberg which add a GTK+ GUI support on perf report. All of these make things 
complicated when someone - like me - who has no idea of this situation wants 
to leave a message for the user. And their meaning and expected behaivor is 
somewhat vague AFAICS.

So I suggest that we should select one method and deprecates other usages, 
make it flexible to deal with current complexity and possible future 
improvements. IMHO pr_err or ui__error (and its friends) can be chosen as the 
candidate. I prefer the pr_err() since it's a standard method of kernel 
logging and shorter to type :). And it supports more than 4 levels to 
differentiate its priory so that the developer can use appropriate one 
depending on the case. Now I think we can provide following semantics for each 
function:

  * pr_err: Fatal error occurred. perf cannot continue to work anymore and
	exits immediately once user noticed - by pressing keys, clicking
	buttons (if GUI provided) or simply printing it on terminal.
  * pr_warning: Serious error occurred. perf may or may not continue to work -
	if UI somehow provides a way to alter related options dynamically?
	User also will be imformed the message by pressing keys or clicking
	buttons. In case of stdio, the message will be followed by sleep(1).
  * pr_info: Just a informative message and can be buried with other messages.
	UI will put this on the helpline. perf will keep doing its work.
  * pr_debug: Messages to make debugging easy. It can be seen only -v option
	is given. UI will put this on the helpline too.

To implement this, I suggest using a table of function pointers to each 
method. By default it's connected to terminal printf-style functions. When 
setup_browser() is called, it'll be overridden to approriate ones. This way, 
we don't need to care about the timing when this function is called - since 
there's a time gap between use_browser is set (by config parser) and 
setup_browser() called, thus simply checking the variable can cause a nasty 
problem IMHO. Also python binding should have its own implementation - I have 
no idea on this :).

I think we should stop using fprintf/perror from now on and convert existing 
ones to generic ones. The generic code have to use above functions only, 
although UI specific code can use its own ones. The die, error, warning and 
usage are easy to convert to use generic functions since they're implemented 
by pointer already. And finally ui__error can remove terminal support(?).

This is a rough idea and there can be things I am missing. So I need to listen 
to your advices.

Thanks,
Namhyung

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists