Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2993361AbbEEQKn (ORCPT ); Tue, 5 May 2015 12:10:43 -0400 Received: from mail7.hitachi.co.jp ([133.145.228.42]:56907 "EHLO mail7.hitachi.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2993418AbbEEPhA (ORCPT ); Tue, 5 May 2015 11:37:00 -0400 Message-ID: <5548E393.7080205@hitachi.com> Date: Wed, 06 May 2015 00:36:51 +0900 From: Masami Hiramatsu Organization: Hitachi, Ltd., Japan User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Arnaldo Carvalho de Melo CC: Peter Zijlstra , Linux Kernel Mailing List , David Ahern , namhyung@kernel.org, Jiri Olsa , Ingo Molnar Subject: Re: Re: [PATCH perf/core v4 4/4] perf probe: Cleanup and consolidate command parsers References: <20150505022945.23399.993.stgit@localhost.localdomain> <20150505022954.23399.61078.stgit@localhost.localdomain> <5548DC68.5020808@hitachi.com> <20150505151242.GT10475@kernel.org> In-Reply-To: <20150505151242.GT10475@kernel.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9825 Lines: 294 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 >>> --- >>> 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 majordomo@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.pt@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.pt@hitachi.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/