| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20100428205803.GF8591@Krystal>
Date: Wed, 28 Apr 2010 16:58:03 -0400
From: Mathieu Desnoyers <compudj@...stal.dyndns.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
Andrew Morton <akpm@...ux-foundation.org>,
Thomas Gleixner <tglx@...utronix.de>,
Peter Zijlstra <peterz@...radead.org>,
Frederic Weisbecker <fweisbec@...il.com>,
Arnaldo Carvalho de Melo <acme@...hat.com>,
Lai Jiangshan <laijs@...fujitsu.com>,
Li Zefan <lizf@...fujitsu.com>,
Masami Hiramatsu <mhiramat@...hat.com>,
Christoph Hellwig <hch@....de>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
Tom Zanussi <tzanussi@...il.com>
Subject: Re: [PATCH 05/10][RFC] tracing: Move fields from event to class
structure
* Steven Rostedt (rostedt@...dmis.org) wrote:
> From: Steven Rostedt <srostedt@...hat.com>
>
> 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 <mathieu.desnoyers@...icios.com>
> Cc: Masami Hiramatsu <mhiramat@...hat.com>
> Cc: Tom Zanussi <tzanussi@...il.com>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Signed-off-by: Steven Rostedt <rostedt@...dmis.org>
> ---
> 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_<template> = {
> * .system = "<system>",
> + * .define_fields = ftrace_define_fields_<call>,
> + .fields = LIST_HEAD_INIT(event_class_##call.fields),\
> + .probe = ftrace_raw_event_##call, \
missing * above.
> * }
> *
> * static struct ftrace_event_call __used
> @@ -437,10 +440,8 @@ static inline notrace int ftrace_get_offsets_##call( \
> * .name = "<call>",
> * .class = event_class_<template>,
> * .raw_init = trace_event_raw_init,
> - * .regfunc = ftrace_reg_event_<call>,
> - * .unregfunc = ftrace_unreg_event_<call>,
> + * .event = &ftrace_event_type_<call>,
> * .print_fmt = print_fmt_<call>,
> - * .define_fields = ftrace_define_fields_<call>,
> * }
> *
> */
> @@ -552,6 +553,8 @@ _TRACE_PERF_PROTO(call, PARAMS(proto)); \
> static const char print_fmt_##call[] = print; \
> static struct ftrace_event_class __used event_class_##call = { \
> .system = __stringify(TRACE_SYSTEM), \
> + .define_fields = ftrace_define_fields_##call, \
> + .fields = LIST_HEAD_INIT(event_class_##call.fields),\
> .probe = ftrace_raw_event_##call, \
> _TRACE_PERF_INIT(call) \
> }
> @@ -567,7 +570,6 @@ __attribute__((section("_ftrace_events"))) event_##call = { \
> .event = &ftrace_event_type_##call, \
> .raw_init = trace_event_raw_init, \
> .print_fmt = print_fmt_##template, \
> - .define_fields = ftrace_define_fields_##template, \
> }
>
> #undef DEFINE_EVENT_PRINT
> diff --git a/include/trace/syscall.h b/include/trace/syscall.h
> index e5e5f48..25087c3 100644
> --- a/include/trace/syscall.h
> +++ b/include/trace/syscall.h
> @@ -25,6 +25,7 @@ struct syscall_metadata {
> int nb_args;
> const char **types;
> const char **args;
> + struct list_head fields;
>
> struct ftrace_event_call *enter_event;
> struct ftrace_event_call *exit_event;
> @@ -34,8 +35,6 @@ struct syscall_metadata {
> extern unsigned long arch_syscall_addr(int nr);
> extern int init_syscall_trace(struct ftrace_event_call *call);
>
> -extern int syscall_enter_define_fields(struct ftrace_event_call *call);
> -extern int syscall_exit_define_fields(struct ftrace_event_call *call);
> extern int reg_event_syscall_enter(struct ftrace_event_call *call);
> extern void unreg_event_syscall_enter(struct ftrace_event_call *call);
> extern int reg_event_syscall_exit(struct ftrace_event_call *call);
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 2825ef2..ff63bee 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -771,6 +771,9 @@ extern void print_subsystem_event_filter(struct event_subsystem *system,
> struct trace_seq *s);
> extern int filter_assign_type(const char *type);
>
> +struct list_head *
> +trace_get_fields(struct ftrace_event_call *event_call);
> +
> static inline int
> filter_check_discard(struct ftrace_event_call *call, void *rec,
> struct ring_buffer *buffer,
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index f84cfcb..c31632e 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -28,11 +28,28 @@ DEFINE_MUTEX(event_mutex);
>
> LIST_HEAD(ftrace_events);
>
> +static int fields_done(struct ftrace_event_call *event_call)
> +{
> + return 0;
> +}
> +
> +struct list_head *
> +trace_get_fields(struct ftrace_event_call *event_call)
> +{
> + if (!event_call->class->get_fields)
> + return &event_call->class->fields;
> + return event_call->class->get_fields(event_call);
> +}
> +
> int trace_define_field(struct ftrace_event_call *call, const char *type,
> const char *name, int offset, int size, int is_signed,
> int filter_type)
> {
> struct ftrace_event_field *field;
> + struct list_head *head;
> +
> + if (WARN_ON(!call->class) || call->class->define_fields == fields_done)
> + return 0;
>
> field = kzalloc(sizeof(*field), GFP_KERNEL);
> if (!field)
> @@ -55,7 +72,8 @@ int trace_define_field(struct ftrace_event_call *call, const char *type,
> field->size = size;
> field->is_signed = is_signed;
>
> - list_add(&field->link, &call->fields);
> + head = trace_get_fields(call);
> + list_add(&field->link, head);
>
> return 0;
>
> @@ -81,6 +99,9 @@ static int trace_define_common_fields(struct ftrace_event_call *call)
> int ret;
> struct trace_entry ent;
>
> + if (call->class->define_fields == fields_done)
> + return 0;
> +
> __common_field(unsigned short, type);
> __common_field(unsigned char, flags);
> __common_field(unsigned char, preempt_count);
> @@ -93,8 +114,10 @@ static int trace_define_common_fields(struct ftrace_event_call *call)
> void trace_destroy_fields(struct ftrace_event_call *call)
> {
> struct ftrace_event_field *field, *next;
> + struct list_head *head;
>
> - list_for_each_entry_safe(field, next, &call->fields, link) {
> + head = trace_get_fields(call);
> + list_for_each_entry_safe(field, next, head, link) {
> list_del(&field->link);
> kfree(field->type);
> kfree(field->name);
> @@ -110,7 +133,6 @@ int trace_event_raw_init(struct ftrace_event_call *call)
> if (!id)
> return -ENODEV;
> call->id = id;
> - INIT_LIST_HEAD(&call->fields);
>
> return 0;
> }
> @@ -536,6 +558,7 @@ event_format_read(struct file *filp, char __user *ubuf, size_t cnt,
> {
> struct ftrace_event_call *call = filp->private_data;
> struct ftrace_event_field *field;
> + struct list_head *head;
> struct trace_seq *s;
> int common_field_count = 5;
> char *buf;
> @@ -554,7 +577,8 @@ event_format_read(struct file *filp, char __user *ubuf, size_t cnt,
> trace_seq_printf(s, "ID: %d\n", call->id);
> trace_seq_printf(s, "format:\n");
>
> - list_for_each_entry_reverse(field, &call->fields, link) {
> + head = trace_get_fields(call);
> + list_for_each_entry_reverse(field, head, link) {
> /*
> * Smartly shows the array type(except dynamic array).
> * Normal:
> @@ -954,10 +978,10 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events,
> trace_create_file("id", 0444, call->dir, call,
> id);
>
> - if (call->define_fields) {
> + if (call->class->define_fields) {
> ret = trace_define_common_fields(call);
> if (!ret)
> - ret = call->define_fields(call);
> + ret = call->class->define_fields(call);
> if (ret < 0) {
> pr_warning("Could not initialize trace point"
> " events/%s\n", call->name);
> @@ -965,6 +989,13 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events,
> }
> trace_create_file("filter", 0644, call->dir, call,
> filter);
> +
> + /*
> + * Other events with the same class will call
> + * define fields again, Set the define_fields
> + * to a stub, and it will be skipped.
> + */
> + call->class->define_fields = fields_done;
> }
>
> trace_create_file("format", 0444, call->dir, call,
> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index 22fa89f..560683d 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -499,8 +499,10 @@ static struct ftrace_event_field *
> find_event_field(struct ftrace_event_call *call, char *name)
> {
> struct ftrace_event_field *field;
> + struct list_head *head;
>
> - list_for_each_entry(field, &call->fields, link) {
> + head = trace_get_fields(call);
> + list_for_each_entry(field, head, link) {
> if (!strcmp(field->name, name))
> return field;
> }
> @@ -624,7 +626,7 @@ static int init_subsystem_preds(struct event_subsystem *system)
> int err;
>
> list_for_each_entry(call, &ftrace_events, list) {
> - if (!call->define_fields)
> + if (!call->class || !call->class->define_fields)
> continue;
>
> if (strcmp(call->class->system, system->name) != 0)
> @@ -643,7 +645,7 @@ static void filter_free_subsystem_preds(struct event_subsystem *system)
> struct ftrace_event_call *call;
>
> list_for_each_entry(call, &ftrace_events, list) {
> - if (!call->define_fields)
> + if (!call->class || !call->class->define_fields)
> continue;
>
> if (strcmp(call->class->system, system->name) != 0)
> @@ -1248,7 +1250,7 @@ static int replace_system_preds(struct event_subsystem *system,
> list_for_each_entry(call, &ftrace_events, list) {
> struct event_filter *filter = call->filter;
>
> - if (!call->define_fields)
> + if (!call->class || !call->class->define_fields)
> continue;
>
> if (strcmp(call->class->system, system->name) != 0)
> diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c
> index 7f16e21..e700a0c 100644
> --- a/kernel/trace/trace_export.c
> +++ b/kernel/trace/trace_export.c
> @@ -18,10 +18,6 @@
> #undef TRACE_SYSTEM
> #define TRACE_SYSTEM ftrace
>
> -struct ftrace_event_class event_class_ftrace = {
> - .system = __stringify(TRACE_SYSTEM),
> -};
> -
> /* not needed for this file */
> #undef __field_struct
> #define __field_struct(type, item)
> @@ -131,7 +127,7 @@ ftrace_define_fields_##name(struct ftrace_event_call *event_call) \
>
> static int ftrace_raw_init_event(struct ftrace_event_call *call)
> {
> - INIT_LIST_HEAD(&call->fields);
> + INIT_LIST_HEAD(&call->class->fields);
> return 0;
> }
>
> @@ -159,15 +155,19 @@ static int ftrace_raw_init_event(struct ftrace_event_call *call)
> #undef FTRACE_ENTRY
> #define FTRACE_ENTRY(call, struct_name, type, tstruct, print) \
> \
> +struct ftrace_event_class event_class_ftrace_##call = { \
> + .system = __stringify(TRACE_SYSTEM), \
> + .define_fields = ftrace_define_fields_##call, \
> +}; \
> + \
> struct ftrace_event_call __used \
> __attribute__((__aligned__(4))) \
> __attribute__((section("_ftrace_events"))) event_##call = { \
> .name = #call, \
> .id = type, \
> - .class = &event_class_ftrace, \
> + .class = &event_class_ftrace_##call, \
> .raw_init = ftrace_raw_init_event, \
> .print_fmt = print, \
> - .define_fields = ftrace_define_fields_##call, \
> }; \
>
> #include "trace_entries.h"
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index f8af21a..b14bf74 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -1112,8 +1112,6 @@ static void probe_event_disable(struct ftrace_event_call *call)
>
> static int probe_event_raw_init(struct ftrace_event_call *event_call)
> {
> - INIT_LIST_HEAD(&event_call->fields);
> -
> return 0;
> }
>
> @@ -1362,11 +1360,13 @@ static int register_probe_event(struct trace_probe *tp)
> if (probe_is_return(tp)) {
> tp->event.trace = print_kretprobe_event;
> call->raw_init = probe_event_raw_init;
> - call->define_fields = kretprobe_event_define_fields;
> + INIT_LIST_HEAD(&call->class->fields);
> + call->class->define_fields = kretprobe_event_define_fields;
> } else {
> tp->event.trace = print_kprobe_event;
> call->raw_init = probe_event_raw_init;
> - call->define_fields = kprobe_event_define_fields;
> + INIT_LIST_HEAD(&call->class->fields);
> + call->class->define_fields = kprobe_event_define_fields;
> }
> if (set_print_fmt(tp) < 0)
> return -ENOMEM;
> diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
> index c92934d..eb535ba 100644
> --- a/kernel/trace/trace_syscalls.c
> +++ b/kernel/trace/trace_syscalls.c
> @@ -19,14 +19,29 @@ static int syscall_enter_register(struct ftrace_event_call *event,
> static int syscall_exit_register(struct ftrace_event_call *event,
> enum trace_reg type);
>
> +static int syscall_enter_define_fields(struct ftrace_event_call *call);
> +static int syscall_exit_define_fields(struct ftrace_event_call *call);
> +
> +static struct list_head *
> +syscall_get_fields(struct ftrace_event_call *call)
> +{
> + struct syscall_metadata *entry = call->data;
> +
> + return &entry->fields;
> +}
> +
> struct ftrace_event_class event_class_syscall_enter = {
> .system = "syscalls",
> - .reg = syscall_enter_register
> + .reg = syscall_enter_register,
> + .define_fields = syscall_enter_define_fields,
> + .get_fields = syscall_get_fields,
> };
>
> struct ftrace_event_class event_class_syscall_exit = {
> .system = "syscalls",
> - .reg = syscall_exit_register
> + .reg = syscall_exit_register,
> + .define_fields = syscall_exit_define_fields,
> + .get_fields = syscall_get_fields,
> };
>
> extern unsigned long __start_syscalls_metadata[];
> @@ -219,7 +234,7 @@ static void free_syscall_print_fmt(struct ftrace_event_call *call)
> kfree(call->print_fmt);
> }
>
> -int syscall_enter_define_fields(struct ftrace_event_call *call)
> +static int syscall_enter_define_fields(struct ftrace_event_call *call)
> {
> struct syscall_trace_enter trace;
> struct syscall_metadata *meta = call->data;
> @@ -242,7 +257,7 @@ int syscall_enter_define_fields(struct ftrace_event_call *call)
> return ret;
> }
>
> -int syscall_exit_define_fields(struct ftrace_event_call *call)
> +static int syscall_exit_define_fields(struct ftrace_event_call *call)
> {
> struct syscall_trace_exit trace;
> int ret;
> --
> 1.7.0
>
>
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists