lkml.org 
[lkml]   [2009]   [Jul]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/8] ksym_tracer: Extract trace entry from struct trace_ksym
    On Tue, Jul 07, 2009 at 01:52:36PM +0800, Li Zefan wrote:
    > struct trace_ksym is used as an entry in hbp list, and is also
    > used as trace_entry stored in ring buffer.
    >
    > This is not necessary and is a waste of memory in ring buffer.
    >
    > There is also a bug that dereferencing field->ksym_hbp in
    > ksym_trace_output() can be invalid.
    >
    > Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
    > ---
    > kernel/trace/trace.h | 13 ++++---------
    > kernel/trace/trace_ksym.c | 26 ++++++++++++++++++--------
    > 2 files changed, 22 insertions(+), 17 deletions(-)
    >
    > diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
    > index 61b4e94..6868902 100644
    > --- a/kernel/trace/trace.h
    > +++ b/kernel/trace/trace.h
    > @@ -215,17 +215,12 @@ struct syscall_trace_exit {
    > #define KSYM_SELFTEST_ENTRY "ksym_selftest_dummy"
    > extern int process_new_ksym_entry(char *ksymname, int op, unsigned long addr);
    >
    > -struct trace_ksym {
    > +struct ksym_trace_entry {
    > struct trace_entry ent;
    > - struct hw_breakpoint *ksym_hbp;
    > - unsigned long ksym_addr;
    > unsigned long ip;
    > -#ifdef CONFIG_PROFILE_KSYM_TRACER
    > - unsigned long counter;
    > -#endif
    > - struct hlist_node ksym_hlist;
    > + unsigned char type;
    > char ksym_name[KSYM_NAME_LEN];
    > - char p_name[TASK_COMM_LEN];
    > + char cmd[TASK_COMM_LEN];
    > };
    >
    > /*
    > @@ -343,7 +338,7 @@ extern void __ftrace_bad_type(void);
    > TRACE_SYSCALL_ENTER); \
    > IF_ASSIGN(var, ent, struct syscall_trace_exit, \
    > TRACE_SYSCALL_EXIT); \
    > - IF_ASSIGN(var, ent, struct trace_ksym, TRACE_KSYM); \
    > + IF_ASSIGN(var, ent, struct ksym_trace_entry, TRACE_KSYM);\
    > __ftrace_bad_type(); \
    > } while (0)
    >
    > diff --git a/kernel/trace/trace_ksym.c b/kernel/trace/trace_ksym.c
    > index eef97e7..085ff05 100644
    > --- a/kernel/trace/trace_ksym.c
    > +++ b/kernel/trace/trace_ksym.c
    > @@ -37,6 +37,15 @@
    > #define KSYM_TRACER_OP_LEN 3 /* rw- */
    > #define KSYM_FILTER_ENTRY_LEN (KSYM_NAME_LEN + KSYM_TRACER_OP_LEN + 1)
    >
    > +struct trace_ksym {
    > + struct hw_breakpoint *ksym_hbp;
    > + unsigned long ksym_addr;
    > +#ifdef CONFIG_PROFILE_KSYM_TRACER
    > + unsigned long counter;
    > +#endif
    > + struct hlist_node ksym_hlist;
    > +};
    > +
    > static struct trace_array *ksym_trace_array;
    >
    > static unsigned int ksym_filter_entry_count;
    > @@ -71,7 +80,7 @@ void ksym_hbp_handler(struct hw_breakpoint *hbp, struct pt_regs *regs)
    > {
    > struct ring_buffer_event *event;
    > struct trace_array *tr;
    > - struct trace_ksym *entry;
    > + struct ksym_trace_entry *entry;
    > int pc;
    >
    > if (!ksym_tracing_enabled)
    > @@ -85,11 +94,12 @@ void ksym_hbp_handler(struct hw_breakpoint *hbp, struct pt_regs *regs)
    > if (!event)
    > return;
    >
    > - entry = ring_buffer_event_data(event);
    > + entry = ring_buffer_event_data(event);
    > + entry->ip = instruction_pointer(regs);
    > + entry->type = hbp->info.type;
    > strlcpy(entry->ksym_name, hbp->info.name, KSYM_SYMBOL_LEN);
    > - entry->ksym_hbp = hbp;
    > - entry->ip = instruction_pointer(regs);
    > - strlcpy(entry->p_name, current->comm, TASK_COMM_LEN);
    > + strlcpy(entry->cmd, current->comm, TASK_COMM_LEN);


    Ah, I did not realized that the cmdline was that roughly saved.
    It would be more ring-buffer-friendly to use tracing_record_cmdline.



    > +
    > #ifdef CONFIG_PROFILE_KSYM_TRACER
    > ksym_collect_stats(hbp->info.address);
    > #endif /* CONFIG_PROFILE_KSYM_TRACER */
    > @@ -380,7 +390,7 @@ static enum print_line_t ksym_trace_output(struct trace_iterator *iter)
    > {
    > struct trace_entry *entry = iter->ent;
    > struct trace_seq *s = &iter->seq;
    > - struct trace_ksym *field;
    > + struct ksym_trace_entry *field;
    > char str[KSYM_SYMBOL_LEN];
    > int ret;
    >
    > @@ -389,12 +399,12 @@ static enum print_line_t ksym_trace_output(struct trace_iterator *iter)
    >
    > trace_assign_type(field, entry);
    >
    > - ret = trace_seq_printf(s, "%-15s %-5d %-3d %-20s ", field->p_name,
    > + ret = trace_seq_printf(s, "%-15s %-5d %-3d %-20s ", field->cmd,
    > entry->pid, iter->cpu, field->ksym_name);
    > if (!ret)
    > return TRACE_TYPE_PARTIAL_LINE;
    >
    > - switch (field->ksym_hbp->info.type) {
    > + switch (field->type) {
    > case HW_BREAKPOINT_WRITE:
    > ret = trace_seq_printf(s, " W ");
    > break;


    Thanks!

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

    Prasad, any objection?



    \
     
     \ /
      Last update: 2009-07-07 23:29    [W:0.040 / U:63.020 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site