lkml.org 
[lkml]   [2015]   [Mar]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC][PATCH 02/10] tracing: Allow for modules to export their trace enums as well
    On Fri, Mar 27, 2015 at 05:37:06PM -0400, Steven Rostedt wrote:
    > From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
    >
    > Update the infrastructure such that modules that declare TRACE_DEFINE_ENUM()
    > will have those enums shown in the enum_map files in the tracing directory.
    >
    > Cc: Rusty Russell <rusty@rustcorp.com.au>
    > Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
    > ---
    > include/linux/module.h | 2 +
    > kernel/module.c | 3 +
    > kernel/trace/trace.c | 184 ++++++++++++++++++++++++++++++++++++++++++++-----
    > 3 files changed, 173 insertions(+), 16 deletions(-)
    >
    > diff --git a/include/linux/module.h b/include/linux/module.h
    > index 42999fe2dbd0..53dc41dd5c62 100644
    > --- a/include/linux/module.h
    > +++ b/include/linux/module.h
    > @@ -338,6 +338,8 @@ struct module {
    > #ifdef CONFIG_EVENT_TRACING
    > struct ftrace_event_call **trace_events;
    > unsigned int num_trace_events;
    > + struct trace_enum_map **trace_enums;
    > + unsigned int num_trace_enums;
    > #endif
    > #ifdef CONFIG_FTRACE_MCOUNT_RECORD
    > unsigned int num_ftrace_callsites;
    > diff --git a/kernel/module.c b/kernel/module.c
    > index b3d634ed06c9..d8f8ab271c2b 100644
    > --- a/kernel/module.c
    > +++ b/kernel/module.c
    > @@ -2753,6 +2753,9 @@ static int find_module_sections(struct module *mod, struct load_info *info)
    > mod->trace_events = section_objs(info, "_ftrace_events",
    > sizeof(*mod->trace_events),
    > &mod->num_trace_events);
    > + mod->trace_enums = section_objs(info, "_ftrace_enum_map",
    > + sizeof(*mod->trace_enums),
    > + &mod->num_trace_enums);
    > #endif
    > #ifdef CONFIG_TRACING
    > mod->trace_bprintk_fmt_start = section_objs(info, "__trace_printk_fmt",
    > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
    > index 53b449c522a7..97031878eb3d 100644
    > --- a/kernel/trace/trace.c
    > +++ b/kernel/trace/trace.c
    > @@ -124,7 +124,38 @@ enum ftrace_dump_mode ftrace_dump_on_oops;
    > int __disable_trace_on_warning;
    >
    > /* Map of enums to their values, for "enum_map" file */
    > -static struct trace_enum_map *trace_enum_maps;
    > +struct trace_enum_map_head {
    > + struct module *mod;
    > + unsigned long length;
    > +};
    > +
    > +union trace_enum_map_item;
    > +
    > +struct trace_enum_map_tail {
    > + /*
    > + * "end" is first and points to NULL as it must be different
    > + * than "mod" or "enum_string"
    > + */
    > + const char *end; /* points to NULL */
    > + union trace_enum_map_item *next;
    > +};
    > +
    > +static DEFINE_MUTEX(trace_enum_mutex);
    > +
    > +/*
    > + * The trace_enum_maps are saved in an array whith two extra elements,

    s/whith/with/

    > + * one at the beginning, and one at the end. The beginning item contains
    > + * the count of the saved maps (head.length), and the module they
    > + * belong to if not built in (head.mod). The ending item contains a
    > + * pointer to the next array of saved enum_map items.
    > + */
    > +union trace_enum_map_item {
    > + struct trace_enum_map map;
    > + struct trace_enum_map_head head;
    > + struct trace_enum_map_tail tail;
    > +};
    > +
    > +static union trace_enum_map_item *trace_enum_maps;
    >
    > static int tracing_set_tracer(struct trace_array *tr, const char *buf);
    >
    > @@ -3911,29 +3942,47 @@ static const struct file_operations tracing_saved_cmdlines_size_fops = {
    > .write = tracing_saved_cmdlines_size_write,
    > };
    >
    > +static union trace_enum_map_item *
    > +update_enum_map(union trace_enum_map_item *ptr)
    > +{
    > + if (!ptr->map.enum_string) {
    > + if (ptr->tail.next) {
    > + ptr = ptr->tail.next;
    > + ptr++;

    I guess this additional increment is to skip head item. Maybe it
    deserves a comment. :)


    > + } else
    > + return NULL;
    > + }
    > + return ptr;
    > +}
    > +
    > static void *enum_map_next(struct seq_file *m, void *v, loff_t *pos)
    > {
    > - struct trace_enum_map *ptr = v;
    > + union trace_enum_map_item *ptr = v;
    >
    > - if (!ptr->enum_string)
    > + ptr = update_enum_map(ptr);

    Again, is this necessary?


    > + if (!ptr)
    > return NULL;
    >
    > ptr++;
    >
    > (*pos)++;
    >
    > - if (!ptr->enum_string)
    > - return NULL;
    > + ptr = update_enum_map(ptr);
    >
    > return ptr;
    > }
    >
    > static void *enum_map_start(struct seq_file *m, loff_t *pos)
    > {
    > - struct trace_enum_map *v;
    > + union trace_enum_map_item *v;
    > loff_t l = 0;
    >
    > + mutex_lock(&trace_enum_mutex);
    > +
    > v = trace_enum_maps;
    > + if (v)
    > + v++;
    > +
    > while (v && l < *pos) {
    > v = enum_map_next(m, v, &l);
    > }
    > @@ -3943,13 +3992,15 @@ static void *enum_map_start(struct seq_file *m, loff_t *pos)
    >
    > static void enum_map_stop(struct seq_file *m, void *v)
    > {
    > + mutex_unlock(&trace_enum_mutex);
    > }
    >
    > static int enum_map_show(struct seq_file *m, void *v)
    > {
    > - struct trace_enum_map *ptr = v;
    > + union trace_enum_map_item *ptr = v;
    > +
    > + seq_printf(m, "%s %ld\n", ptr->map.enum_string, ptr->map.enum_value);
    >
    > - seq_printf(m, "%s %ld\n", ptr->enum_string, ptr->enum_value);
    > return 0;
    > }
    >
    > @@ -3975,29 +4026,56 @@ static const struct file_operations tracing_enum_map_fops = {
    > .release = seq_release,
    > };
    >
    > -static void
    > -trace_insert_enum_map(struct trace_enum_map **start, struct trace_enum_map **stop)
    > +static void trace_insert_enum_map(struct module *mod,
    > + struct trace_enum_map **start, int len)
    > {
    > + struct trace_enum_map **stop;
    > struct trace_enum_map **map;
    > - struct trace_enum_map *map_array;
    > - int len = stop - start;
    > + union trace_enum_map_item *map_array;
    > + union trace_enum_map_item *ptr;
    >
    > if (len <= 0)
    > return;
    >
    > - map_array = kmalloc(sizeof(*map_array) * (len + 1), GFP_KERNEL);
    > + stop = start + len;
    > +
    > + /*
    > + * The trace_enum_maps contains the map plus a head and tail item,
    > + * where the head holds the module and length of array, and the
    > + * tail holds a pointer to the next list.
    > + */
    > + map_array = kmalloc(sizeof(*map_array) * (len + 2), GFP_KERNEL);
    > if (!map_array) {
    > pr_warning("Unable to allocate trace enum mapping\n");
    > return;
    > }
    >
    > - trace_enum_maps = map_array;
    > + mutex_lock(&trace_enum_mutex);
    > +
    > + if (!trace_enum_maps)
    > + trace_enum_maps = map_array;
    > + else {
    > + ptr = trace_enum_maps;
    > + for (;;) {
    > + ptr = ptr + ptr->head.length + 1;

    Looks like access to tail item is used frequently. How about adding a
    helper function like enum_maps_tail_item()?

    Thanks,
    Namhyung

    > + if (!ptr->tail.next)
    > + break;
    > + ptr = ptr->tail.next;
    > +
    > + }
    > + ptr->tail.next = map_array;
    > + }
    > + map_array->head.mod = mod;
    > + map_array->head.length = len;
    > + map_array++;
    >
    > for (map = start; (unsigned long)map < (unsigned long)stop; map++) {
    > - *map_array = **map;
    > + map_array->map = **map;
    > map_array++;
    > }
    > memset(map_array, 0, sizeof(*map_array));
    > +
    > + mutex_unlock(&trace_enum_mutex);
    > }
    >
    >
    > @@ -6640,9 +6718,79 @@ extern struct trace_enum_map *__stop_ftrace_enum_maps[];
    >
    > static void __init trace_enum_init(void)
    > {
    > - trace_insert_enum_map(__start_ftrace_enum_maps, __stop_ftrace_enum_maps);
    > + int len;
    > +
    > + len = __stop_ftrace_enum_maps - __start_ftrace_enum_maps;
    > + trace_insert_enum_map(NULL, __start_ftrace_enum_maps, len);
    > }
    >
    > +#ifdef CONFIG_MODULES
    > +static void trace_module_add_enums(struct module *mod)
    > +{
    > + if (!mod->num_trace_enums)
    > + return;
    > +
    > + /*
    > + * Modules with bad taint do not have events created, do
    > + * not bother with enums either.
    > + */
    > + if (trace_module_has_bad_taint(mod))
    > + return;
    > +
    > + trace_insert_enum_map(mod, mod->trace_enums, mod->num_trace_enums);
    > +}
    > +
    > +static void trace_module_remove_enums(struct module *mod)
    > +{
    > + union trace_enum_map_item *map;
    > + union trace_enum_map_item **last = &trace_enum_maps;
    > +
    > + if (!mod->num_trace_enums)
    > + return;
    > +
    > + mutex_lock(&trace_enum_mutex);
    > +
    > + map = trace_enum_maps;
    > +
    > + while (map) {
    > + if (map->head.mod == mod)
    > + break;
    > + map += map->head.length + 1;
    > + last = &map->tail.next;
    > + map = map->tail.next;
    > + }
    > + if (!map)
    > + goto out;
    > +
    > + *last = map[map->head.length + 1].tail.next;
    > + kfree(map);
    > + out:
    > + mutex_unlock(&trace_enum_mutex);
    > +}
    > +
    > +static int trace_module_notify(struct notifier_block *self,
    > + unsigned long val, void *data)
    > +{
    > + struct module *mod = data;
    > +
    > + switch (val) {
    > + case MODULE_STATE_COMING:
    > + trace_module_add_enums(mod);
    > + break;
    > + case MODULE_STATE_GOING:
    > + trace_module_remove_enums(mod);
    > + break;
    > + }
    > +
    > + return 0;
    > +}
    > +
    > +static struct notifier_block trace_module_nb = {
    > + .notifier_call = trace_module_notify,
    > + .priority = 0,
    > +};
    > +#endif
    > +
    > static __init int tracer_init_debugfs(void)
    > {
    > struct dentry *d_tracer;
    > @@ -6672,6 +6820,10 @@ static __init int tracer_init_debugfs(void)
    > trace_create_file("enum_map", 0444, d_tracer,
    > NULL, &tracing_enum_map_fops);
    >
    > +#ifdef CONFIG_MODULES
    > + register_module_notifier(&trace_module_nb);
    > +#endif
    > +
    > #ifdef CONFIG_DYNAMIC_FTRACE
    > trace_create_file("dyn_ftrace_total_info", 0444, d_tracer,
    > &ftrace_update_tot_cnt, &tracing_dyn_info_fops);
    > --
    > 2.1.4
    >
    >
    > --
    > 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/


    \
     
     \ /
      Last update: 2015-03-30 05:21    [W:3.460 / U:0.712 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site