Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758169AbYGCVtX (ORCPT ); Thu, 3 Jul 2008 17:49:23 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1757473AbYGCVtB (ORCPT ); Thu, 3 Jul 2008 17:49:01 -0400 Received: from mx1.redhat.com ([66.187.233.31]:54737 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757158AbYGCVtA (ORCPT ); Thu, 3 Jul 2008 17:49:00 -0400 Message-ID: <486D48E0.9010708@redhat.com> Date: Thu, 03 Jul 2008 17:47:12 -0400 From: Masami Hiramatsu User-Agent: Thunderbird 2.0.0.14 (X11/20080501) MIME-Version: 1.0 To: Mathieu Desnoyers CC: akpm@linux-foundation.org, Ingo Molnar , linux-kernel@vger.kernel.org, Peter Zijlstra , "Frank Ch. Eigler" , Hideo AOKI , Takashi Nishiie , Steven Rostedt , Alexander Viro Subject: Re: [RFC patch 1/3] Kernel Tracepoints References: <20080703160526.637679319@polymtl.ca> <20080703161102.721606976@polymtl.ca> In-Reply-To: <20080703161102.721606976@polymtl.ca> X-Enigmail-Version: 0.95.6 Content-Type: text/plain; charset=ISO-2022-JP Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7846 Lines: 229 Hi Mathieu, I couldn't apply this patch against 2.6.26-rc8, and have some trivial comments. Mathieu Desnoyers wrote: > Implementation of kernel tracepoints. Inspired from the Linux Kernel Markers. > > Allows complete typing verification. No format string required. > > TODO : Documentation/tracepoint.txt > > Changelog : > - Use #name ":" #proto as string to identify the tracepoint in the > tracepoint table. This will make sure not type mismatch happens due to > connexion of a probe with the wrong type to a tracepoint declared with > the same name in a different header. > > Signed-off-by: Mathieu Desnoyers > CC: 'Peter Zijlstra' > CC: "Frank Ch. Eigler" > CC: 'Ingo Molnar' > CC: 'Hideo AOKI' > CC: Takashi Nishiie > CC: 'Steven Rostedt' > CC: Alexander Viro > --- > include/asm-generic/vmlinux.lds.h | 6 > include/linux/module.h | 17 + > include/linux/tracepoint.h | 139 ++++++++++ > init/Kconfig | 16 + > kernel/Makefile | 1 > kernel/module.c | 66 ++++- > kernel/tracepoint.c | 498 ++++++++++++++++++++++++++++++++++++++ > 7 files changed, 741 insertions(+), 2 deletions(-) > > Index: linux-2.6-lttng/init/Kconfig > =================================================================== > --- linux-2.6-lttng.orig/init/Kconfig 2008-07-03 11:47:15.000000000 -0400 > +++ linux-2.6-lttng/init/Kconfig 2008-07-03 11:49:54.000000000 -0400 > @@ -782,12 +782,28 @@ config PROFILING > Say Y here to enable the extended profiling support mechanisms used > by profilers such as OProfile. > > +config TRACEPOINTS > + bool "Activate tracepoints" > + default y > + help > + Place an empty function call at each tracepoint site. Can be > + dynamically changed for a probe function. > + > config MARKERS > bool "Activate markers" > help > Place an empty function call at each marker site. Can be > dynamically changed for a probe function. > > +config TRACEPROBES > + tristate "Compile generic tracing probes" > + depends on MARKERS > + default y > + help > + Compile generic tracing probes, which connect to the tracepoints when > + loaded and format the information collected by the tracepoints with > + the Markers. > + > source "arch/Kconfig" might this part better go to PATCH 3/3? [...] > + > + > +#define TPPROTO(args...) args > +#define TPARGS(args...) args > + > +#ifdef CONFIG_TRACEPOINTS > + > +/* > + * Note : the empty asm volatile with read constraint is used here instead of a > + * "used" attribute to fix a gcc 4.1.x bug. There seems no empty asm... > + * Make sure the alignment of the structure in the __tracepoints section will > + * not add unwanted padding between the beginning of the section and the > + * structure. Force alignment to the same alignment as the section start. > + */ > +#define DEFINE_TRACE(name, proto, args) \ > + static inline void _do_trace_##name(struct tracepoint *tp, proto) \ > + { \ > + int i; \ > + struct tracepoint_probe_closure *multi; \ > + preempt_disable(); \ > + multi = tp->multi; \ > + smp_read_barrier_depends(); \ > + if (multi) { \ > + for (i = 0; multi[i].func; i++) { \ > + ((void(*)(void *private_data, proto)) \ > + (multi[i].func))(multi[i].probe_private, args);\ > + } \ > + } \ > + preempt_enable(); \ > + } \ > + static inline void trace_##name(proto) \ > + { \ > + static const char __tpstrtab_##name[] \ > + __attribute__((section("__tracepoints_strings"))) \ > + = #name ":" #proto; \ > + static struct tracepoint __tracepoint_##name \ > + __attribute__((section("__tracepoints"), aligned(8))) = \ > + { __tpstrtab_##name, 0, NULL }; \ > + if (unlikely(__tracepoint_##name.state)) \ > + _do_trace_##name(&__tracepoint_##name, args); \ > + } \ > + static inline int register_trace_##name( \ > + void (*probe)(void *private_data, proto), \ > + void *private_data) \ > + { \ > + return tracepoint_probe_register(#name ":" #proto, \ > + (void *)probe, private_data); \ > + } \ > + static inline void unregister_trace_##name( \ > + void (*probe)(void *private_data, proto), \ > + void *private_data) \ > + { \ > + tracepoint_probe_unregister(#name ":" #proto, \ > + (void *)probe, private_data); \ > + } > + As we are discussing on another thread, this macro can't handle non-argument tracepoint. [...] > Index: linux-2.6-lttng/kernel/tracepoint.c > =================================================================== > --- /dev/null 1970-01-01 00:00:00.000000000 +0000 > +++ linux-2.6-lttng/kernel/tracepoint.c 2008-07-03 11:54:30.000000000 -0400 > @@ -0,0 +1,498 @@ > +/* > + * Copyright (C) 2007 Mathieu Desnoyers trivial: it should be 2008? :-) [...] > Index: linux-2.6-lttng/kernel/module.c > =================================================================== > --- linux-2.6-lttng.orig/kernel/module.c 2008-07-03 11:49:54.000000000 -0400 > +++ linux-2.6-lttng/kernel/module.c 2008-07-03 11:54:01.000000000 -0400 > @@ -46,6 +46,7 @@ > #include > #include > #include > +#include > > #if 0 > #define DEBUGP printk > @@ -1770,6 +1771,8 @@ static struct module *load_module(void _ > unsigned int unusedgplcrcindex; > unsigned int markersindex; > unsigned int markersstringsindex; > + unsigned int tracepointsindex; > + unsigned int tracepointsstringsindex; > struct module *mod; > long err = 0; > void *percpu = NULL, *ptr = NULL; /* Stops spurious gcc warning */ > @@ -2049,6 +2052,9 @@ static struct module *load_module(void _ > markersindex = find_sec(hdr, sechdrs, secstrings, "__markers"); > markersstringsindex = find_sec(hdr, sechdrs, secstrings, > "__markers_strings"); > + tracepointsindex = find_sec(hdr, sechdrs, secstrings, "__tracepoints"); > + tracepointsstringsindex = find_sec(hdr, sechdrs, secstrings, > + "__tracepoints_strings"); > > /* Now do relocations. */ > for (i = 1; i < hdr->e_shnum; i++) { > @@ -2076,6 +2082,12 @@ static struct module *load_module(void _ > mod->num_markers = > sechdrs[markersindex].sh_size / sizeof(*mod->markers); > #endif > +#ifdef CONFIG_TRACEPOINTS > + mod->tracepoints = (void *)sechdrs[tracepointsindex].sh_addr; > + mod->num_tracepoints = > + sechdrs[tracepointsindex].sh_size / sizeof(*mod->tracepoints); > +#endif > + > > /* Find duplicate symbols */ > err = verify_export_symbols(mod); > @@ -2094,11 +2106,16 @@ static struct module *load_module(void _ > > add_kallsyms(mod, sechdrs, symindex, strindex, secstrings); > > + if (!(mod->taints & TAINT_FORCED_MODULE)) { > #ifdef CONFIG_MARKERS > - if (!(mod->taints & TAINT_FORCED_MODULE)) > marker_update_probe_range(mod->markers, > mod->markers + mod->num_markers); Here, this hunk was rejected, because "Markers Support for Proprierary Modules" patch doesn't merged yet. > #endif > +#ifdef CONFIG_TRACEPOINTS > + tracepoint_update_probe_range(mod->tracepoints, > + mod->tracepoints + mod->num_tracepoints); > +#endif > + } > err = module_finalize(hdr, sechdrs, mod); > if (err < 0) > goto cleanup; Thank you, -- Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: mhiramat@redhat.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/