Re: [Patch 1/3] ksym_tracer: Eliminate trace concatenation and machine stall issues post removal

Frederic Weisbecker
2009-06-19T23:03:59Z

On Fri, Jun 19, 2009 at 10:54:53PM +0530, K.Prasad wrote:
> This patch fixes two issues reported with ksym tracer, seen after
> removal of a symbol from the tracer i) Concatenation of trace logs
> into a single line ii) Machine stall seen when logs are viewed
> using 'trace_pipe'.
>
> Known issue: Upon removal, 'cat trace_pipe' (without any preceding
> command and any further output in the trace buffer) responds to
> SIGTERM quickly but only after an indeterminate amount of delay
> for SIGINT.
>
> Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>
> ---
> kernel/trace/trace.h | 12 +++++++++---
> kernel/trace/trace_ksym.c | 11 ++++-------
> 2 files changed, 13 insertions(+), 10 deletions(-)
>
> Index: linux-2.6-tip.hbkpt/kernel/trace/trace_ksym.c
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/kernel/trace/trace_ksym.c
> +++ linux-2.6-tip.hbkpt/kernel/trace/trace_ksym.c
> @@ -71,7 +71,7 @@ void ksym_hbp_handler(struct hw_breakpoi
> {
> struct ring_buffer_event *event;
> struct trace_array *tr;
> - struct trace_ksym *entry;
> + struct trace_ksym_rb *entry;
> int pc;
>
> if (!ksym_tracing_enabled)
> @@ -87,7 +87,7 @@ void ksym_hbp_handler(struct hw_breakpoi
>
> entry = ring_buffer_event_data(event);
> strlcpy(entry->ksym_name, hbp->info.name, KSYM_SYMBOL_LEN);
> - entry->ksym_hbp = hbp;
> + memcpy(&(entry->ksym_hbp), hbp, sizeof(struct hw_breakpoint));

That looks wasteful. You only need the type from the arch_hw_breakpoint
and there you copy the whole generic breakpoint.

Frederic.

> entry->ip = instruction_pointer(regs);
> strlcpy(entry->p_name, current->comm, TASK_COMM_LEN);
> #ifdef CONFIG_PROFILE_KSYM_TRACER
> @@ -380,7 +380,7 @@ static enum print_line_t ksym_trace_outp
> {
> struct trace_entry *entry = iter->ent;
> struct trace_seq *s = &iter->seq;
> - struct trace_ksym *field;
> + struct trace_ksym_rb *field;
> char str[KSYM_SYMBOL_LEN];
> int ret;
>
> @@ -394,17 +394,14 @@ static enum print_line_t ksym_trace_outp
> if (!ret)
> return TRACE_TYPE_PARTIAL_LINE;
>
> - switch (field->ksym_hbp->info.type) {
> + switch (field->ksym_hbp.info.type) {
> case HW_BREAKPOINT_WRITE:
> ret = trace_seq_printf(s, " W ");
> break;
> case HW_BREAKPOINT_RW:
> ret = trace_seq_printf(s, " RW ");
> break;
> - default:
> - return TRACE_TYPE_PARTIAL_LINE;
> }
> -
> if (!ret)
> return TRACE_TYPE_PARTIAL_LINE;
>
> Index: linux-2.6-tip.hbkpt/kernel/trace/trace.h
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/kernel/trace/trace.h
> +++ linux-2.6-tip.hbkpt/kernel/trace/trace.h
> @@ -216,15 +216,21 @@ struct syscall_trace_exit {
> extern int process_new_ksym_entry(char *ksymname, int op, unsigned long addr);
>
> struct trace_ksym {
> - 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;
> char ksym_name[KSYM_NAME_LEN];
> +};
> +
> +/* Ring buffer's copy of the breakpoint data */
> +struct trace_ksym_rb {
> + struct trace_entry ent;
> + struct hw_breakpoint ksym_hbp;
> + unsigned long ip;
> + char ksym_name[KSYM_NAME_LEN];
> char p_name[TASK_COMM_LEN];
> };
>
> @@ -343,7 +349,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 trace_ksym_rb, TRACE_KSYM); \
> __ftrace_bad_type(); \
> } while (0)
>
>