lkml.org 
[lkml]   [2009]   [Mar]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 1/4] tracing: add run-time field descriptions for event filtering
    Hi Tom,


    On Sun, Mar 22, 2009 at 03:30:39AM -0500, Tom Zanussi wrote:
    > This patch makes the field descriptions defined for event tracing
    > available at run-time, for the event-filtering mechanism introduced in a
    > subsequent patch.
    >
    > The common event fields are prepended with 'common_' in the format
    > display, allowing them to be distinguished from the other fields that
    > might internally have same name and can therefore be unambiguously used
    > in filters.
    >
    > Signed-off-by: Tom Zanussi <tzanussi@gmail.com>
    >
    > ---
    > kernel/trace/trace.h | 30 ++++++++++++++++-------
    > kernel/trace/trace_events.c | 40 ++++++++++++++++++++++++++++++-
    > kernel/trace/trace_events_stage_2.h | 45 +++++++++++++++++++++++++++++++++++
    > kernel/trace/trace_events_stage_3.h | 2 +
    > 4 files changed, 107 insertions(+), 10 deletions(-)
    >
    > diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
    > index 7cfb741..9288dc7 100644
    > --- a/kernel/trace/trace.h
    > +++ b/kernel/trace/trace.h
    > @@ -775,16 +775,26 @@ enum {
    > TRACE_EVENT_TYPE_RAW = 2,
    > };
    >
    > +struct ftrace_event_field {
    > + struct list_head link;
    > + char *name;
    > + char *type;
    > + int offset;
    > + int size;
    > +};
    > +
    > struct ftrace_event_call {
    > - char *name;
    > - char *system;
    > - struct dentry *dir;
    > - int enabled;
    > - int (*regfunc)(void);
    > - void (*unregfunc)(void);
    > - int id;
    > - int (*raw_init)(void);
    > - int (*show_format)(struct trace_seq *s);
    > + char *name;
    > + char *system;
    > + struct dentry *dir;
    > + int enabled;
    > + int (*regfunc)(void);
    > + void (*unregfunc)(void);
    > + int id;
    > + int (*raw_init)(void);
    > + int (*show_format)(struct trace_seq *s);
    > + int (*define_fields)(void);
    > + struct list_head fields;
    >
    > #ifdef CONFIG_EVENT_PROFILE
    > atomic_t profile_count;
    > @@ -793,6 +803,8 @@ struct ftrace_event_call {
    > #endif
    > };
    >
    > +int trace_define_field(struct ftrace_event_call *call, char *type,
    > + char *name, int offset, int size);
    > void event_trace_printk(unsigned long ip, const char *fmt, ...);
    > extern struct ftrace_event_call __start_ftrace_events[];
    > extern struct ftrace_event_call __stop_ftrace_events[];
    > diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
    > index 3047b56..961b057 100644
    > --- a/kernel/trace/trace_events.c
    > +++ b/kernel/trace/trace_events.c
    > @@ -19,6 +19,34 @@
    >
    > static DEFINE_MUTEX(event_mutex);
    >
    > +int trace_define_field(struct ftrace_event_call *call, char *type,
    > + char *name, int offset, int size)
    > +{
    > + struct ftrace_event_field *field;
    > +
    > + field = kmalloc(sizeof(*field), GFP_KERNEL);
    > + if (!field)
    > + goto err;
    > + field->name = kstrdup(name, GFP_KERNEL);
    > + if (!field->name)
    > + goto err;
    > + field->type = kstrdup(type, GFP_KERNEL);
    > + if (!field->type)
    > + goto err;
    > + field->offset = offset;
    > + field->size = size;
    > + list_add(&field->link, &call->fields);
    > +
    > + return 0;
    > +err:
    > + if (field) {
    > + kfree(field->name);
    > + kfree(field->type);


    You need kzalloc to allocate field.
    With kmalloc, field will point to random filled memory
    after a fresh allocation.

    Imagine this path:

    if (!field->name)
    goto err;
    ...
    err:
    kfree(field->name) <- field->name = NULL, correct
    kfree(field->type) <- field->type = ?


    > + }
    > + kfree(field);
    > + return -ENOMEM;
    > +}
    > +
    > static void ftrace_clear_events(void)
    > {
    > struct ftrace_event_call *call = (void *)__start_ftrace_events;
    > @@ -343,7 +371,8 @@ event_enable_write(struct file *filp, const char __user *ubuf, size_t cnt,
    >
    > #undef FIELD
    > #define FIELD(type, name) \
    > - #type, #name, offsetof(typeof(field), name), sizeof(field.name)
    > + #type, "common_" #name, offsetof(typeof(field), name), \
    > + sizeof(field.name)
    >
    > static int trace_write_header(struct trace_seq *s)
    > {
    > @@ -581,6 +610,15 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events)
    > call->name);
    > }
    >
    > + if (call->define_fields) {
    > + ret = call->define_fields();
    > + if (ret < 0) {
    > + pr_warning("Could not initialize trace point"
    > + " events/%s\n", call->name);
    > + return ret;
    > + }
    > + }
    > +
    > /* A trace may not want to export its format */
    > if (!call->show_format)
    > return 0;
    > diff --git a/kernel/trace/trace_events_stage_2.h b/kernel/trace/trace_events_stage_2.h
    > index 5117c43..30743f7 100644
    > --- a/kernel/trace/trace_events_stage_2.h
    > +++ b/kernel/trace/trace_events_stage_2.h
    > @@ -129,3 +129,48 @@ ftrace_format_##call(struct trace_seq *s) \
    > }
    >
    > #include <trace/trace_event_types.h>
    > +
    > +#undef __field
    > +#define __field(type, item) \
    > + ret = trace_define_field(event_call, #type, #item, \
    > + offsetof(typeof(field), item), \
    > + sizeof(field.item)); \
    > + if (ret) \
    > + return ret;
    > +
    > +#undef __array
    > +#define __array(type, item, len) \
    > + ret = trace_define_field(event_call, #type "[" #len "]", #item, \
    > + offsetof(typeof(field), item), \
    > + sizeof(field.item)); \
    > + if (ret) \
    > + return ret;
    > +
    > +#define __common_field(type, item) \
    > + ret = trace_define_field(event_call, #type, "common_" #item, \
    > + offsetof(typeof(field.ent), item), \
    > + sizeof(field.ent.item)); \
    > + if (ret) \
    > + return ret;
    > +
    > +#undef TRACE_EVENT
    > +#define TRACE_EVENT(call, proto, args, tstruct, func, print) \
    > +int \
    > +ftrace_define_fields_##call(void) \
    > +{ \
    > + struct ftrace_raw_##call field; \
    > + struct ftrace_event_call *event_call = &event_##call; \
    > + int ret; \
    > + \
    > + __common_field(unsigned char, type); \
    > + __common_field(unsigned char, flags); \
    > + __common_field(unsigned char, preempt_count); \
    > + __common_field(int, pid); \
    > + __common_field(int, tgid); \
    > + \
    > + tstruct; \
    > + \
    > + return ret; \
    > +}
    > +
    > +#include <trace/trace_event_types.h>
    > diff --git a/kernel/trace/trace_events_stage_3.h b/kernel/trace/trace_events_stage_3.h
    > index 6b3261c..468938f 100644
    > --- a/kernel/trace/trace_events_stage_3.h
    > +++ b/kernel/trace/trace_events_stage_3.h
    > @@ -252,6 +252,7 @@ static int ftrace_raw_init_event_##call(void) \
    > if (!id) \
    > return -ENODEV; \
    > event_##call.id = id; \
    > + INIT_LIST_HEAD(&event_##call.fields); \
    > return 0; \
    > } \
    > \
    > @@ -264,6 +265,7 @@ __attribute__((section("_ftrace_events"))) event_##call = { \
    > .regfunc = ftrace_raw_reg_event_##call, \
    > .unregfunc = ftrace_raw_unreg_event_##call, \
    > .show_format = ftrace_format_##call, \
    > + .define_fields = ftrace_define_fields_##call, \
    > _TRACE_PROFILE_INIT(call) \
    > }
    >

    Other than the small possible memory leak, it looks good.

    Acked-by: Frederic Weisbecker <fweisbec@gmail.com>



    \
     
     \ /
      Last update: 2009-03-22 18:41    [W:0.044 / U:61.084 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site