public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Alexander Gordeev <agordeev@redhat.com>
Cc: linux-kernel@vger.kernel.org, x86@kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
	Jiri Olsa <jolsa@redhat.com>,
	Frederic Weisbecker <fweisbec@gmail.com>
Subject: Re: [PATCH RFC -tip 0/6] perf: IRQ-bound performance events
Date: Mon, 3 Jun 2013 14:22:23 +0200	[thread overview]
Message-ID: <20130603122223.GA1999@gmail.com> (raw)
In-Reply-To: <20130603094132.GB30878@dhcp-26-207.brq.redhat.com>


* Alexander Gordeev <agordeev@redhat.com> wrote:

> This patchset is against perf/core branch.
> 
> As Linux is able to measure task-bound and CPU-bound performance
> events there are no convenient means to monitor performance of
> an execution context which requires control and tuning probably
> most - interrupt service routines.
> 
> This series is an attempt to introduce IRQ-bound performance
> events - ones that only count in a context of a hardware interrupt
> handler.
> 
> The implementation is pretty straightforward: an IRQ-bound event
> is registered with the IRQ descriptor and gets enabled/disabled
> using new PMU callbacks: pmu_enable_irq() and pmu_disable_irq().
> 
> The series has not been tested thoroughly and is a concept proof
> rather than a decent implementation: no group events could be be
> loaded, inappropriate (i.e. software) events are not rejected,
> only Intel and AMD PMUs were tried for 'perf stat', only Intel
> PMU works with precise events. Perf tool changes are just a hack.
> 
> Yet, I would like first to ensure if the approach taken is not
> screwed and I did not miss anything vital. Not to mention if the
> change is wanted at all.
> 
> Below is a sample session on a machine with x2apic in cluster mode.
> IRQ number is passed using new argument -I <irq> (please nevermind
> '...process id '8'...' in the output for now):

Looks useful.

I think the main challenges are:

 - Creating a proper ABI for all this:

   - IRQ numbers alone are probably not specific enough: we'd also want to 
     be more specific to match on handler names - or handler numbers if
     the handler name is not unique.

   - another useful variant would be where IRQ numbers are too specific:
     something like 'perf top irq' would be a natural thing to do, to see 
     only overhead in hardirq execution - without limiting it to a
     specific handler. An 'all irq contexts' wildcard concept?

 - Covering softirqs as well. If we handle both hardirqs and softirqs,
   then we are pretty much feature complete: all major context types that 
   the Linux kernel cares about are covered in instrumentation. For things
   like networking the softirq overhead is obviously very important, and 
   for example on routers it will do most of the execution.

 - Covering threaded IRQs as well, in a similar model. So if someone types
   'perf top irq', and some IRQ handlers are running threaded, those
   should probaby be included as well.

 - Making the tooling friendlier: 'perf top irq' would be useful, and
   accepting handler names would be useful as well.

The runtime overhead of your patches seems to be pretty low: when no IRQ 
contexts are instrumented then it's a single 'is the list empty' check at 
context scheduling time. That looks acceptable.

Regarding the ABI and IRQ/softirq context enumeration you are breaking 
lots of new ground here, because unlike tasks, cgroups and CPUs the IRQ 
execution contexts do not have a good programmatically accessible 
namespace (yet). So it has to be thought out pretty well I think, but once 
we have it, it will be a lovely feature IMO.

Thanks,

	Ingo

  reply	other threads:[~2013-06-03 12:22 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-03  9:41 [PATCH RFC -tip 0/6] perf: IRQ-bound performance events Alexander Gordeev
2013-06-03 12:22 ` Ingo Molnar [this message]
2013-06-03 19:41   ` Frederic Weisbecker
2013-06-04  8:51     ` Jiri Olsa
2013-06-03 13:36 ` Alexander Gordeev
2013-06-04  9:38   ` Peter Zijlstra
2013-06-04 10:14     ` Alexander Gordeev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130603122223.GA1999@gmail.com \
    --to=mingo@kernel.org \
    --cc=acme@ghostprotocols.net \
    --cc=agordeev@redhat.com \
    --cc=fweisbec@gmail.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox