Re: [PATCH v2] trace: Add x86 irq vector entry/exit tracepoints

[Frederic Weisbecker <fweisbec@gmail.com>]

Sorry guys I only found your answers today by browsing my email client.
I guess I posponed my reading but completely forgot :-(

On Wed, Jun 01, 2011 at 04:30:18PM -0700, David Sharp wrote:
> On Wed, Jun 1, 2011 at 3:38 PM, Vaibhav Nagarnaik <vnagarnaik@google.com> wrote:
> > On Tue, May 31, 2011 at 5:00 PM, Frederic Weisbecker
> > <fweisbec@gmail.com> wrote:
> >>
> >> IIRC, Thomas suggested in the v1 to turn arch tracepoints into core ones when they
> >> stand for generic (ie: shared by all archs) event meaning.
> >>
> >> I don't know where the discussion went after that, but that still seems to
> >> me an important direction. Tracing irqs by their vector number should be reserved
> >> for pure arch meanings. Sharing common tracepoints for generic events amongst archs makes life
> >> easier for tools. Bonus when the tracepoint can be placed in the generic code.
> >>
> >
> > I agree with you that the tracepoints should be as generic as possible
> > to have them traced across platforms, but I see these tracepoints as
> > specific to x86 platform. Since most of these vectors are x86 specific.
> > The only ones that you pointed out as generic were:
> 
> I think what Vaibhav means is that this is "the one trace event for
> all hard IRQs on the platform". A tracepoint for the cross-platform
> irq handlers is also useful, but that is not the purpose of this
> tracepoint.

If a generic tracepoint can replace one per arch, we all want that instead.
This is 24 times (24 archs) less maintainance burden.

So if that doesn't meet your needs, you really need to explain us in detail why.

I believe it's fine to deduce from a generic timer tracepoint that the arch fired
a specific interrupt to achieve that.

You might need some low level details, but then tell us why.

> There's not actually much overlap between platforms for
> IRQs, so each platform needs to do the work of covering all their
> IRQs, and won't see much benefit from the generic tracepoints.

There is at least 5 overlaps. And everybody will benefit from the generic tracepoints.

> On Tue, May 31, 2011 at 5:00 PM, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > Tracing irqs by their vector number should be reserved for pure arch meanings.
> 
> exactly, and this tracepoint is for that "arch meaning".

Ok but there is no point in having an arch tracepoint where the arch code is only
doing:

arch_foo_irq()
{
	generic_foo_irq();
}

It may do some specific low level things, ack the apic, etc... But that's not what
is interesting, right? What is interesting is the fact you have an interrupt.

> I'd also add that having these tracepoints here makes it more obvious
> that we will get all IRQ events, which is what is important to us.

Look at those arch level handlers, it's doesn't seem you can miss anything there.

It can happen in the case of the local apic timer, but then you get a warning
of spurious LAPIC timer interrupt. They are corner cases, better check these
rather than tracing every layers to ensure you always get everything.