| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1309358386.6701.1307.camel@twins> Date: Wed, 29 Jun 2011 16:39:46 +0200 From: Peter Zijlstra <a.p.zijlstra@...llo.nl> To: Robert Richter <robert.richter@....com> 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 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@...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