Re: [tip:perf/core] perf: Ignore non-sampling overflows

[Peter Zijlstra]

On Wed, 2011-06-29 at 16:10 +0200, Robert Richter wrote:

> > 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? 

No. perf stat which only counts (has period==0) uses hardware counters
just fine but doesn't sample anything, yet has the INT bit set (as
explained a few emails back).

> Then, when calling perf_event_overflow() from an
> interrupt handler there are no other events than sampling events.

Thus false. (Also, even if we didn't always set the INT bit, it might
see the overflow of a non-sampling event while dealing with the PMI
triggered by another event).

> > 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.

Arguably true, currently we would still write a rudimentary sample,
consisting of just the header with a 0-sized payload. This is, I'd
rather we do that than add yet another conditional on the sample fast
path. If the user doesn't want samples he should've set period==0, if
the does he had better set a non-zero sample_type. 

>  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.

But what for? period==0 is defined as: does not generate samples.

> > 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?

It might generate spurious samples, nothing too bad, just unexpected.

> > > 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.

Could you point where exactly in the IBS code this happens? Even for
IBS, if period==0 it should not generate samples. Arguably IBS with
period==0 is pretty pointless, but that's another story.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/