lkml.org 
[lkml]   [2015]   [May]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: Re: [PATCH perf/core v4 4/4] perf probe: Cleanup and consolidate command parsers
    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.pt@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, &params.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 = &params.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", &params.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(&params.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


    \
     
     \ /
      Last update: 2015-05-05 18:41    [W:4.081 / U:0.476 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site