Message-ID: <20110629141056.GN4590@erda.amd.com>
Date:	Wed, 29 Jun 2011 16:10:56 +0200
From:	Robert Richter <robert.richter@....com>
To:	Peter Zijlstra <a.p.zijlstra@...llo.nl>
CC:	Francis Moreau <francis.moro@...il.com>,
	"linux-tip-commits@...r.kernel.org" 
	<linux-tip-commits@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"hpa@...or.com" <hpa@...or.com>,
	"mingo@...hat.com" <mingo@...hat.com>,
	"tglx@...utronix.de" <tglx@...utronix.de>,
	"mingo@...e.hu" <mingo@...e.hu>
Subject: Re: [tip:perf/core] perf: Ignore non-sampling overflows

On 29.06.11 06:50:55, Peter Zijlstra wrote:
> On Wed, 2011-06-29 at 12:37 +0200, Robert Richter wrote:
> 
> > I looked at the interrupt handlers. The events are always determined
> > from a per-cpu array:
> > 
> > 	cpuc = &__get_cpu_var(cpu_hw_events);
> > 	...
> > 	event = cpuc->events[idx];
> > 
> > In case of interrupts the event should then always be a hw event (or
> > uninitialized).  Even if the interrupt was triggered by a different
> > source, it would always be mapped to the same event and the check
> > is_sampling_event() would be meaningless.
> 
> I'm probably not quite getting what you mean, but how is
> is_sampling_event() meaningless? the INT bit is enabled for _all_
> events, whether they were configured as a sampling event or not.

Aren't all events that are mapped to counters via cpu_hw_events always
sampling events? Then, when calling perf_event_overflow() from an
interrupt handler there are no other events than sampling events.

> 
> Its just that for !sampling events we shouldn't attempt to generate any
> output.

If attr.sample_type is null, there is no output to generate. Better
use this instead of attr.sample_type in is_sampling_event()?
perf_event_overflow() could be used then to generate output also for
samples where no period is specified.

> 
> > There are other occurrences of perf_event_overflow() in
> > kernel/events/core.c for events of type PERF_TYPE_SOFTWARE. These
> > events are initialized with sample_period set and a check would always
> > be true too.
> 
> I'm failing to see what you mean, where do we always set
> event->attr.sample_period for software events?

Hmm, I read the code wrong and the check in perf_event_overflow()
might be needed for swevents.

> 
> > For both cases I stil don't see a reason for the check.
> 
> You're going to have to spell things out for me, I'm really not getting
> your argument.

I was thinking about to change this check and haven't seen cases for
that the check is for. What would happen if the check isn't there and
perf_event_overflow() is called from the interrupt handler?

> 
> > Anyway, would the following extentension of the check above ok?
> > 
> > 	if (unlikely(!is_sampling_event(event) && !event->attr.sample_type))
> > 		...
> > 
> > With no bits set in attr.sample_type the sample would be empty and
> > nothing to report. Now, with this change, samples that have data to
> > report wouldn't be dropped anymore.
> 
> Also, could you explain in what way data is dropped? Where do
> non-sampling events need to write sample data?

I stumbled over this while rebasing my perf ibs patches:

 http://git.kernel.org/?p=linux/kernel/git/rric/oprofile.git;a=shortlog;h=refs/heads/perf-ibs

Hope I could explain this to you better now.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists