From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757868AbZCYNGU (ORCPT ); Wed, 25 Mar 2009 09:06:20 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754725AbZCYNGM (ORCPT ); Wed, 25 Mar 2009 09:06:12 -0400 Received: from ey-out-2122.google.com ([74.125.78.24]:29805 "EHLO ey-out-2122.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754294AbZCYNGL (ORCPT ); Wed, 25 Mar 2009 09:06:11 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; b=ZkhkaSOtideloGVV0RJbQ5N/FYl06NutAEIwxfI0Me5znbi3jGFugp3wm+EgOYajUi QeYBlQ0+PYAPOo2FLZDbCBuuFUCB2xgUO9QFVAWodfwvIbX85sf/xnwar7MrUguaAmrS lSo2dAvL0fbPOB64SgMvh7vFx/fGwnTu4hi1c= Date: Wed, 25 Mar 2009 14:06:04 +0100 From: Frederic Weisbecker To: Ingo Molnar Cc: Mathieu Desnoyers , Rusty Russell , akpm@linux-foundation.org, linux-kernel@vger.kernel.org, ltt-dev@lists.casi.polymtl.ca, Masami Hiramatsu , Peter Zijlstra , "Frank Ch. Eigler" , Hideo AOKI , Takashi Nishiie , Steven Rostedt , Eduard - Gabriel Munteanu Subject: Re: [patch 7/9] LTTng instrumentation - kernel Message-ID: <20090325130603.GD5976@nowhere> References: <20090324155625.420966314@polymtl.ca> <20090324160148.872305963@polymtl.ca> <20090324183313.GH31117@elte.hu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20090324183313.GH31117@elte.hu> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Mar 24, 2009 at 07:33:13PM +0100, Ingo Molnar wrote: > > (Rusty Cc:-ed - for the module.c tracepoints below) > > * Mathieu Desnoyers wrote: > > > Instrument the core kernel : module load/free and printk events. It helps the > > tracer to keep track of module related events and to export valuable printk > > information into the traces. > > > > Those tracepoints are used by LTTng. > > > > About the performance impact of tracepoints (which is comparable to markers), > > even without immediate values optimizations, tests done by Hideo Aoki on ia64 > > show no regression. His test case was using hackbench on a kernel where > > scheduler instrumentation (about 5 events in code scheduler code) was added. > > See the "Tracepoints" patch header for performance result detail. > > > > Signed-off-by: Mathieu Desnoyers > > CC: 'Ingo Molnar' > > CC: Frederic Weisbecker > > CC: Andrew Morton > > CC: Masami Hiramatsu > > CC: 'Peter Zijlstra' > > CC: "Frank Ch. Eigler" > > CC: 'Hideo AOKI' > > CC: Takashi Nishiie > > CC: 'Steven Rostedt' > > CC: Eduard - Gabriel Munteanu > > --- > > include/trace/kernel.h | 19 +++++++++++++++++++ > > kernel/module.c | 8 ++++++++ > > kernel/printk.c | 7 +++++++ > > 3 files changed, 34 insertions(+) > > > > Index: linux-2.6-lttng/kernel/printk.c > > =================================================================== > > --- linux-2.6-lttng.orig/kernel/printk.c 2009-03-24 09:09:52.000000000 -0400 > > +++ linux-2.6-lttng/kernel/printk.c 2009-03-24 09:31:53.000000000 -0400 > > @@ -32,6 +32,7 @@ > > #include > > #include > > #include > > +#include > > > > #include > > > > @@ -59,6 +60,7 @@ int console_printk[4] = { > > MINIMUM_CONSOLE_LOGLEVEL, /* minimum_console_loglevel */ > > DEFAULT_CONSOLE_LOGLEVEL, /* default_console_loglevel */ > > }; > > +EXPORT_SYMBOL_GPL(console_printk); > > > > /* > > * Low level drivers may need that to know if they can schedule in > > @@ -128,6 +130,9 @@ EXPORT_SYMBOL(console_set_on_cmdline); > > /* Flag: console code may call schedule() */ > > static int console_may_schedule; > > > > +DEFINE_TRACE(kernel_printk); > > +DEFINE_TRACE(kernel_vprintk); > > + > > #ifdef CONFIG_PRINTK > > > > static char __log_buf[__LOG_BUF_LEN]; > > @@ -560,6 +565,7 @@ asmlinkage int printk(const char *fmt, . > > int r; > > > > va_start(args, fmt); > > + trace_kernel_printk(_RET_IP_); > > r = vprintk(fmt, args); > > va_end(args); > > > > @@ -667,6 +673,7 @@ asmlinkage int vprintk(const char *fmt, > > printed_len += vscnprintf(printk_buf + printed_len, > > sizeof(printk_buf) - printed_len, fmt, args); > > > > + trace_kernel_vprintk(_RET_IP_, printk_buf, printed_len); > > So here we pass in the formatted output. What sense does it make to > have the above printk tracepoint? Little i think. > > Also, i'm not entirely convinced about the wiseness of instrumenting > an essential debug facility like printk(). Lets keep this one at the > tail portion of the patch-queue, ok? Especially the trace_kernel_printk hook which only probes the printk callers. I don't think a performance measurement of a printk call in that relevant. Concerning trace_kernel_vprintk(), if the goal is to capture the printk messages, I would rather see it through the dynamic printk facility or setting a console which route printk output to trace_printk(). If that is useful for someone. Thanks, Frederic. > > Index: linux-2.6-lttng/kernel/module.c > > =================================================================== > > --- linux-2.6-lttng.orig/kernel/module.c 2009-03-24 09:09:59.000000000 -0400 > > +++ linux-2.6-lttng/kernel/module.c 2009-03-24 09:31:53.000000000 -0400 > > @@ -51,6 +51,7 @@ > > #include > > #include > > #include > > +#include > > > > #if 0 > > #define DEBUGP printk > > @@ -78,6 +79,9 @@ static BLOCKING_NOTIFIER_HEAD(module_not > > /* Bounds of module allocation, for speeding __module_text_address */ > > static unsigned long module_addr_min = -1UL, module_addr_max = 0; > > > > +DEFINE_TRACE(kernel_module_load); > > +DEFINE_TRACE(kernel_module_free); > > I believe that to have a complete picture of module usage, module > refcount get/put events should be included as well, beyond the basic > load/free events. > > These both have performance impact (a module get/put in a fastpath > hurts scalability), and are informative in terms of establishing the > module dependency graph. > > Another thing that is iteresting and which is not covered here are > module request events and usermode helper callouts. These too have > instrumentation and performance analysis uses. > > Plus, here too it would be desired to put in default probes as well, > via TRACE_EVENT(). > > Thanks, > > Ingo