[RFD] perf: Integrate logging facilities

[RFD] perf: Integrate logging facilities

[Namhyung Kim]

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

Re: [RFD] perf: Integrate logging facilities

[David Ahern]

On 3/12/12 3:58 AM, Namhyung Kim wrote:
> This is a rough idea and there can be things I am missing. So I need to
> listen to your advices.

What Arnaldo proposed was having core, library code return error codes 
and not print anything to the user. Higher layers (commands only?) can 
take the return error code and pass it to perf_<subsystem>_strerror() to 
dump a user friendly string for the error. See

http://lkml.org/lkml/2012/2/13/302

David

Re: [RFD] perf: Integrate logging facilities

[Namhyung Kim]

Hi,

2012-03-13 12:52 AM, David Ahern wrote:
> On 3/12/12 3:58 AM, Namhyung Kim wrote:
>> This is a rough idea and there can be things I am missing. So I need to
>> listen to your advices.
>
> What Arnaldo proposed was having core, library code return error codes and not
> print anything to the user. Higher layers (commands only?) can take the return
> error code and pass it to perf_<subsystem>_strerror() to dump a user friendly
> string for the error. See
>
> http://lkml.org/lkml/2012/2/13/302
>

It will work well for error cases, but how about warnings? I mean the case of 
something like handling new features on old kernels. I want give a (warning) 
message to user but let the code keep going instead of returning to the user. 
It seems not possible on above scenario, so should it be handled at all in 
higher layers? Couldn't the core code have that kind of flexiblity?

And current library code has lots of calls to pr_debug*, dump_printf as well 
as (something-or-)die. How can we handle this? For debugging functions, it 
cannot be moved out of the library for obvious reasons. For die, it can be 
converted to return a error code, but it might require adding non-trivial 
error handling path especially for deeply nested code IMHO.

Also, I think my proposal can still be applied for higher layer codes. It'd be 
better to standardize logging facilities at least for high level in order to 
be flexible for future changes.

Thanks,
Namhyung