Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755041Ab0D1U6Y (ORCPT ); Wed, 28 Apr 2010 16:58:24 -0400 Received: from tomts20.bellnexxia.net ([209.226.175.74]:59429 "EHLO tomts20-srv.bellnexxia.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753589Ab0D1U6S (ORCPT ); Wed, 28 Apr 2010 16:58:18 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AvsEAAs92EtGGNqG/2dsb2JhbACce3K+MYUOBA Date: Wed, 28 Apr 2010 16:58:03 -0400 From: Mathieu Desnoyers To: Steven Rostedt Cc: linux-kernel@vger.kernel.org, Ingo Molnar , Andrew Morton , Thomas Gleixner , Peter Zijlstra , Frederic Weisbecker , Arnaldo Carvalho de Melo , Lai Jiangshan , Li Zefan , Masami Hiramatsu , Christoph Hellwig , Mathieu Desnoyers , Tom Zanussi Subject: Re: [PATCH 05/10][RFC] tracing: Move fields from event to class structure Message-ID: <20100428205803.GF8591@Krystal> References: <20100426195024.256424113@goodmis.org> <20100426200242.470116062@goodmis.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline In-Reply-To: <20100426200242.470116062@goodmis.org> X-Editor: vi X-Info: http://krystal.dyndns.org:8080 X-Operating-System: Linux/2.6.27.31-grsec (i686) X-Uptime: 16:48:35 up 21 days, 6:42, 4 users, load average: 0.48, 0.40, 0.27 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 Content-Length: 20808 Lines: 546 * Steven Rostedt (rostedt@goodmis.org) wrote: > From: Steven Rostedt > > Move the defined fields from the event to the class structure. > Since the fields of the event are defined by the class they belong > to, it makes sense to have the class hold the information instead > of the individual events. The events of the same class would just > hold duplicate information. > > After this change the size of the kernel dropped another 8K: > > text data bss dec hex filename > 5788186 1337252 9351592 16477030 fb6b66 vmlinux.orig > 5774316 1306580 9351592 16432488 fabd68 vmlinux.reg > 5774503 1297492 9351592 16423587 fa9aa3 vmlinux.fields > > Although the text increased, this was mainly due to the C files > having to adapt to the change. This is a constant increase, where > new tracepoints will not increase the Text. But the big drop is > in the data size (as well as needed allocations to hold the fields). > This will give even more savings as more tracepoints are created. > > Note, if just TRACE_EVENT()s are used and not DECLARE_EVENT_CLASS() > with several DEFINE_EVENT()s, then the savings will be lost. But > we are pushing developers to consolidate events with DEFINE_EVENT() > so this should not be an issue. > > The kprobes define a unique class to every new event, but are dynamic > so it should not be a issue. > > The syscalls however have a single class but the fields for the individual > events are different. The syscalls use a metadata to define the > fields. I moved the fields list from the event to the metadata and > added a "get_fields()" function to the class. This function is used > to find the fields. For normal events and kprobes, get_fields() just > returns a pointer to the fields list_head in the class. For syscall > events, it returns the fields list_head in the metadata for the event. So, playing catch-up here, why don't we simply put each syscall event in their own class ? We could possibly share the class where it makes sense (e.g. exact same fields). With the per-class sub-metadata, what's the limitations we have to expect with these system call events ? Can we map to a field size directly from the event ID, or do we have to somehow have the event size encoded in the header to make sense of the payload ? Thanks, Mathieu > > Cc: Mathieu Desnoyers > Cc: Masami Hiramatsu > Cc: Tom Zanussi > Cc: Peter Zijlstra > Signed-off-by: Steven Rostedt > --- > include/linux/ftrace_event.h | 5 ++- > include/linux/syscalls.h | 12 +++++----- > include/trace/ftrace.h | 10 +++++--- > include/trace/syscall.h | 3 +- > kernel/trace/trace.h | 3 ++ > kernel/trace/trace_events.c | 43 +++++++++++++++++++++++++++++++----- > kernel/trace/trace_events_filter.c | 10 +++++--- > kernel/trace/trace_export.c | 14 ++++++------ > kernel/trace/trace_kprobe.c | 8 +++--- > kernel/trace/trace_syscalls.c | 23 ++++++++++++++++--- > 10 files changed, 92 insertions(+), 39 deletions(-) > > diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h > index dd0051e..1e2c8f5 100644 > --- a/include/linux/ftrace_event.h > +++ b/include/linux/ftrace_event.h > @@ -128,6 +128,9 @@ struct ftrace_event_class { > void *perf_probe; > int (*reg)(struct ftrace_event_call *event, > enum trace_reg type); > + int (*define_fields)(struct ftrace_event_call *); > + struct list_head *(*get_fields)(struct ftrace_event_call *); > + struct list_head fields; > }; > > struct ftrace_event_call { > @@ -140,8 +143,6 @@ struct ftrace_event_call { > int id; > const char *print_fmt; > int (*raw_init)(struct ftrace_event_call *); > - int (*define_fields)(struct ftrace_event_call *); > - struct list_head fields; > int filter_active; > struct event_filter *filter; > void *mod; > diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h > index e3348c4..ef4f81c 100644 > --- a/include/linux/syscalls.h > +++ b/include/linux/syscalls.h > @@ -122,7 +122,7 @@ extern struct ftrace_event_class event_class_syscall_enter; > extern struct ftrace_event_class event_class_syscall_exit; > > #define SYSCALL_TRACE_ENTER_EVENT(sname) \ > - static const struct syscall_metadata __syscall_meta_##sname; \ > + static struct syscall_metadata __syscall_meta_##sname; \ > static struct ftrace_event_call \ > __attribute__((__aligned__(4))) event_enter_##sname; \ > static struct trace_event enter_syscall_print_##sname = { \ > @@ -136,12 +136,11 @@ extern struct ftrace_event_class event_class_syscall_exit; > .class = &event_class_syscall_enter, \ > .event = &enter_syscall_print_##sname, \ > .raw_init = init_syscall_trace, \ > - .define_fields = syscall_enter_define_fields, \ > .data = (void *)&__syscall_meta_##sname,\ > } > > #define SYSCALL_TRACE_EXIT_EVENT(sname) \ > - static const struct syscall_metadata __syscall_meta_##sname; \ > + static struct syscall_metadata __syscall_meta_##sname; \ > static struct ftrace_event_call \ > __attribute__((__aligned__(4))) event_exit_##sname; \ > static struct trace_event exit_syscall_print_##sname = { \ > @@ -155,14 +154,13 @@ extern struct ftrace_event_class event_class_syscall_exit; > .class = &event_class_syscall_exit, \ > .event = &exit_syscall_print_##sname, \ > .raw_init = init_syscall_trace, \ > - .define_fields = syscall_exit_define_fields, \ > .data = (void *)&__syscall_meta_##sname,\ > } > > #define SYSCALL_METADATA(sname, nb) \ > SYSCALL_TRACE_ENTER_EVENT(sname); \ > SYSCALL_TRACE_EXIT_EVENT(sname); \ > - static const struct syscall_metadata __used \ > + static struct syscall_metadata __used \ > __attribute__((__aligned__(4))) \ > __attribute__((section("__syscalls_metadata"))) \ > __syscall_meta_##sname = { \ > @@ -172,12 +170,13 @@ extern struct ftrace_event_class event_class_syscall_exit; > .args = args_##sname, \ > .enter_event = &event_enter_##sname, \ > .exit_event = &event_exit_##sname, \ > + .fields = LIST_HEAD_INIT(__syscall_meta_##sname.fields), \ > }; > > #define SYSCALL_DEFINE0(sname) \ > SYSCALL_TRACE_ENTER_EVENT(_##sname); \ > SYSCALL_TRACE_EXIT_EVENT(_##sname); \ > - static const struct syscall_metadata __used \ > + static struct syscall_metadata __used \ > __attribute__((__aligned__(4))) \ > __attribute__((section("__syscalls_metadata"))) \ > __syscall_meta__##sname = { \ > @@ -185,6 +184,7 @@ extern struct ftrace_event_class event_class_syscall_exit; > .nb_args = 0, \ > .enter_event = &event_enter__##sname, \ > .exit_event = &event_exit__##sname, \ > + .fields = LIST_HEAD_INIT(__syscall_meta__##sname.fields), \ > }; \ > asmlinkage long sys_##sname(void) > #else > diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h > index 62fe622..e6ec392 100644 > --- a/include/trace/ftrace.h > +++ b/include/trace/ftrace.h > @@ -429,6 +429,9 @@ static inline notrace int ftrace_get_offsets_##call( \ > * > * static struct ftrace_event_class __used event_class_