Re: [PATCH] perf/x86/intel: Drop kernel samples even though :u is specified

2017-05-22 Thread Jin, Yao
On 5/22/2017 5:26 PM, Peter Zijlstra wrote: On Mon, May 22, 2017 at 09:45:30AM +0100, Mark Rutland wrote: On Mon, May 22, 2017 at 10:12:22AM +0800, Jin, Yao wrote: But the code is being ugly and hard to maintain. And frankly I don't know kernel address space for all arch. Any idea? Could we

Re: [PATCH] perf/x86/intel: Drop kernel samples even though :u is specified

2017-05-22 Thread Peter Zijlstra
On Mon, May 22, 2017 at 09:45:30AM +0100, Mark Rutland wrote: > On Mon, May 22, 2017 at 10:12:22AM +0800, Jin, Yao wrote: > > But the code is being ugly and hard to maintain. And frankly I don't > > know kernel address space for all arch. > > > > Any idea? Could we just do at x86 side this time?

Re: [PATCH] perf/x86/intel: Drop kernel samples even though :u is specified

2017-05-22 Thread Mark Rutland
On Mon, May 22, 2017 at 10:12:22AM +0800, Jin, Yao wrote: > Hi Peter, > > Maybe it's not very easy to move the skid checking to generic code > because we don't have a common kernel_ip() available to determine if > ip is a kernel address. > > I was trying to move kernel_ip() from arch/x86/events/p

Re: [PATCH] perf/x86/intel: Drop kernel samples even though :u is specified

2017-05-21 Thread Jin, Yao
On 5/19/2017 9:33 PM, Jin, Yao wrote: On 5/19/2017 8:36 PM, Peter Zijlstra wrote: On Fri, May 19, 2017 at 08:24:19PM +0800, Jin, Yao wrote: Ah, I was more thinking of something like PERF_PMU_CAP_NO_SKID or something that would skip the test and preserve current behaviour. OK, I understand n

Re: [PATCH] perf/x86/intel: Drop kernel samples even though :u is specified

2017-05-19 Thread Jin, Yao
On 5/19/2017 8:36 PM, Peter Zijlstra wrote: On Fri, May 19, 2017 at 08:24:19PM +0800, Jin, Yao wrote: Ah, I was more thinking of something like PERF_PMU_CAP_NO_SKID or something that would skip the test and preserve current behaviour. OK, I understand now. For example, for PEBS event, its cap

Re: [PATCH] perf/x86/intel: Drop kernel samples even though :u is specified

2017-05-19 Thread Peter Zijlstra
On Fri, May 19, 2017 at 08:24:19PM +0800, Jin, Yao wrote: > > Ah, I was more thinking of something like PERF_PMU_CAP_NO_SKID or > > something that would skip the test and preserve current behaviour. > > OK, I understand now. For example, for PEBS event, its capabilities should > be set with PERF_P

Re: [PATCH] perf/x86/intel: Drop kernel samples even though :u is specified

2017-05-19 Thread Jin, Yao
On 5/19/2017 8:10 PM, Peter Zijlstra wrote: On Fri, May 19, 2017 at 08:06:09PM +0800, Jin, Yao wrote: SNIP I would much rather see this in generic code, somewhere around __perf_event_overflow() I suppose. That would retain proper accounting for the interrupt rate etc.. Also it would work fo

Re: [PATCH] perf/x86/intel: Drop kernel samples even though :u is specified

2017-05-19 Thread Peter Zijlstra
On Fri, May 19, 2017 at 08:06:09PM +0800, Jin, Yao wrote: > SNIP > > > I would much rather see this in generic code, somewhere around > > __perf_event_overflow() I suppose. That would retain proper accounting > > for the interrupt rate etc.. > > > > Also it would work for all architectures. Becau

Re: [PATCH] perf/x86/intel: Drop kernel samples even though :u is specified

2017-05-19 Thread Jin, Yao
SNIP I would much rather see this in generic code, somewhere around __perf_event_overflow() I suppose. That would retain proper accounting for the interrupt rate etc.. Also it would work for all architectures. Because I'm thinking more than just x86 will suffer from skid. Yes, moving to generic

Re: [PATCH] perf/x86/intel: Drop kernel samples even though :u is specified

2017-05-19 Thread Will Deacon
On Fri, May 19, 2017 at 11:29:05AM +0200, Peter Zijlstra wrote: > On Fri, May 19, 2017 at 06:19:12PM +0800, Jin Yao wrote: > > +bool skid_kernel_samples(struct perf_event *event, struct pt_regs *regs) > > +{ > > + u64 ip; > > + > > + /* > > +* Without PEBS, we may get kernel samples even th

Re: [PATCH] perf/x86/intel: Drop kernel samples even though :u is specified

2017-05-19 Thread Peter Zijlstra
On Fri, May 19, 2017 at 06:19:12PM +0800, Jin Yao wrote: > When doing sampling without PEBS > > perf record -e cycles:u ... > > On workloads that do a lot of kernel entry/exits we see kernel > samples, even though :u is specified. This is due to skid existing. > > This is a security issue becaus