public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung.kim@lge.com>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@elte.hu>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Paul Mackerras <paulus@samba.org>,
	David Ahern <dsahern@gmail.com>,
	Namhyung Kim <namhyung@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: [RFD] perf: Integrate logging facilities
Date: Mon, 12 Mar 2012 18:58:49 +0900	[thread overview]
Message-ID: <4F5DC8D9.9030608@lge.com> (raw)

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


             reply	other threads:[~2012-03-12  9:59 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-12  9:58 Namhyung Kim [this message]
2012-03-12 15:52 ` [RFD] perf: Integrate logging facilities David Ahern
2012-03-13  1:08   ` Namhyung Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F5DC8D9.9030608@lge.com \
    --to=namhyung.kim@lge.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=dsahern@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=namhyung@gmail.com \
    --cc=paulus@samba.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox