public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Frederic Weisbecker <fweisbec@gmail.com>
To: Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	Stephane Eranian <eranian@google.com>
Subject: Re: [PATCH 2/4] perf: Add exclude_task perf event attribute
Date: Mon, 7 Jun 2010 03:38:51 +0200	[thread overview]
Message-ID: <20100607013848.GA11837@nowhere> (raw)
In-Reply-To: <1274770688.5882.168.camel@twins>

On Tue, May 25, 2010 at 08:58:08AM +0200, Peter Zijlstra wrote:
> On Tue, 2010-05-25 at 11:43 +1000, Paul Mackerras wrote:
> > On Fri, May 21, 2010 at 04:05:13PM +0200, Frederic Weisbecker wrote:
> > 
> > > Excluding is useful when you want to trace only hard and softirqs.
> > > 
> > > For this we use a new generic perf_exclude_event() (the previous
> > > one beeing turned into perf_exclude_swevent) to which you can pass
> > > the preemption offset to which your events trigger.
> > > 
> > > Computing preempt_count() - offset gives us the preempt_count() of
> > > the context that the event has interrupted, on top of which we
> > > can filter the non-irq contexts.
> > 
> > How does this work for hardware events when we are sampling and
> > getting an interrupt every N events?  It seems like the hardware is
> > still counting all events and interrupting every N events, but we are
> > only recording a sample if the interrupt occurred in the context we
> > want.  In other words the context of the Nth event is considered to be
> > the context for the N-1 events preceding that, which seems a pretty
> > poor approximation.
> > 
> > Also, for hardware events, if we are counting rather than sampling,
> > the exclude_task bit will have no effect.  So perhaps in that case the
> > perf_event_open should fail rather than appear to succeed but give
> > wrong data.
> 
> Right, so for hardware event we'd need to go with those irq_{enter,exit}
> hooks and either fully disable the call, or do as Ingo suggested, read
> the count delta and add that to period_left, so that we'll delay the
> sample (and subtract from ->count, which is I think the trickiest bit as
> it'll generate a non-monotonic ->count).
> 
> So I prefer the disable/enable from irq_enter/exit, however I also
> suspect that that is by far the most expensive option.


Playing with that, it's easy to contain the counting on the filtered
contexts: I can just flush (event->read()) when we enter/exit a context
but filter the update of event->count depending on exclude_* things.

There are several problems with that though:

- overflow interrupts continue, we can block them, but still...
- periods become randomly async as the interrupts happen. We
  could save the period_left on context enter to solve this


It would be certainly easier and clearer to use stop/start things on context
enter/exit.

And the only thing that seem to happen in these paths is a write
to the event config register.
Is it what is going to be too slow?
If you compare that to all the reads on the counter,
the interrupts that still need to be serviced and filtered with the
other solution, may be the stop/start solution is eventually better
in contrast.

How much time approximately does it take to write in this config register?

Thanks.


  parent reply	other threads:[~2010-06-07  1:38 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-21 14:05 [PATCH 0/4] perf: Tasks and irq exclusion Frederic Weisbecker
2010-05-21 14:05 ` [PATCH 1/4] irq: Support to compute context on top of a given preempt_count offset Frederic Weisbecker
2010-05-21 14:05 ` [PATCH 2/4] perf: Add exclude_task perf event attribute Frederic Weisbecker
2010-05-25  1:43   ` Paul Mackerras
2010-05-25  6:58     ` Peter Zijlstra
2010-05-25 10:06       ` Frederic Weisbecker
2010-06-07  1:38       ` Frederic Weisbecker [this message]
2010-06-08 18:59         ` Ingo Molnar
2010-06-08 19:02           ` Frederic Weisbecker
2010-05-21 14:05 ` [PATCH 3/4] perf: Support for irq exclusion Frederic Weisbecker
2010-05-21 14:06   ` Frederic Weisbecker
2010-05-21 14:05 ` [PATCH 4/4] perf: Support for task/softirq/hardirq exclusion on tools Frederic Weisbecker
2010-05-21 15:12 ` [PATCH 0/4] perf: Precise task / softirq / hardirq filtered stats/profiles Ingo Molnar
2010-05-21 16:15   ` Peter Zijlstra
2010-05-21 18:36     ` Ingo Molnar

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=20100607013848.GA11837@nowhere \
    --to=fweisbec@gmail.com \
    --cc=acme@redhat.com \
    --cc=eranian@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=paulus@samba.org \
    --cc=peterz@infradead.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