Re: [ltt-dev] [LTTng][RFC][Patch 2/2] add irq-id parameter to irq_exit tracepoint and marker

[Mathieu Desnoyers <compudj at krystal dot dyndns dot org>]

* Masami Hiramatsu (mhiramat@redhat.com) wrote:
> Pierre-Marc Fournier wrote:
> > Masami Hiramatsu wrote:
> > 
> >> Index: linux-2.6-lttng/kernel/kernel-trace.c
> >> ===================================================================
> >> --- linux-2.6-lttng.orig/kernel/kernel-trace.c	2008-08-19 21:45:16.000000000 -0400
> >> +++ linux-2.6-lttng/kernel/kernel-trace.c	2008-08-21 15:58:39.000000000 -0400
> >> @@ -18,9 +18,10 @@
> >>  		(regs)?instruction_pointer(regs):0UL);
> >>  }
> >>
> >> -static void probe_irq_exit(irqreturn_t retval)
> >> +static void probe_irq_exit(unsigned int id, irqreturn_t retval)
> >>  {
> >> -	trace_mark(kernel_irq_exit, "handled #1u%u", IRQ_RETVAL(retval));
> >> +	trace_mark(kernel_irq_exit, "irq_id %u handled #1u%u",
> >> +		id, IRQ_RETVAL(retval));
> > 
> > This is redundant information. The IRQ id is already specified in
> > kernel_irq_entry events. The IRQ id of a kernel_irq_exit event can be
> > known by keeping a stack of nested IRQs, as is done in LTTV in the file
> > state.c.
> 
> I think it helps some corner case when we dropped irq entry event
> from memory(or pushed away from log). I know it is just a rare case,
> but it will happen.
> 

If you start losing events, having the irq id at irq_exit won't help
you, since you may be losing random irq entry/exit events and think you
are pairing correct events when in fact you pair two unrelated
entry/exit event. I therefore do not think we have to consider any
correctness issue about the information gathered when we have to drop
events. The trace viewer should be solid enough so it does not crash
though, and maybe detect inconsistency, but that's about it. I would not
add a performance cost for such a corner-case.

> And when we use these markers not from LTTng (ex. systemtap),
> it could be handled as isolated events. For example, I can check which
> irq handler returns error by tracing ONLY irq_exit events, with this patch.
> 

Hrm, this is precisely why I created the tracepoints. I would be all in
to add a struct pt_regs parameter and a irq id parameter to the irq exit
_tracepoint_ (since this is a kernel internal API), but I am very
reluctant to add it to the marker, given it will add useless information
in the traces.

I propose that systemtap move to tracepoints instead of markers, given
that they run in kernel-space anyway. It'a really a plus to have correct
typing of pointers, structures, etc.


> 
> > Here we need to compromise between the trace size and the amount of work
> > needed to analyze the trace. kernel_irq_exit is a very high rate event
> > and the work needed to keep track of the state is small. Therefore I
> > doubt including the redundant information is the best choice.
> 
> Indeed, could LTTng ignores(or filters) the parameter?
> 

LTTng just parses the format string and dumps them to userspace. Since I
developed the tracepoints, I see more and more the markers as being a
"binary formatting" infrastructure more closely tied to LTTng. But
tracepoints are taking over, so there is no features removed, just added
flexibility for in-kernel tracers.

Mathieu


> Thank you,
> 
> > 
> > pmf
> 
> -- 
> Masami Hiramatsu
> 
> Software Engineer
> Hitachi Computer Products (America) Inc.
> Software Solutions Division
> 
> e-mail: mhiramat@redhat.com
> 
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev@lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68