On 2015/05/06 0:12, Arnaldo Carvalho de Melo wrote:
> Em Wed, May 06, 2015 at 12:06:16AM +0900, Masami Hiramatsu escreveu:
>> On 2015/05/05 11:29, Masami Hiramatsu wrote:
>>> To simplify the perf-probe command code, consolidate some
>>> similar functions and use command short-name for command
>>> classification, instead of separated booleans.
>>
>> Oops, I've found my mistake on this patch. perf-probe without
>> any command but probe defs should imply "--add", but this patch
>> drops that.
>> I'll update this for v5 patch.
>
> Humm, ok, so I'll drop this one from my perf/core branch now, to keep it
> bisectable, ok?
>
Yes, I've sent the newer version of this patch.
Thank you!
> - Arnaldo
>
>> Thank you,
>>
>>>
>>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu...@hitachi.com>
>>> ---
>>> tools/perf/builtin-probe.c | 105
>>> ++++++++++++++++----------------------------
>>> 1 file changed, 37 insertions(+), 68 deletions(-)
>>>
>>> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
>>> index 08c9481..17870b8 100644
>>> --- a/tools/perf/builtin-probe.c
>>> +++ b/tools/perf/builtin-probe.c
>>> @@ -48,14 +48,10 @@
>>>
>>> /* Session management structure */
>>> static struct {
>>> + int command; /* Command short_name */
>>> bool list_events;
>>> bool force_add;
>>> - bool show_lines;
>>> - bool show_vars;
>>> bool show_ext_vars;
>>> - bool show_funcs;
>>> - bool mod_events;
>>> - bool del_events;
>>> bool uprobes;
>>> bool quiet;
>>> bool target_used;
>>> @@ -175,44 +171,11 @@ static int parse_probe_event_argv(int argc, const
>>> char **argv)
>>>
>>> len += sprintf(&buf[len], "%s ", argv[i]);
>>> }
>>> - params.mod_events = true;
>>> ret = parse_probe_event(buf);
>>> free(buf);
>>> return ret;
>>> }
>>>
>>> -static int opt_add_probe_event(const struct option *opt __maybe_unused,
>>> - const char *str, int unset __maybe_unused)
>>> -{
>>> - if (str) {
>>> - params.mod_events = true;
>>> - return parse_probe_event(str);
>>> - } else
>>> - return 0;
>>> -}
>>> -
>>> -static int opt_del_probe_event(const struct option *opt __maybe_unused,
>>> - const char *str, int unset __maybe_unused)
>>> -{
>>> - if (str) {
>>> - params.del_events = true;
>>> - return params_add_filter(str);
>>> - }
>>> - return 0;
>>> -}
>>> -
>>> -static int opt_list_probe_event(const struct option *opt __maybe_unused,
>>> - const char *str, int unset)
>>> -{
>>> - if (!unset)
>>> - params.list_events = true;
>>> -
>>> - if (str)
>>> - return params_add_filter(str);
>>> -
>>> - return 0;
>>> -}
>>> -
>>> static int opt_set_target(const struct option *opt, const char *str,
>>> int unset __maybe_unused)
>>> {
>>> @@ -250,8 +213,10 @@ static int opt_set_target(const struct option *opt,
>>> const char *str,
>>> return ret;
>>> }
>>>
>>> +/* Command option callbacks */
>>> +
>>> #ifdef HAVE_DWARF_SUPPORT
>>> -static int opt_show_lines(const struct option *opt __maybe_unused,
>>> +static int opt_show_lines(const struct option *opt,
>>> const char *str, int unset __maybe_unused)
>>> {
>>> int ret = 0;
>>> @@ -259,19 +224,19 @@ static int opt_show_lines(const struct option *opt
>>> __maybe_unused,
>>> if (!str)
>>> return 0;
>>>
>>> - if (params.show_lines) {
>>> + if (params.command == 'L') {
>>> pr_warning("Warning: more than one --line options are"
>>> " detected. Only the first one is valid.\n");
>>> return 0;
>>> }
>>>
>>> - params.show_lines = true;
>>> + params.command = opt->short_name;
>>> ret = parse_line_range_desc(str, ¶ms.line_range);
>>>
>>> return ret;
>>> }
>>>
>>> -static int opt_show_vars(const struct option *opt __maybe_unused,
>>> +static int opt_show_vars(const struct option *opt,
>>> const char *str, int unset __maybe_unused)
>>> {
>>> struct perf_probe_event *pev = ¶ms.events[params.nevents];
>>> @@ -285,16 +250,27 @@ static int opt_show_vars(const struct option *opt
>>> __maybe_unused,
>>> pr_err(" Error: '--vars' doesn't accept arguments.\n");
>>> return -EINVAL;
>>> }
>>> - params.show_vars = true;
>>> + params.command = opt->short_name;
>>>
>>> return ret;
>>> }
>>> #endif
>>> -static int opt_show_funcs(const struct option *opt __maybe_unused,
>>> - const char *str, int unset)
>>> +static int opt_add_probe_event(const struct option *opt,
>>> + const char *str, int unset __maybe_unused)
>>> +{
>>> + if (str) {
>>> + params.command = opt->short_name;
>>> + return parse_probe_event(str);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int opt_set_filter_with_command(const struct option *opt,
>>> + const char *str, int unset)
>>> {
>>> if (!unset)
>>> - params.show_funcs = true;
>>> + params.command = opt->short_name;
>>>
>>> if (str)
>>> return params_add_filter(str);
>>> @@ -360,10 +336,10 @@ __cmd_probe(int argc, const char **argv, const char
>>> *prefix __maybe_unused)
>>> OPT_BOOLEAN('q', "quiet", ¶ms.quiet,
>>> "be quiet (do not show any mesages)"),
>>> OPT_CALLBACK_DEFAULT('l', "list", NULL, "[GROUP:]EVENT",
>>> - "list up probe events", opt_list_probe_event,
>>> - DEFAULT_LIST_FILTER),
>>> + "list up probe events",
>>> + opt_set_filter_with_command, DEFAULT_LIST_FILTER),
>>> OPT_CALLBACK('d', "del", NULL, "[GROUP:]EVENT", "delete a probe event.",
>>> - opt_del_probe_event),
>>> + opt_set_filter_with_command),
>>> OPT_CALLBACK('a', "add", NULL,
>>> #ifdef HAVE_DWARF_SUPPORT
>>> "[EVENT=]FUNC[@SRC][+OFF|%return|:RL|;PT]|SRC:AL|SRC;PT"
>>> @@ -412,7 +388,7 @@ __cmd_probe(int argc, const char **argv, const char
>>> *prefix __maybe_unused)
>>> "Set how many probe points can be found for a probe."),
>>> OPT_CALLBACK_DEFAULT('F', "funcs", NULL, "[FILTER]",
>>> "Show potential probe-able functions.",
>>> - opt_show_funcs, DEFAULT_FUNC_FILTER),
>>> + opt_set_filter_with_command, DEFAULT_FUNC_FILTER),
>>> OPT_CALLBACK('\0', "filter", NULL,
>>> "[!]FILTER", "Set a filter (with --vars/funcs only)\n"
>>> "\t\t\t(default: \"" DEFAULT_VAR_FILTER "\" for --vars,\n"
>>> @@ -462,16 +438,13 @@ __cmd_probe(int argc, const char **argv, const char
>>> *prefix __maybe_unused)
>>> if (params.max_probe_points == 0)
>>> params.max_probe_points = MAX_PROBES;
>>>
>>> - if ((!params.nevents && !params.del_events && !params.list_events &&
>>> - !params.show_lines && !params.show_funcs))
>>> - usage_with_options(probe_usage, options);
>>> -
>>> /*
>>> * Only consider the user's kernel image path if given.
>>> */
>>> symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);
>>>
>>> - if (params.list_events) {
>>> + switch (params.command) {
>>> + case 'l':
>>> if (params.uprobes) {
>>> pr_warning(" Error: Don't use --list with --exec.\n");
>>> usage_with_options(probe_usage, options);
>>> @@ -480,24 +453,20 @@ __cmd_probe(int argc, const char **argv, const char
>>> *prefix __maybe_unused)
>>> if (ret < 0)
>>> pr_err_with_code(" Error: Failed to show event list.",
>>> ret);
>>> return ret;
>>> - }
>>> - if (params.show_funcs) {
>>> + case 'F':
>>> ret = show_available_funcs(params.target, params.filter,
>>> params.uprobes);
>>> if (ret < 0)
>>> pr_err_with_code(" Error: Failed to show functions.",
>>> ret);
>>> return ret;
>>> - }
>>> -
>>> #ifdef HAVE_DWARF_SUPPORT
>>> - if (params.show_lines) {
>>> + case 'L':
>>> ret = show_line_range(¶ms.line_range, params.target,
>>> params.uprobes);
>>> if (ret < 0)
>>> pr_err_with_code(" Error: Failed to show lines.", ret);
>>> return ret;
>>> - }
>>> - if (params.show_vars) {
>>> + case 'V':
>>> if (!params.filter)
>>> params.filter = strfilter__new(DEFAULT_VAR_FILTER,
>>> NULL);
>>> @@ -510,18 +479,15 @@ __cmd_probe(int argc, const char **argv, const char
>>> *prefix __maybe_unused)
>>> if (ret < 0)
>>> pr_err_with_code(" Error: Failed to show vars.", ret);
>>> return ret;
>>> - }
>>> #endif
>>> -
>>> - if (params.del_events) {
>>> + case 'd':
>>> ret = del_perf_probe_events(params.filter);
>>> if (ret < 0) {
>>> pr_err_with_code(" Error: Failed to delete events.",
>>> ret);
>>> return ret;
>>> }
>>> - }
>>> -
>>> - if (params.nevents) {
>>> + break;
>>> + case 'a':
>>> /* Ensure the last given target is used */
>>> if (params.target && !params.target_used) {
>>> pr_warning(" Error: -x/-m must follow the probe
>>> definitions.\n");
>>> @@ -535,6 +501,9 @@ __cmd_probe(int argc, const char **argv, const char
>>> *prefix __maybe_unused)
>>> pr_err_with_code(" Error: Failed to add events.", ret);
>>> return ret;
>>> }
>>> + break;
>>> + default:
>>> + usage_with_options(probe_usage, options);
>>> }
>>> return 0;
>>> }
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> Please read the FAQ at http://www.tux.org/lkml/
>>>
>>
>>
>> --
>> 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...@hitachi.com
>
--
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...@hitachi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
