From mboxrd@z Thu Jan  1 00:00:00 1970
From: mathieu.desnoyers@polymtl.ca (Mathieu Desnoyers)
Date: Thu, 26 Mar 2009 14:28:59 -0400
Subject: [ltt-dev] [patch 2/9] LTTng instrumentation - irq
In-Reply-To: <alpine.DEB.2.00.0903242203480.22830@gandalf.stny.rr.com>
References: <20090324155625.420966314@polymtl.ca>
	<20090324160148.080628193@polymtl.ca>
	<20090324173354.GC3129@redhat.com> <20090324175049.GC31117@elte.hu>
	<20090324175750.GE3129@redhat.com> <20090324191252.GA11665@elte.hu>
	<20090324201146.GA4350@Krystal>
	<alpine.DEB.2.00.0903242203480.22830@gandalf.stny.rr.com>
Message-ID: <20090326182859.GB6399@Krystal>

* Steven Rostedt (rostedt at goodmis.org) wrote:
> 
> On Tue, 24 Mar 2009, Mathieu Desnoyers wrote:
> > 
> > This third type of tracepoint for IRQs you are talking about is actually
> > what I had in LTTng. I decided to add the irq_next_handler and to add a
> > action field to irq_entry to include the irq handler information needed
> > by Jason.
> > 
> > If we want to do this logically, without thinking about tracer
> > performance impact, we could/should do :
> > 
> > trace_irq_entry(irqno, pt_regs)
> >   for_each_handler() {
> >     trace_irq_handler_entry(action)
> >     action->handler()
> >     trace_irq_handler_exit(ret)
> >   }
> > trace_irq_exit(retval)
> > 
> > And add the irq_entry/irq_exit events to the arch-specific reschedule,
> > tlb flush, local timer irq, as I have in my lttng tree already.
> > 
> > But given the trace_irq_handler_entry/trace_irq_handler_exit events
> > could be combined, given we can record action and ret in the
> > irq_entry/exit events, I decided to remove 2 tracepoints (out of 4) from
> > the single-handler fast path by adding this information to the irq
> > entry/exit events, and decided to combine the irq_handler entry/exit
> > into a single next_handler event, which records the previous ret value
> > and the next action to execute.
> > 
> > On an interrupt-driven workload, it will have a significant impact.
> > (2 events vs 4).
> 
> I thought tracepoints while not active are very low overhead.
> 
> If you only want to use 2 of the 4, would that be just as fast?
> 

Probably, but I was talking about active tracing overhead here.

> > 
> > If we add interrupt threads to the kernel, then we can switch to the
> > following scheme :
> > 
> > * instrumentation of the real interrupt handler :
> > 
> > trace_irq_entry(irqno, pt_regs)
> > 
> > trace_irq_exit(ret)
> > 
> > * instrumentation of the irq threads :
> > 
> > trace_irq_thread_entry()
> > 
> > trace_irq_thread_exit()
> > 
> > I don't see why we should mind trying to make the tracepoints "logical",
> > especially if it hurts performance considerably. Doing these
> > implementation-specific versions of irq tracepoints would provide the
> > best performance we can get when tracing. It's up to the tracers to
> > specialize their analysis based on the underlying IRQ mechanism
> > (non-threaded vs threaded).
> 
> Perhaps we want to make them logical so that other things might hook into 
> these trace points besides a tracer. I do not agree that the code should 
> be modified just to make the trace points faster. The trace points are 
> just hooks into code, and should have no effect when disabled. Once the 
> code starts to change due to better placement of tracepoints for tracers, 
> that's when those trace points should be NACKed.
> 
> -- Steve
> 

If it makes the code messy, then yes, I agree that those tracepoints
should not go in.

Mathieu

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