Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1363330 > unrolled thread

Re: [PATCH 2/5] ftrace perf: Move exclude_kernel tracepoint check to init event

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/5] ftrace perf: Move exclude_kernel tracepoint check to  init event Peter Zijlstra <peterz@infradead.org> - 2016-03-23 12:10 +0100
    Re: [PATCH 2/5] ftrace perf: Move exclude_kernel tracepoint check to  init event Jiri Olsa <jolsa@redhat.com> - 2016-03-24 11:00 +0100
      Re: [PATCH 2/5] ftrace perf: Move exclude_kernel tracepoint check to  init event Peter Zijlstra <peterz@infradead.org> - 2016-03-24 12:00 +0100
        Re: [PATCH 2/5] ftrace perf: Move exclude_kernel tracepoint check to  init event Jiri Olsa <jolsa@redhat.com> - 2016-03-24 13:30 +0100
          Re: [PATCH 2/5] ftrace perf: Move exclude_kernel tracepoint check to  init event Peter Zijlstra <peterz@infradead.org> - 2016-03-24 14:10 +0100
            Re: [PATCH 2/5] ftrace perf: Move exclude_kernel tracepoint check to  init event Jiri Olsa <jolsa@redhat.com> - 2016-03-24 14:40 +0100

#1363330 — Re: [PATCH 2/5] ftrace perf: Move exclude_kernel tracepoint check to init event

On Wed, Mar 16, 2016 at 03:34:30PM +0100, Jiri Olsa wrote:
> We suppress events with attr::exclude_kernel set when
> the event is generated, so following capture will
> give no warning but won't produce any data:
> 
>   $ sudo perf record -e sched:sched_switch:u ls
>   $ sudo /perf script | wc -l
>   0
> 
> Checking the attr::exclude_(kernel|user) at the event
> init time and failing right away for tracepoints from
> uprobes/kprobes and native ones:
> 
>   $ sudo perf record -e sched:sched_switch:u ls
>   Error:
>   The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (sched:sched_switch).
>   /bin/dmesg may provide additional information.
>   No CONFIG_PERF_EVENTS=y kernel support configured?
> 
>   $ sudo perf record -e probe:sys_read:u ls
>   Error:
>   The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (probe:sys_read).
>   /bin/dmesg may provide additional information.
>   No CONFIG_PERF_EVENTS=y kernel support configured?
> 
>   $ ./perf record -e probe_ex:main:k ./ex
>   Error:
>   The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (probe_ex:main).
>   /bin/dmesg may provide additional information.
>   No CONFIG_PERF_EVENTS=y kernel support configured?

Not sure about this one. The previous behaviour suggests
exclude_{user,kernel} is implemented, while the new behaviour says these
flags are not implemented, which is a functional regression.

That is, if all events are from kernel space, and we exclude all kernel
events, 0 is the right answer not an error.

Sure, with uprobes the situation is currently broken, but this isn't a
fix.

[toc] | [next] | [standalone]

#1364037

On Wed, Mar 23, 2016 at 11:41:29AM +0100, Peter Zijlstra wrote:
> On Wed, Mar 16, 2016 at 03:34:30PM +0100, Jiri Olsa wrote:
> > We suppress events with attr::exclude_kernel set when
> > the event is generated, so following capture will
> > give no warning but won't produce any data:
> > 
> >   $ sudo perf record -e sched:sched_switch:u ls
> >   $ sudo /perf script | wc -l
> >   0
> > 
> > Checking the attr::exclude_(kernel|user) at the event
> > init time and failing right away for tracepoints from
> > uprobes/kprobes and native ones:
> > 
> >   $ sudo perf record -e sched:sched_switch:u ls
> >   Error:
> >   The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (sched:sched_switch).
> >   /bin/dmesg may provide additional information.
> >   No CONFIG_PERF_EVENTS=y kernel support configured?
> > 
> >   $ sudo perf record -e probe:sys_read:u ls
> >   Error:
> >   The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (probe:sys_read).
> >   /bin/dmesg may provide additional information.
> >   No CONFIG_PERF_EVENTS=y kernel support configured?
> > 
> >   $ ./perf record -e probe_ex:main:k ./ex
> >   Error:
> >   The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (probe_ex:main).
> >   /bin/dmesg may provide additional information.
> >   No CONFIG_PERF_EVENTS=y kernel support configured?
> 
> Not sure about this one. The previous behaviour suggests
> exclude_{user,kernel} is implemented, while the new behaviour says these
> flags are not implemented, which is a functional regression.

well I would not expect 'sched:sched_switch:u' to work (be implemented)

and I thought it's better to trigger an error than silently 'produce' no data

> 
> That is, if all events are from kernel space, and we exclude all kernel
> events, 0 is the right answer not an error.
> 
> Sure, with uprobes the situation is currently broken, but this isn't a
> fix.

[toc] | [prev] | [next] | [standalone]

#1364065

On Thu, Mar 24, 2016 at 10:56:48AM +0100, Jiri Olsa wrote:
> On Wed, Mar 23, 2016 at 11:41:29AM +0100, Peter Zijlstra wrote:
> > On Wed, Mar 16, 2016 at 03:34:30PM +0100, Jiri Olsa wrote:
> > > We suppress events with attr::exclude_kernel set when
> > > the event is generated, so following capture will
> > > give no warning but won't produce any data:
> > > 
> > >   $ sudo perf record -e sched:sched_switch:u ls
> > >   $ sudo /perf script | wc -l
> > >   0
> > > 
> > > Checking the attr::exclude_(kernel|user) at the event
> > > init time and failing right away for tracepoints from
> > > uprobes/kprobes and native ones:
> > > 
> > >   $ sudo perf record -e sched:sched_switch:u ls
> > >   Error:
> > >   The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (sched:sched_switch).
> > >   /bin/dmesg may provide additional information.
> > >   No CONFIG_PERF_EVENTS=y kernel support configured?

> > Not sure about this one. The previous behaviour suggests
> > exclude_{user,kernel} is implemented, while the new behaviour says these
> > flags are not implemented, which is a functional regression.
> 
> well I would not expect 'sched:sched_switch:u' to work (be implemented)
> 
> and I thought it's better to trigger an error than silently 'produce' no data

We're not in the business of protecting people from themselves are we?
And if you want to help them, do so in userspace.

And its not like the [uk] flags are hard to implement here.

[toc] | [prev] | [next] | [standalone]

#1364113

On Thu, Mar 24, 2016 at 11:49:34AM +0100, Peter Zijlstra wrote:
> On Thu, Mar 24, 2016 at 10:56:48AM +0100, Jiri Olsa wrote:
> > On Wed, Mar 23, 2016 at 11:41:29AM +0100, Peter Zijlstra wrote:
> > > On Wed, Mar 16, 2016 at 03:34:30PM +0100, Jiri Olsa wrote:
> > > > We suppress events with attr::exclude_kernel set when
> > > > the event is generated, so following capture will
> > > > give no warning but won't produce any data:
> > > > 
> > > >   $ sudo perf record -e sched:sched_switch:u ls
> > > >   $ sudo /perf script | wc -l
> > > >   0
> > > > 
> > > > Checking the attr::exclude_(kernel|user) at the event
> > > > init time and failing right away for tracepoints from
> > > > uprobes/kprobes and native ones:
> > > > 
> > > >   $ sudo perf record -e sched:sched_switch:u ls
> > > >   Error:
> > > >   The sys_perf_event_open() syscall returned with 22 (Invalid argument) for event (sched:sched_switch).
> > > >   /bin/dmesg may provide additional information.
> > > >   No CONFIG_PERF_EVENTS=y kernel support configured?
> 
> > > Not sure about this one. The previous behaviour suggests
> > > exclude_{user,kernel} is implemented, while the new behaviour says these
> > > flags are not implemented, which is a functional regression.
> > 
> > well I would not expect 'sched:sched_switch:u' to work (be implemented)
> > 
> > and I thought it's better to trigger an error than silently 'produce' no data
> 
> We're not in the business of protecting people from themselves are we?
> And if you want to help them, do so in userspace.

yep, I planned to make user space patch
to make that error more user friendly

> 
> And its not like the [uk] flags are hard to implement here.

sched:sched_switch:u ?

jirka

[toc] | [prev] | [next] | [standalone]

#1364148

On Thu, Mar 24, 2016 at 01:25:44PM +0100, Jiri Olsa wrote:
> > > > >   $ sudo perf record -e sched:sched_switch:u ls
> > > > >   $ sudo /perf script | wc -l
> > > > >   0

> > And its not like the [uk] flags are hard to implement here.
> 
> sched:sched_switch:u ?

As per the above, its implemented and correct. The answer is 0.

The only problem is that currently it assumes all tracepoint events are
from the kernel, and that is wrong for uprobes.

But returning an error is not right, its a valid configuration, a daft
one, sure, but not invalid.

Similarly uprobe:foo:k is daft, but broken.

[toc] | [prev] | [next] | [standalone]

#1364169

On Thu, Mar 24, 2016 at 02:00:45PM +0100, Peter Zijlstra wrote:
> On Thu, Mar 24, 2016 at 01:25:44PM +0100, Jiri Olsa wrote:
> > > > > >   $ sudo perf record -e sched:sched_switch:u ls
> > > > > >   $ sudo /perf script | wc -l
> > > > > >   0
> 
> > > And its not like the [uk] flags are hard to implement here.
> > 
> > sched:sched_switch:u ?
> 
> As per the above, its implemented and correct. The answer is 0.
> 
> The only problem is that currently it assumes all tracepoint events are
> from the kernel, and that is wrong for uprobes.
> 
> But returning an error is not right, its a valid configuration, a daft
> one, sure, but not invalid.
> 
> Similarly uprobe:foo:k is daft, but broken.

ok, will send fix for uprobes then

thanks,
jirka 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web