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

[K.Prasad]

On Sat, Jun 20, 2009 at 01:03:49AM +0200, Frederic Weisbecker wrote:
> 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@xxxxxxxxxxxxxxxxxx>
> > ---
> >  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.
>

Well, the entire copy operation for ksym_name and p_name is indeed a
much bigger waste (KSYM_SYMBOL_LEN and TASK_COMM_LEN bytes respetively)
and the breakpoint structure size is tiny in comparison. 

The type information is an arch-specific field and it encapsulated
within 'struct hw_breakpoint'. In any case this solution needs
improvement more in terms of getting the tracer infrastructure to
identify stale entries (belonging to removed symbols) and prevent their
display. The existing trace logs contain huge redundant information due
to multiple copies of the same data and it would yield much better results
than such trivial optimisations.  The ksym_tracer, I believe, has just
unearthed the opportunity for the same as it is (is it?) the first such
ftrace plugin to partially remove entries.

Thanks,
K.Prasad

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/