Messages in this thread | | | Date | Wed, 04 Sep 2013 17:01:56 +0900 | From | Masami Hiramatsu <> | Subject | Re: [PATCH 1/2] SDT markers listing by perf |
| |
(2013/09/04 15:42), Namhyung Kim wrote: > On Tue, 03 Sep 2013 13:06:55 +0530, Hemant Kumar wrote: > > [SNIP] > >> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c >> index e8a66f9..3d8dcdf 100644 >> --- a/tools/perf/builtin-probe.c >> +++ b/tools/perf/builtin-probe.c >> @@ -55,6 +55,7 @@ static struct { >> bool show_funcs; >> bool mod_events; >> bool uprobes; >> + bool sdt; >> int nevents; >> struct perf_probe_event events[MAX_PROBES]; >> struct strlist *dellist; >> @@ -325,6 +326,8 @@ int cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused) >> opt_set_filter), >> OPT_CALLBACK('x', "exec", NULL, "executable|path", >> "target executable name or path", opt_set_target), >> + OPT_BOOLEAN('S', "sdt", ¶ms.sdt, >> + "Show and probe on the SDT markers"), > > You need to add it to Documentation/perf-probe.txt too. In addition if > the --sdt option is only able to work with libelf, it should be wrapped > into the #ifdef LIBELF_SUPPORT pair. > > And I'm not sure that it's a good idea to have two behavior on a single > option (S) - show and probe (add). Maybe it can be separated into two > or the S option can be used as a flag with existing --list and --add > option? >
Good catch! :) No, that is really bad idea. All probes must be added by "--add" action. So we need a new probe syntax for specifying sdt marker.
How about the below syntax?
[EVENT=]%PROVIDER:MARKER [ARG ...]
Of course, this will require to list up all markers with "%" prefix for continuity.
And since --list option is to list up all existing(defined) probe events, I think --markers (as like as --funcs) is better for listing it up.
Thank you!
-- Masami HIRAMATSU IT Management Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com
| |