LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [RFC] perf probe: -x option position issue
@ 2015-03-30 17:46 Jiri Olsa
  2015-03-30 19:48 ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 12+ messages in thread
From: Jiri Olsa @ 2015-03-30 17:46 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Ingo Molnar, Namhyung Kim, Arnaldo Carvalho de Melo,
	Peter Zijlstra, David Ahern, linux-kernel, Martin Cermak

hi,
Martin found out following issue.. having following ex binary:

---
int main(void)
{
        return 0;
}
---

following will create uprobe on main:

  [root@dell-per510-01 perf]# gcc -g -o ex ex.c
  [root@dell-per510-01 perf]# ./perf probe -x ./ex -a main
  Added new event:
    probe_ex:main        (on main in /root/linux/tools/perf/ex)

  You can now use it in all perf tools, such as:

          perf record -e probe_ex:main -aR sleep 1

  [root@dell-per510-01 perf]# cat /sys/kernel/debug/tracing/uprobe_events 
  p:probe_ex/main /root/linux/tools/perf/ex:0x00000000000004f6


while following will create (?) kprobe with complain in dmesg:

  [root@dell-per510-01 perf]# gcc -g -o ex ex.c
  [root@dell-per510-01 perf]# ./perf probe -a main -x ./ex
  Added new event:
    probe:main           (on main in ex)

  You can now use it in all perf tools, such as:

          perf record -e probe:main -aR sleep 1

  [root@dell-per510-01 perf]# dmesg | tail -2
  [16986.182159] Could not insert probe at ex:main+0: -2
  [16986.187030] This probe might be able to register aftertarget module is loaded. Continue.


that does not seem as an expected behaviour, or am I missing something?

thanks,
jirka

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] perf probe: -x option position issue
  2015-03-30 17:46 [RFC] perf probe: -x option position issue Jiri Olsa
@ 2015-03-30 19:48 ` Arnaldo Carvalho de Melo
  2015-03-31  8:04   ` Masami Hiramatsu
  0 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-03-30 19:48 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Masami Hiramatsu, Ingo Molnar, Namhyung Kim, Peter Zijlstra,
	David Ahern, linux-kernel, Martin Cermak

Em Mon, Mar 30, 2015 at 07:46:55PM +0200, Jiri Olsa escreveu:
> hi,
> Martin found out following issue.. having following ex binary:
> 
> ---
> int main(void)
> {
>         return 0;
> }
> ---
> 
> following will create uprobe on main:
> 
>   [root@dell-per510-01 perf]# gcc -g -o ex ex.c
>   [root@dell-per510-01 perf]# ./perf probe -x ./ex -a main
>   Added new event:
>     probe_ex:main        (on main in /root/linux/tools/perf/ex)
> 
>   You can now use it in all perf tools, such as:
> 
>           perf record -e probe_ex:main -aR sleep 1
> 
>   [root@dell-per510-01 perf]# cat /sys/kernel/debug/tracing/uprobe_events 
>   p:probe_ex/main /root/linux/tools/perf/ex:0x00000000000004f6
> 
> 
> while following will create (?) kprobe with complain in dmesg:

Right, it looks like it will create the probe on the currently selected
DSO, which, if you have none, will be the kernel, thus the kprobe
(probe:main), while if you do a '-x ./ex -a main' you are selecting the
'ex' DSO and then asking for the probe to be added to a function named
'main', on that DSO, that 'perf probe' realizes is a userspace binary,
thus creates a uprobe: probe_%DSONAME:%FUNCTIONAME.

I wonder if I can do:

[root@ssdandy acme]# perf probe -a icmp_rcv -x ./ex -a main
Probe point 'icmp_rcv' not found.
  Error: Failed to add events.


No, I can't, I'd say we should support that, i.e. inserting multiple
probes per command line, for different DSOs, etc. I.e. the above would
be equivalent to these two calls:

[root@ssdandy acme]# perf probe -a icmp_rcv
Added new event:
  probe:icmp_rcv       (on icmp_rcv)

You can now use it in all perf tools, such as:

	perf record -e probe:icmp_rcv -aR sleep 1

[root@ssdandy acme]# perf probe -x ./ex -a main
Added new event:
  probe_ex:main        (on main in /home/acme/ex)

You can now use it in all perf tools, such as:

	perf record -e probe_ex:main -aR sleep 1

[root@ssdandy acme]#


But it isn't like that, so, yes, what you report is a bug, both for your
expectation (that I think is that it should put a uprobes with both your
examples) and for mine (that it would add the first to the kernel, and
the second to the selected DSO via -x).

- Arnaldo
 
>   [root@dell-per510-01 perf]# gcc -g -o ex ex.c
>   [root@dell-per510-01 perf]# ./perf probe -a main -x ./ex
>   Added new event:
>     probe:main           (on main in ex)
> 
>   You can now use it in all perf tools, such as:
> 
>           perf record -e probe:main -aR sleep 1
> 
>   [root@dell-per510-01 perf]# dmesg | tail -2
>   [16986.182159] Could not insert probe at ex:main+0: -2
>   [16986.187030] This probe might be able to register aftertarget module is loaded. Continue.
> 
> 
> that does not seem as an expected behaviour, or am I missing something?
> 
> thanks,
> jirka

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] perf probe: -x option position issue
  2015-03-30 19:48 ` Arnaldo Carvalho de Melo
@ 2015-03-31  8:04   ` Masami Hiramatsu
  2015-03-31 13:33     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 12+ messages in thread
From: Masami Hiramatsu @ 2015-03-31  8:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Ingo Molnar, Namhyung Kim, Peter Zijlstra,
	David Ahern, linux-kernel, Martin Cermak

(2015/03/31 4:48), Arnaldo Carvalho de Melo wrote:
> Em Mon, Mar 30, 2015 at 07:46:55PM +0200, Jiri Olsa escreveu:
>> hi,
>> Martin found out following issue.. having following ex binary:
>>
>> ---
>> int main(void)
>> {
>>         return 0;
>> }
>> ---
>>
>> following will create uprobe on main:
>>
>>   [root@dell-per510-01 perf]# gcc -g -o ex ex.c
>>   [root@dell-per510-01 perf]# ./perf probe -x ./ex -a main
>>   Added new event:
>>     probe_ex:main        (on main in /root/linux/tools/perf/ex)
>>
>>   You can now use it in all perf tools, such as:
>>
>>           perf record -e probe_ex:main -aR sleep 1
>>
>>   [root@dell-per510-01 perf]# cat /sys/kernel/debug/tracing/uprobe_events 
>>   p:probe_ex/main /root/linux/tools/perf/ex:0x00000000000004f6
>>
>>
>> while following will create (?) kprobe with complain in dmesg:
> 
> Right, it looks like it will create the probe on the currently selected
> DSO, which, if you have none, will be the kernel, thus the kprobe
> (probe:main), while if you do a '-x ./ex -a main' you are selecting the
> 'ex' DSO and then asking for the probe to be added to a function named
> 'main', on that DSO, that 'perf probe' realizes is a userspace binary,
> thus creates a uprobe: probe_%DSONAME:%FUNCTIONAME.

Right, but this looks strange and not easy to expect.

> 
> I wonder if I can do:
> 
> [root@ssdandy acme]# perf probe -a icmp_rcv -x ./ex -a main
> Probe point 'icmp_rcv' not found.
>   Error: Failed to add events.

Ah, this should be fixed. Even with multiple -x, it fails.

# ./perf probe -x /usr/lib64/libc-2.17.so -a malloc -x ./perf -a main

 usage: perf probe [<options>] 'PROBEDEF' ['PROBEDEF' ...]
    or: perf probe [<options>] --add 'PROBEDEF' [--add 'PROBEDEF' ...]
    or: perf probe [<options>] --del '[GROUP:]EVENT' ...
    or: perf probe --list
    or: perf probe [<options>] --line 'LINEDESC'
    or: perf probe [<options>] --vars 'PROBEPOINT'

    -x, --exec <executable|path>
                          target executable name or path

...

I'd like to start with setting up the event only on single binary,
and showing appropriate error message.

> No, I can't, I'd say we should support that, i.e. inserting multiple
> probes per command line, for different DSOs, etc. I.e. the above would
> be equivalent to these two calls:
> 
> [root@ssdandy acme]# perf probe -a icmp_rcv
> Added new event:
>   probe:icmp_rcv       (on icmp_rcv)
> 
> You can now use it in all perf tools, such as:
> 
> 	perf record -e probe:icmp_rcv -aR sleep 1
> 
> [root@ssdandy acme]# perf probe -x ./ex -a main
> Added new event:
>   probe_ex:main        (on main in /home/acme/ex)
> 
> You can now use it in all perf tools, such as:
> 
> 	perf record -e probe_ex:main -aR sleep 1
> 
> [root@ssdandy acme]#

OK, finally we should support that.

> But it isn't like that, so, yes, what you report is a bug, both for your
> expectation (that I think is that it should put a uprobes with both your
> examples) and for mine (that it would add the first to the kernel, and
> the second to the selected DSO via -x).

Yes, both are bugs. I'll fix that.

BTW, let me check that the below behaviors are OK for you.

perf probe -x BIN -a XXX
	 -> setup XXX on BIN
perf probe -a XXX -x BIN
	 -> setup XXX on BIN
perf probe -a XXX -x BIN -a YYY
	 -> setup XXX on kernel and YYY on BIN
perf probe -x BIN -a XXX -x BIN2 -a YYY
	 -> setup XXX on BIN and YYY on BIN2

Thank you,

> 
> - Arnaldo
>  
>>   [root@dell-per510-01 perf]# gcc -g -o ex ex.c
>>   [root@dell-per510-01 perf]# ./perf probe -a main -x ./ex
>>   Added new event:
>>     probe:main           (on main in ex)
>>
>>   You can now use it in all perf tools, such as:
>>
>>           perf record -e probe:main -aR sleep 1
>>
>>   [root@dell-per510-01 perf]# dmesg | tail -2
>>   [16986.182159] Could not insert probe at ex:main+0: -2
>>   [16986.187030] This probe might be able to register aftertarget module is loaded. Continue.
>>
>>
>> that does not seem as an expected behaviour, or am I missing something?
>>
>> thanks,
>> jirka
> 


-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] perf probe: -x option position issue
  2015-03-31  8:04   ` Masami Hiramatsu
@ 2015-03-31 13:33     ` Arnaldo Carvalho de Melo
  2015-04-01  8:37       ` Masami Hiramatsu
                         ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-03-31 13:33 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Jiri Olsa, Ingo Molnar, Namhyung Kim, Peter Zijlstra,
	David Ahern, linux-kernel, Martin Cermak

Em Tue, Mar 31, 2015 at 05:04:18PM +0900, Masami Hiramatsu escreveu:
> (2015/03/31 4:48), Arnaldo Carvalho de Melo wrote:
> > No, I can't, I'd say we should support that, i.e. inserting multiple
> > probes per command line, for different DSOs, etc. I.e. the above would
> > be equivalent to these two calls:

> > [root@ssdandy acme]# perf probe -a icmp_rcv
> > Added new event:
> >   probe:icmp_rcv       (on icmp_rcv)

> > You can now use it in all perf tools, such as:

> > 	perf record -e probe:icmp_rcv -aR sleep 1

> > [root@ssdandy acme]# perf probe -x ./ex -a main
> > Added new event:
> >   probe_ex:main        (on main in /home/acme/ex)

> > You can now use it in all perf tools, such as:

> > 	perf record -e probe_ex:main -aR sleep 1

> > [root@ssdandy acme]#

> OK, finally we should support that.

> > But it isn't like that, so, yes, what you report is a bug, both for your
> > expectation (that I think is that it should put a uprobes with both your
> > examples) and for mine (that it would add the first to the kernel, and
> > the second to the selected DSO via -x).

> Yes, both are bugs. I'll fix that.

> BTW, let me check that the below behaviors are OK for you.

> perf probe -x BIN -a XXX
> 	 -> setup XXX on BIN

Ok

> perf probe -a XXX -x BIN
> 	 -> setup XXX on BIN
> perf probe -a XXX -x BIN -a YYY
> 	 -> setup XXX on kernel and YYY on BIN

The two above are inconsistent, I think, first one, for me, doesn't make
sense, i.e. it says: Add XXX to the selected DSO, which, as none was
specified at that point, should be the kernel, right?

I.e. if we do:

> perf probe -a XXX

Without that extra -x that is coming _after_ the command to add a probe
to XXX (-a XXX), what is that the tool should do (does from day 1, when
'probe' was first introduced in tools/perf/):

Add a probe to XXX _in the kernel_, i.e. not specifying a DSO means: its
for the kernel.

So, for me:

> perf probe -a XXX -x BIN
> 	 -> setup XXX on BIN

Is invalid (or inocuous if what one wants is to add a probe for XXX on
the BIN dso), because it doesn't make sense _if you want to support
adding multiple probes for different DSOs on the same command line_,
because it would mean:

  Add a probe to XXX _in the kernel_, then select BIN as the DSO for
which probes will be then specified, but in this example, none are
specified after that "-x BIN", so, I think that:

 perf probe -a XXX -x BIN

and:

 perf probe -a XXX

Mean the same thing, i.e. add a probe for XXX in the kernel.

> perf probe -x BIN -a XXX -x BIN2 -a YYY
> 	 -> setup XXX on BIN and YYY on BIN2
 
Ok.

Also, more generically, I think that:

perf probe -a AAA -a BBB -a CCC -a DDD -x /lib64/libc-2.17.so -a malloc -a free \
           -x /usr/lib64/libthread_db-1.0.so -a td_lookup -a td_thr_event_enable

Should add kprobes for AAA, BBB, CCC and DDD in the kernel, uprobes for
malloc and free on libc and uprobes for td_lookup and
td_thr_event_enable on libthread_db.

Making it even more compact would be a bonus:

perf probe -a AAA,BBB,CCC,DDD -x /lib64/libc-2.17.so -a malloc,free \
           -x /usr/lib64/libthread_db-1.0.so -a td_lookup,td_thr_event_enable

:-)

- Arnaldo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Re: [RFC] perf probe: -x option position issue
  2015-03-31 13:33     ` Arnaldo Carvalho de Melo
@ 2015-04-01  8:37       ` Masami Hiramatsu
  2015-04-01 10:25       ` [PATCH perf/core 1/2] perf-probe: Support multiple probes on different binaries Masami Hiramatsu
  2015-04-01 10:25       ` [PATCH perf/core 2/2] perf-probe: Check the orphaned -x option Masami Hiramatsu
  2 siblings, 0 replies; 12+ messages in thread
From: Masami Hiramatsu @ 2015-04-01  8:37 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Ingo Molnar, Namhyung Kim, Peter Zijlstra,
	David Ahern, linux-kernel, Martin Cermak

(2015/03/31 22:33), Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 31, 2015 at 05:04:18PM +0900, Masami Hiramatsu escreveu:
>> (2015/03/31 4:48), Arnaldo Carvalho de Melo wrote:
>>> No, I can't, I'd say we should support that, i.e. inserting multiple
>>> probes per command line, for different DSOs, etc. I.e. the above would
>>> be equivalent to these two calls:
> 
>>> [root@ssdandy acme]# perf probe -a icmp_rcv
>>> Added new event:
>>>   probe:icmp_rcv       (on icmp_rcv)
> 
>>> You can now use it in all perf tools, such as:
> 
>>> 	perf record -e probe:icmp_rcv -aR sleep 1
> 
>>> [root@ssdandy acme]# perf probe -x ./ex -a main
>>> Added new event:
>>>   probe_ex:main        (on main in /home/acme/ex)
> 
>>> You can now use it in all perf tools, such as:
> 
>>> 	perf record -e probe_ex:main -aR sleep 1
> 
>>> [root@ssdandy acme]#
> 
>> OK, finally we should support that.
> 
>>> But it isn't like that, so, yes, what you report is a bug, both for your
>>> expectation (that I think is that it should put a uprobes with both your
>>> examples) and for mine (that it would add the first to the kernel, and
>>> the second to the selected DSO via -x).
> 
>> Yes, both are bugs. I'll fix that.
> 
>> BTW, let me check that the below behaviors are OK for you.
> 
>> perf probe -x BIN -a XXX
>> 	 -> setup XXX on BIN
> 
> Ok
> 
>> perf probe -a XXX -x BIN
>> 	 -> setup XXX on BIN
>> perf probe -a XXX -x BIN -a YYY
>> 	 -> setup XXX on kernel and YYY on BIN
> 
> The two above are inconsistent, I think, first one, for me, doesn't make
> sense, i.e. it says: Add XXX to the selected DSO, which, as none was
> specified at that point, should be the kernel, right?

OK.

> 
> I.e. if we do:
> 
>> perf probe -a XXX
> 
> Without that extra -x that is coming _after_ the command to add a probe
> to XXX (-a XXX), what is that the tool should do (does from day 1, when
> 'probe' was first introduced in tools/perf/):
> 
> Add a probe to XXX _in the kernel_, i.e. not specifying a DSO means: its
> for the kernel.
> 
> So, for me:
> 
>> perf probe -a XXX -x BIN
>> 	 -> setup XXX on BIN
> 
> Is invalid (or inocuous if what one wants is to add a probe for XXX on
> the BIN dso), because it doesn't make sense _if you want to support
> adding multiple probes for different DSOs on the same command line_,
> because it would mean:
> 
>   Add a probe to XXX _in the kernel_, then select BIN as the DSO for
> which probes will be then specified, but in this example, none are
> specified after that "-x BIN", so, I think that:
> 
>  perf probe -a XXX -x BIN
> 
> and:
> 
>  perf probe -a XXX
> 
> Mean the same thing, i.e. add a probe for XXX in the kernel.

OK, so if we have -x BIN after -a, it should have an error, since
that may be not what the user intend.

>> perf probe -x BIN -a XXX -x BIN2 -a YYY
>> 	 -> setup XXX on BIN and YYY on BIN2
>  
> Ok.
> 
> Also, more generically, I think that:
> 
> perf probe -a AAA -a BBB -a CCC -a DDD -x /lib64/libc-2.17.so -a malloc -a free \
>            -x /usr/lib64/libthread_db-1.0.so -a td_lookup -a td_thr_event_enable
> 
> Should add kprobes for AAA, BBB, CCC and DDD in the kernel, uprobes for
> malloc and free on libc and uprobes for td_lookup and
> td_thr_event_enable on libthread_db.

Yes, that is what I'll do on perf probe. All the probes after -x XXX are
defined on XXX binary (or module).

> 
> Making it even more compact would be a bonus:
> 
> perf probe -a AAA,BBB,CCC,DDD -x /lib64/libc-2.17.so -a malloc,free \
>            -x /usr/lib64/libthread_db-1.0.so -a td_lookup,td_thr_event_enable
> 
> :-)

Sorry, this does not fit to current syntax of probe definition, since
we may have some arguments on each event...

Thank you,

-- 
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@hitachi.com


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH perf/core 1/2] perf-probe: Support multiple probes on different binaries
  2015-03-31 13:33     ` Arnaldo Carvalho de Melo
  2015-04-01  8:37       ` Masami Hiramatsu
@ 2015-04-01 10:25       ` Masami Hiramatsu
  2015-04-11  6:37         ` [tip:perf/core] perf probe: " tip-bot for Masami Hiramatsu
  2015-04-01 10:25       ` [PATCH perf/core 2/2] perf-probe: Check the orphaned -x option Masami Hiramatsu
  2 siblings, 1 reply; 12+ messages in thread
From: Masami Hiramatsu @ 2015-04-01 10:25 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra,
	Linux Kernel Mailing List, David Ahern, namhyung, Jiri Olsa,
	Ingo Molnar

Support multiple probes on different binaries with just
one command.

In the result, this example sets up the probes on icmp_rcv in
kernel, on main and set_target in perf, and on pcspkr_event
in pcspker.ko driver.
  -----
  # perf probe -a icmp_rcv -x ./perf -a main -a set_target \
   -m /lib/modules/4.0.0-rc5+/kernel/drivers/input/misc/pcspkr.ko \
   -a pcspkr_event
  Added new event:
    probe:icmp_rcv       (on icmp_rcv)

  You can now use it in all perf tools, such as:

          perf record -e probe:icmp_rcv -aR sleep 1

  Added new event:
    probe_perf:main      (on main in /home/mhiramat/ksrc/linux-3/tools/perf/perf)

  You can now use it in all perf tools, such as:

          perf record -e probe_perf:main -aR sleep 1

  Added new event:
    probe_perf:set_target (on set_target in /home/mhiramat/ksrc/linux-3/tools/perf/perf)

  You can now use it in all perf tools, such as:

          perf record -e probe_perf:set_target -aR sleep 1

  Added new event:
    probe:pcspkr_event   (on pcspkr_event in pcspkr)

  You can now use it in all perf tools, such as:

          perf record -e probe:pcspkr_event -aR sleep 1
  -----

Reported-by: Arnaldo Carvalho de Melo <acme@infradead.org>
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/builtin-probe.c    |    9 +++++++--
 tools/perf/util/probe-event.c |    5 +++--
 tools/perf/util/probe-event.h |    6 +++---
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 921bb69..2df23e1 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -78,6 +78,11 @@ static int parse_probe_event(const char *str)
 	}
 
 	pev->uprobes = params.uprobes;
+	if (params.target) {
+		pev->target = strdup(params.target);
+		if (!pev->target)
+			return -ENOMEM;
+	}
 
 	/* Parse a perf-probe command into event */
 	ret = parse_perf_probe_command(str, pev);
@@ -178,7 +183,7 @@ static int opt_set_target(const struct option *opt, const char *str,
 	int ret = -ENOENT;
 	char *tmp;
 
-	if  (str && !params.target) {
+	if  (str) {
 		if (!strcmp(opt->long_name, "exec"))
 			params.uprobes = true;
 #ifdef HAVE_DWARF_SUPPORT
@@ -200,6 +205,7 @@ static int opt_set_target(const struct option *opt, const char *str,
 			if (!tmp)
 				return -ENOMEM;
 		}
+		free(params.target);
 		params.target = tmp;
 		ret = 0;
 	}
@@ -487,7 +493,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (params.nevents) {
 		ret = add_perf_probe_events(params.events, params.nevents,
 					    params.max_probe_points,
-					    params.target,
 					    params.force_add);
 		if (ret < 0) {
 			pr_err_with_code("  Error: Failed to add events.", ret);
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 8feac07..9b02db8 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1903,6 +1903,7 @@ void clear_perf_probe_event(struct perf_probe_event *pev)
 
 	free(pev->event);
 	free(pev->group);
+	free(pev->target);
 	clear_perf_probe_point(&pev->point);
 
 	for (i = 0; i < pev->nargs; i++) {
@@ -2651,7 +2652,7 @@ struct __event_package {
 };
 
 int add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
-			  int max_tevs, const char *target, bool force_add)
+			  int max_tevs, bool force_add)
 {
 	int i, j, ret;
 	struct __event_package *pkgs;
@@ -2675,7 +2676,7 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
 		ret  = convert_to_probe_trace_events(pkgs[i].pev,
 						     &pkgs[i].tevs,
 						     max_tevs,
-						     target);
+						     pkgs[i].pev->target);
 		if (ret < 0)
 			goto end;
 		pkgs[i].ntevs = ret;
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index e01e994..d6b7834 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -73,7 +73,8 @@ struct perf_probe_event {
 	char			*group;	/* Group name */
 	struct perf_probe_point	point;	/* Probe point */
 	int			nargs;	/* Number of arguments */
-	bool			uprobes;
+	bool			uprobes;	/* Uprobe event flag */
+	char			*target;	/* Target binary */
 	struct perf_probe_arg	*args;	/* Arguments */
 };
 
@@ -124,8 +125,7 @@ extern int line_range__init(struct line_range *lr);
 extern const char *kernel_get_module_path(const char *module);
 
 extern int add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
-				 int max_probe_points, const char *module,
-				 bool force_add);
+				 int max_probe_points, bool force_add);
 extern int del_perf_probe_events(struct strlist *dellist);
 extern int show_perf_probe_events(void);
 extern int show_line_range(struct line_range *lr, const char *module,


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH perf/core  2/2] perf-probe: Check the orphaned -x option
  2015-03-31 13:33     ` Arnaldo Carvalho de Melo
  2015-04-01  8:37       ` Masami Hiramatsu
  2015-04-01 10:25       ` [PATCH perf/core 1/2] perf-probe: Support multiple probes on different binaries Masami Hiramatsu
@ 2015-04-01 10:25       ` Masami Hiramatsu
  2015-04-01 11:11         ` Jiri Olsa
                           ` (2 more replies)
  2 siblings, 3 replies; 12+ messages in thread
From: Masami Hiramatsu @ 2015-04-01 10:25 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Linux Kernel Mailing List, David Ahern, namhyung,
	Jiri Olsa, Ingo Molnar

To avoid probing in unintended binary, the orphaned -x option
must be checked and warned.

Without this patch, following command sets up the probe in
the kernel.
  -----
  # perf probe -a strcpy -x ./perf
  Added new event:
    probe:strcpy         (on strcpy)

  You can now use it in all perf tools, such as:

          perf record -e probe:strcpy -aR sleep 1
  -----

But in this case, it seems that the user may want to probe
in the perf binary. With this patch, perf-probe correctly
handles the orphaned -x.
  -----
  # perf probe -a strcpy -x ./perf
    Error: -x/-m must follow the probe definitions.
  ...
  -----

Reported-by: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/builtin-probe.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 2df23e1..f7b1af6 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -56,6 +56,7 @@ static struct {
 	bool mod_events;
 	bool uprobes;
 	bool quiet;
+	bool target_used;
 	int nevents;
 	struct perf_probe_event events[MAX_PROBES];
 	struct strlist *dellist;
@@ -82,6 +83,7 @@ static int parse_probe_event(const char *str)
 		pev->target = strdup(params.target);
 		if (!pev->target)
 			return -ENOMEM;
+		params.target_used = true;
 	}
 
 	/* Parse a perf-probe command into event */
@@ -107,6 +109,7 @@ static int set_target(const char *ptr)
 		params.target = strdup(ptr);
 		if (!params.target)
 			return -ENOMEM;
+		params.target_used = false;
 
 		found = 1;
 		buf = ptr + (strlen(ptr) - 3);
@@ -207,6 +210,7 @@ static int opt_set_target(const struct option *opt, const char *str,
 		}
 		free(params.target);
 		params.target = tmp;
+		params.target_used = false;
 		ret = 0;
 	}
 
@@ -491,6 +495,12 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 	}
 
 	if (params.nevents) {
+		/* Ensure the last given target is used */
+		if (params.target && !params.target_used) {
+			pr_warning("  Error: -x/-m must follow the probe definitions.\n");
+			usage_with_options(probe_usage, options);
+		}
+
 		ret = add_perf_probe_events(params.events, params.nevents,
 					    params.max_probe_points,
 					    params.force_add);


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH perf/core  2/2] perf-probe: Check the orphaned -x option
  2015-04-01 10:25       ` [PATCH perf/core 2/2] perf-probe: Check the orphaned -x option Masami Hiramatsu
@ 2015-04-01 11:11         ` Jiri Olsa
  2015-04-10  6:51         ` Masami Hiramatsu
  2015-04-11  6:38         ` [tip:perf/core] perf probe: " tip-bot for Masami Hiramatsu
  2 siblings, 0 replies; 12+ messages in thread
From: Jiri Olsa @ 2015-04-01 11:11 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra,
	Linux Kernel Mailing List, David Ahern, namhyung, Ingo Molnar

On Wed, Apr 01, 2015 at 07:25:42PM +0900, Masami Hiramatsu wrote:
> To avoid probing in unintended binary, the orphaned -x option
> must be checked and warned.
> 
> Without this patch, following command sets up the probe in
> the kernel.
>   -----
>   # perf probe -a strcpy -x ./perf
>   Added new event:
>     probe:strcpy         (on strcpy)
> 
>   You can now use it in all perf tools, such as:
> 
>           perf record -e probe:strcpy -aR sleep 1
>   -----
> 
> But in this case, it seems that the user may want to probe
> in the perf binary. With this patch, perf-probe correctly
> handles the orphaned -x.
>   -----
>   # perf probe -a strcpy -x ./perf
>     Error: -x/-m must follow the probe definitions.
>   ...
>   -----
> 
> Reported-by: Jiri Olsa <jolsa@redhat.com>

works nicely, thanks a lot!

Tested-by: Jiri Olsa <jolsa@kernel.org>

jirka

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH perf/core  2/2] perf-probe: Check the orphaned -x option
  2015-04-01 10:25       ` [PATCH perf/core 2/2] perf-probe: Check the orphaned -x option Masami Hiramatsu
  2015-04-01 11:11         ` Jiri Olsa
@ 2015-04-10  6:51         ` Masami Hiramatsu
  2015-04-10 13:22           ` Arnaldo Carvalho de Melo
  2015-04-11  6:38         ` [tip:perf/core] perf probe: " tip-bot for Masami Hiramatsu
  2 siblings, 1 reply; 12+ messages in thread
From: Masami Hiramatsu @ 2015-04-10  6:51 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Linux Kernel Mailing List, David Ahern, namhyung,
	Jiri Olsa, Ingo Molnar

Ping?

(2015/04/01 19:25), Masami Hiramatsu wrote:
> To avoid probing in unintended binary, the orphaned -x option
> must be checked and warned.
> 
> Without this patch, following command sets up the probe in
> the kernel.
>   -----
>   # perf probe -a strcpy -x ./perf
>   Added new event:
>     probe:strcpy         (on strcpy)
> 
>   You can now use it in all perf tools, such as:
> 
>           perf record -e probe:strcpy -aR sleep 1
>   -----
> 
> But in this case, it seems that the user may want to probe
> in the perf binary. With this patch, perf-probe correctly
> handles the orphaned -x.
>   -----
>   # perf probe -a strcpy -x ./perf
>     Error: -x/-m must follow the probe definitions.
>   ...
>   -----
> 
> Reported-by: Jiri Olsa <jolsa@redhat.com>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> ---
>  tools/perf/builtin-probe.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index 2df23e1..f7b1af6 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -56,6 +56,7 @@ static struct {
>  	bool mod_events;
>  	bool uprobes;
>  	bool quiet;
> +	bool target_used;
>  	int nevents;
>  	struct perf_probe_event events[MAX_PROBES];
>  	struct strlist *dellist;
> @@ -82,6 +83,7 @@ static int parse_probe_event(const char *str)
>  		pev->target = strdup(params.target);
>  		if (!pev->target)
>  			return -ENOMEM;
> +		params.target_used = true;
>  	}
>  
>  	/* Parse a perf-probe command into event */
> @@ -107,6 +109,7 @@ static int set_target(const char *ptr)
>  		params.target = strdup(ptr);
>  		if (!params.target)
>  			return -ENOMEM;
> +		params.target_used = false;
>  
>  		found = 1;
>  		buf = ptr + (strlen(ptr) - 3);
> @@ -207,6 +210,7 @@ static int opt_set_target(const struct option *opt, const char *str,
>  		}
>  		free(params.target);
>  		params.target = tmp;
> +		params.target_used = false;
>  		ret = 0;
>  	}
>  
> @@ -491,6 +495,12 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>  	}
>  
>  	if (params.nevents) {
> +		/* Ensure the last given target is used */
> +		if (params.target && !params.target_used) {
> +			pr_warning("  Error: -x/-m must follow the probe definitions.\n");
> +			usage_with_options(probe_usage, options);
> +		}
> +
>  		ret = add_perf_probe_events(params.events, params.nevents,
>  					    params.max_probe_points,
>  					    params.force_add);
> 
> 


-- 
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@hitachi.com



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH perf/core  2/2] perf-probe: Check the orphaned -x option
  2015-04-10  6:51         ` Masami Hiramatsu
@ 2015-04-10 13:22           ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-04-10 13:22 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Peter Zijlstra, Linux Kernel Mailing List, David Ahern, namhyung,
	Jiri Olsa, Ingo Molnar

Em Fri, Apr 10, 2015 at 03:51:57PM +0900, Masami Hiramatsu escreveu:
> Ping?

Sorry for the delay, thought I had processed those already, done now,
will be in my next pull req.

Thanks for working on it, I tested both and all seems fine,

- Arnaldo
 
> (2015/04/01 19:25), Masami Hiramatsu wrote:
> > To avoid probing in unintended binary, the orphaned -x option
> > must be checked and warned.
> > 
> > Without this patch, following command sets up the probe in
> > the kernel.
> >   -----
> >   # perf probe -a strcpy -x ./perf
> >   Added new event:
> >     probe:strcpy         (on strcpy)
> > 
> >   You can now use it in all perf tools, such as:
> > 
> >           perf record -e probe:strcpy -aR sleep 1
> >   -----
> > 
> > But in this case, it seems that the user may want to probe
> > in the perf binary. With this patch, perf-probe correctly
> > handles the orphaned -x.
> >   -----
> >   # perf probe -a strcpy -x ./perf
> >     Error: -x/-m must follow the probe definitions.
> >   ...
> >   -----
> > 
> > Reported-by: Jiri Olsa <jolsa@redhat.com>
> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> > ---
> >  tools/perf/builtin-probe.c |   10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> > index 2df23e1..f7b1af6 100644
> > --- a/tools/perf/builtin-probe.c
> > +++ b/tools/perf/builtin-probe.c
> > @@ -56,6 +56,7 @@ static struct {
> >  	bool mod_events;
> >  	bool uprobes;
> >  	bool quiet;
> > +	bool target_used;
> >  	int nevents;
> >  	struct perf_probe_event events[MAX_PROBES];
> >  	struct strlist *dellist;
> > @@ -82,6 +83,7 @@ static int parse_probe_event(const char *str)
> >  		pev->target = strdup(params.target);
> >  		if (!pev->target)
> >  			return -ENOMEM;
> > +		params.target_used = true;
> >  	}
> >  
> >  	/* Parse a perf-probe command into event */
> > @@ -107,6 +109,7 @@ static int set_target(const char *ptr)
> >  		params.target = strdup(ptr);
> >  		if (!params.target)
> >  			return -ENOMEM;
> > +		params.target_used = false;
> >  
> >  		found = 1;
> >  		buf = ptr + (strlen(ptr) - 3);
> > @@ -207,6 +210,7 @@ static int opt_set_target(const struct option *opt, const char *str,
> >  		}
> >  		free(params.target);
> >  		params.target = tmp;
> > +		params.target_used = false;
> >  		ret = 0;
> >  	}
> >  
> > @@ -491,6 +495,12 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> >  	}
> >  
> >  	if (params.nevents) {
> > +		/* Ensure the last given target is used */
> > +		if (params.target && !params.target_used) {
> > +			pr_warning("  Error: -x/-m must follow the probe definitions.\n");
> > +			usage_with_options(probe_usage, options);
> > +		}
> > +
> >  		ret = add_perf_probe_events(params.events, params.nevents,
> >  					    params.max_probe_points,
> >  					    params.force_add);
> > 
> > 
> 
> 
> -- 
> Masami HIRAMATSU
> Linux Technology Research Center, System Productivity Research Dept.
> Center for Technology Innovation - Systems Engineering
> Hitachi, Ltd., Research & Development Group
> E-mail: masami.hiramatsu.pt@hitachi.com
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [tip:perf/core] perf probe: Support multiple probes on different binaries
  2015-04-01 10:25       ` [PATCH perf/core 1/2] perf-probe: Support multiple probes on different binaries Masami Hiramatsu
@ 2015-04-11  6:37         ` tip-bot for Masami Hiramatsu
  0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Masami Hiramatsu @ 2015-04-11  6:37 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: masami.hiramatsu.pt, tglx, peterz, mingo, acme, namhyung,
	dsahern, linux-kernel, hpa, jolsa, acme

Commit-ID:  7afb3fab390871b1d20b1dbb94e03b8a3861cb0d
Gitweb:     http://git.kernel.org/tip/7afb3fab390871b1d20b1dbb94e03b8a3861cb0d
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Wed, 1 Apr 2015 19:25:39 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 10 Apr 2015 10:19:53 -0300

perf probe: Support multiple probes on different binaries

Support multiple probes on different binaries with just
one command.

In the result, this example sets up the probes on icmp_rcv in
kernel, on main and set_target in perf, and on pcspkr_event
in pcspker.ko driver.
  -----
  # perf probe -a icmp_rcv -x ./perf -a main -a set_target \
   -m /lib/modules/4.0.0-rc5+/kernel/drivers/input/misc/pcspkr.ko \
   -a pcspkr_event
  Added new event:
    probe:icmp_rcv       (on icmp_rcv)

  You can now use it in all perf tools, such as:

          perf record -e probe:icmp_rcv -aR sleep 1

  Added new event:
    probe_perf:main      (on main in /home/mhiramat/ksrc/linux-3/tools/perf/perf)

  You can now use it in all perf tools, such as:

          perf record -e probe_perf:main -aR sleep 1

  Added new event:
    probe_perf:set_target (on set_target in /home/mhiramat/ksrc/linux-3/tools/perf/perf)

  You can now use it in all perf tools, such as:

          perf record -e probe_perf:set_target -aR sleep 1

  Added new event:
    probe:pcspkr_event   (on pcspkr_event in pcspkr)

  You can now use it in all perf tools, such as:

          perf record -e probe:pcspkr_event -aR sleep 1
  -----

Reported-by: Arnaldo Carvalho de Melo <acme@infradead.org>
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20150401102539.17137.46454.stgit@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-probe.c    | 9 +++++++--
 tools/perf/util/probe-event.c | 5 +++--
 tools/perf/util/probe-event.h | 6 +++---
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 921bb69..2df23e1 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -78,6 +78,11 @@ static int parse_probe_event(const char *str)
 	}
 
 	pev->uprobes = params.uprobes;
+	if (params.target) {
+		pev->target = strdup(params.target);
+		if (!pev->target)
+			return -ENOMEM;
+	}
 
 	/* Parse a perf-probe command into event */
 	ret = parse_perf_probe_command(str, pev);
@@ -178,7 +183,7 @@ static int opt_set_target(const struct option *opt, const char *str,
 	int ret = -ENOENT;
 	char *tmp;
 
-	if  (str && !params.target) {
+	if  (str) {
 		if (!strcmp(opt->long_name, "exec"))
 			params.uprobes = true;
 #ifdef HAVE_DWARF_SUPPORT
@@ -200,6 +205,7 @@ static int opt_set_target(const struct option *opt, const char *str,
 			if (!tmp)
 				return -ENOMEM;
 		}
+		free(params.target);
 		params.target = tmp;
 		ret = 0;
 	}
@@ -487,7 +493,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (params.nevents) {
 		ret = add_perf_probe_events(params.events, params.nevents,
 					    params.max_probe_points,
-					    params.target,
 					    params.force_add);
 		if (ret < 0) {
 			pr_err_with_code("  Error: Failed to add events.", ret);
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index b788517..30545ce 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1906,6 +1906,7 @@ void clear_perf_probe_event(struct perf_probe_event *pev)
 
 	free(pev->event);
 	free(pev->group);
+	free(pev->target);
 	clear_perf_probe_point(&pev->point);
 
 	for (i = 0; i < pev->nargs; i++) {
@@ -2654,7 +2655,7 @@ struct __event_package {
 };
 
 int add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
-			  int max_tevs, const char *target, bool force_add)
+			  int max_tevs, bool force_add)
 {
 	int i, j, ret;
 	struct __event_package *pkgs;
@@ -2678,7 +2679,7 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
 		ret  = convert_to_probe_trace_events(pkgs[i].pev,
 						     &pkgs[i].tevs,
 						     max_tevs,
-						     target);
+						     pkgs[i].pev->target);
 		if (ret < 0)
 			goto end;
 		pkgs[i].ntevs = ret;
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index e01e994..d6b7834 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -73,7 +73,8 @@ struct perf_probe_event {
 	char			*group;	/* Group name */
 	struct perf_probe_point	point;	/* Probe point */
 	int			nargs;	/* Number of arguments */
-	bool			uprobes;
+	bool			uprobes;	/* Uprobe event flag */
+	char			*target;	/* Target binary */
 	struct perf_probe_arg	*args;	/* Arguments */
 };
 
@@ -124,8 +125,7 @@ extern int line_range__init(struct line_range *lr);
 extern const char *kernel_get_module_path(const char *module);
 
 extern int add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
-				 int max_probe_points, const char *module,
-				 bool force_add);
+				 int max_probe_points, bool force_add);
 extern int del_perf_probe_events(struct strlist *dellist);
 extern int show_perf_probe_events(void);
 extern int show_line_range(struct line_range *lr, const char *module,

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [tip:perf/core] perf probe: Check the orphaned -x option
  2015-04-01 10:25       ` [PATCH perf/core 2/2] perf-probe: Check the orphaned -x option Masami Hiramatsu
  2015-04-01 11:11         ` Jiri Olsa
  2015-04-10  6:51         ` Masami Hiramatsu
@ 2015-04-11  6:38         ` tip-bot for Masami Hiramatsu
  2 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Masami Hiramatsu @ 2015-04-11  6:38 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: jolsa, peterz, acme, hpa, linux-kernel, tglx, dsahern, mingo,
	namhyung, masami.hiramatsu.pt

Commit-ID:  8cb0aa4c2db395b143cd5165586dc17677960002
Gitweb:     http://git.kernel.org/tip/8cb0aa4c2db395b143cd5165586dc17677960002
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Wed, 1 Apr 2015 19:25:42 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 10 Apr 2015 10:21:30 -0300

perf probe: Check the orphaned -x option

To avoid probing in unintended binary, the orphaned -x option must be
checked and warned.

Without this patch, following command sets up the probe in the kernel.

  -----
  # perf probe -a strcpy -x ./perf
  Added new event:
    probe:strcpy         (on strcpy)

  You can now use it in all perf tools, such as:

          perf record -e probe:strcpy -aR sleep 1
  -----

But in this case, it seems that the user may want to probe in the perf
binary. With this patch, perf-probe correctly handles the orphaned -x.

  -----
  # perf probe -a strcpy -x ./perf
    Error: -x/-m must follow the probe definitions.
  ...
  -----

Reported-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20150401102541.17137.75477.stgit@localhost.localdomain
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-probe.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 2df23e1..f7b1af6 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -56,6 +56,7 @@ static struct {
 	bool mod_events;
 	bool uprobes;
 	bool quiet;
+	bool target_used;
 	int nevents;
 	struct perf_probe_event events[MAX_PROBES];
 	struct strlist *dellist;
@@ -82,6 +83,7 @@ static int parse_probe_event(const char *str)
 		pev->target = strdup(params.target);
 		if (!pev->target)
 			return -ENOMEM;
+		params.target_used = true;
 	}
 
 	/* Parse a perf-probe command into event */
@@ -107,6 +109,7 @@ static int set_target(const char *ptr)
 		params.target = strdup(ptr);
 		if (!params.target)
 			return -ENOMEM;
+		params.target_used = false;
 
 		found = 1;
 		buf = ptr + (strlen(ptr) - 3);
@@ -207,6 +210,7 @@ static int opt_set_target(const struct option *opt, const char *str,
 		}
 		free(params.target);
 		params.target = tmp;
+		params.target_used = false;
 		ret = 0;
 	}
 
@@ -491,6 +495,12 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 	}
 
 	if (params.nevents) {
+		/* Ensure the last given target is used */
+		if (params.target && !params.target_used) {
+			pr_warning("  Error: -x/-m must follow the probe definitions.\n");
+			usage_with_options(probe_usage, options);
+		}
+
 		ret = add_perf_probe_events(params.events, params.nevents,
 					    params.max_probe_points,
 					    params.force_add);

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2015-04-11  6:39 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-30 17:46 [RFC] perf probe: -x option position issue Jiri Olsa
2015-03-30 19:48 ` Arnaldo Carvalho de Melo
2015-03-31  8:04   ` Masami Hiramatsu
2015-03-31 13:33     ` Arnaldo Carvalho de Melo
2015-04-01  8:37       ` Masami Hiramatsu
2015-04-01 10:25       ` [PATCH perf/core 1/2] perf-probe: Support multiple probes on different binaries Masami Hiramatsu
2015-04-11  6:37         ` [tip:perf/core] perf probe: " tip-bot for Masami Hiramatsu
2015-04-01 10:25       ` [PATCH perf/core 2/2] perf-probe: Check the orphaned -x option Masami Hiramatsu
2015-04-01 11:11         ` Jiri Olsa
2015-04-10  6:51         ` Masami Hiramatsu
2015-04-10 13:22           ` Arnaldo Carvalho de Melo
2015-04-11  6:38         ` [tip:perf/core] perf probe: " tip-bot for Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).