2014-10-22 15:16:00

by Namhyung Kim

Subject: [PATCHSET 0/5] perf tools: option parsing improvement

Hello,

This patchset tries to enhance option parser a bit. Patch 1-3 are to
reuse existing perf record options for other commands like perf kvm
stat record. Patch 4-5 are to support exclusive options that cannot
be used at the same time. The perf probe has such options and upcoming
sdt-cache command also.

You can get it from 'perf/option-share-v2' branch on my tree:

git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

Any comments are welcome, thanks
Namhyung


Cc: Masami Hiramatsu <[email protected]>
Cc: Hemant Kumar <[email protected]>
Cc: Alexander Yarygin <[email protected]>


Namhyung Kim (5):
perf tools: Add PARSE_OPT_DISABLED flag
perf tools: Export usage string and option table of perf record
perf kvm: Print kvm specific --help output
perf tools: Add support for exclusive option
perf probe: Use PARSE_OPT_EXCLUSIVE flag

tools/perf/builtin-kvm.c | 25 +++++++++++++
tools/perf/builtin-probe.c | 54 +++++-----------------------
tools/perf/builtin-record.c | 7 ++--
tools/perf/builtin-script.c | 1 -
tools/perf/builtin-timechart.c | 7 ++--
tools/perf/perf.h | 3 ++
tools/perf/util/parse-options.c | 78 ++++++++++++++++++++++++++++++++++-------
tools/perf/util/parse-options.h | 4 +++
8 files changed, 116 insertions(+), 63 deletions(-)

--
2.0.0

2014-10-22 15:16:08

by Namhyung Kim

Subject: [PATCH 1/5] perf tools: Add PARSE_OPT_DISABLED flag

In some cases, we need to reuse exising options with some of them
disabled. To do that, add PARSE_OPT_DISABLED flag and
set_option_flag() function.

Cc: Masami Hiramatsu <[email protected]>
Cc: Hemant Kumar <[email protected]>
Cc: Alexander Yarygin <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/parse-options.c | 17 +++++++++++++++++
tools/perf/util/parse-options.h | 2 ++
2 files changed, 19 insertions(+)

diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c
index bf48092983c6..b6016101b40b 100644
--- a/tools/perf/util/parse-options.c
+++ b/tools/perf/util/parse-options.c
@@ -42,6 +42,8 @@ static int get_value(struct parse_opt_ctx_t *p,
return opterror(opt, "takes no value", flags);
if (unset && (opt->flags & PARSE_OPT_NONEG))
return opterror(opt, "isn't available", flags);
+ if (opt->flags & PARSE_OPT_DISABLED)
+ return opterror(opt, "is not usable", flags);

if (!(flags & OPT_SHORT) && p->opt) {
switch (opt->type) {
@@ -509,6 +511,8 @@ static void print_option_help(const struct option *opts, int full)
}
if (!full && (opts->flags & PARSE_OPT_HIDDEN))
return;
+ if (opts->flags & PARSE_OPT_DISABLED)
+ return;

pos = fprintf(stderr, " ");
if (opts->short_name)
@@ -679,3 +683,16 @@ int parse_opt_verbosity_cb(const struct option *opt,
}
return 0;
}
+
+void set_option_flag(struct option *opts, int shortopt, const char *longopt,
+ int flag)
+{
+ for (; opts->type != OPTION_END; opts++) {
+ if ((shortopt && opts->short_name == shortopt) ||
+ (opts->long_name && longopt &&
+ !strcmp(opts->long_name, longopt))) {
+ opts->flags |= flag;
+ break;
+ }
+ }
+}
diff --git a/tools/perf/util/parse-options.h b/tools/perf/util/parse-options.h
index b59ba858e73d..b7c80dbc7627 100644
--- a/tools/perf/util/parse-options.h
+++ b/tools/perf/util/parse-options.h
@@ -38,6 +38,7 @@ enum parse_opt_option_flags {
PARSE_OPT_NONEG = 4,
PARSE_OPT_HIDDEN = 8,
PARSE_OPT_LASTARG_DEFAULT = 16,
+ PARSE_OPT_DISABLED = 32,
};

struct option;
@@ -211,4 +212,5 @@ extern int parse_opt_verbosity_cb(const struct option *, const char *, int);

extern const char *parse_options_fix_filename(const char *prefix, const char *file);

+void set_option_flag(struct option *opts, int sopt, const char *lopt, int flag);
#endif /* __PERF_PARSE_OPTIONS_H */
--
2.0.0

2014-10-22 15:16:12

by Namhyung Kim

Subject: [PATCH 3/5] perf kvm: Print kvm specific --help output

The 'perf kvm stat record' tool is an alias of 'perf record' with
predefined kvm related options. All options that passed to 'perf kvm
stat record' are processed by the 'perf record' tool. So, 'perf kvm
stat record --help' prints help of usage for the 'perf record'
command. There are a few options useful for 'perf kvm stat record',
the rest either break kvm related output or don't change it.

Let's print safe for 'perf kvm stat record' options in addition to
general 'perf record' --help output.

With this patch, new output looks like below:

$ perf kvm stat record -h

usage: perf kvm stat record [<options>]

-p, --pid <pid> record events on existing process id
-t, --tid <tid> record events on existing thread id
-r, --realtime <n> collect data with this RT SCHED_FIFO priority
--no-buffering collect data without buffering
-a, --all-cpus system-wide collection from all CPUs
-C, --cpu <cpu> list of cpus to monitor
-c, --count <n> event period to sample
-o, --output <file> output file name
-i, --no-inherit child tasks do not inherit counters
-m, --mmap-pages <pages>
number of mmap data pages
-v, --verbose be more verbose (show counter open errors, etc)
-q, --quiet don't print any message
-s, --stat per thread counts
-D, --delay <n> ms to wait before starting measurement after program start
-u, --uid <user> user to profile
--per-thread use per-thread mmaps

$ perf kvm stat record -n sleep 1
Error: switch `n' is not usable

usage: perf kvm stat record [<options>]

Cc: Alexander Yarygin <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/builtin-kvm.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index b65eb0507b38..3c0f3d4fb021 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -1132,6 +1132,10 @@ kvm_events_record(struct perf_kvm_stat *kvm, int argc, const char **argv)
"-m", "1024",
"-c", "1",
};
+ const char * const kvm_stat_record_usage[] = {
+ "perf kvm stat record [<options>]",
+ NULL
+ };
const char * const *events_tp;
events_tp_size = 0;

@@ -1159,6 +1163,27 @@ kvm_events_record(struct perf_kvm_stat *kvm, int argc, const char **argv)
for (j = 1; j < (unsigned int)argc; j++, i++)
rec_argv[i] = argv[j];

+ set_option_flag(record_options, 'e', "event", PARSE_OPT_HIDDEN);
+ set_option_flag(record_options, 0, "filter", PARSE_OPT_HIDDEN);
+ set_option_flag(record_options, 'R', "raw-samples", PARSE_OPT_HIDDEN);
+
+ set_option_flag(record_options, 'F', "freq", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 0, "group", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 'g', NULL, PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 0, "call-graph", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 'd', "data", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 'T', "timestamp", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 'P', "period", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 'n', "no-samples", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 'N', "no-buildid-cache", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 'B', "no-buildid", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 'G', "cgroup", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 'b', "branch-any", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 'j', "branch-filter", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 'W', "weight", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 0, "transaction", PARSE_OPT_DISABLED);
+
+ record_usage = kvm_stat_record_usage;
return cmd_record(i, rec_argv, NULL);
}

--
2.0.0

2014-10-22 15:16:16

by Namhyung Kim

Subject: [PATCH 4/5] perf tools: Add support for exclusive option

Some options cannot be used at the same time. To handle such options
add a new PARSE_OPT_EXCLUSIVE flag and show error message if more than
one of them is used.

Cc: Masami Hiramatsu <[email protected]>
Cc: Hemant Kumar <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/parse-options.c | 59 +++++++++++++++++++++++++++++++++--------
tools/perf/util/parse-options.h | 2 ++
2 files changed, 50 insertions(+), 11 deletions(-)

diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c
index b6016101b40b..f62dee7bd924 100644
--- a/tools/perf/util/parse-options.c
+++ b/tools/perf/util/parse-options.c
@@ -45,6 +45,23 @@ static int get_value(struct parse_opt_ctx_t *p,
if (opt->flags & PARSE_OPT_DISABLED)
return opterror(opt, "is not usable", flags);

+ if (opt->flags & PARSE_OPT_EXCLUSIVE) {
+ if (p->excl_opt) {
+ char msg[128];
+
+ if (((flags & OPT_SHORT) && p->excl_opt->short_name) ||
+ p->excl_opt->long_name == NULL) {
+ scnprintf(msg, sizeof(msg), "cannot be used with switch `%c'",
+ p->excl_opt->short_name);
+ } else {
+ scnprintf(msg, sizeof(msg), "cannot be used with %s",
+ p->excl_opt->long_name);
+ }
+ opterror(opt, msg, flags);
+ return -3;
+ }
+ p->excl_opt = opt;
+ }
if (!(flags & OPT_SHORT) && p->opt) {
switch (opt->type) {
case OPTION_CALLBACK:
@@ -345,13 +362,14 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
const char * const usagestr[])
{
int internal_help = !(ctx->flags & PARSE_OPT_NO_INTERNAL_HELP);
+ int excl_short_opt = 1;
+ const char *arg;

/* we must reset ->opt, unknown short option leave it dangling */
ctx->opt = NULL;

for (; ctx->argc; ctx->argc--, ctx->argv++) {
- const char *arg = ctx->argv[0];
-
+ arg = ctx->argv[0];
if (*arg != '-' || !arg[1]) {
if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
break;
@@ -360,19 +378,21 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
}

if (arg[1] != '-') {
- ctx->opt = arg + 1;
+ ctx->opt = ++arg;
if (internal_help && *ctx->opt == 'h')
return usage_with_options_internal(usagestr, options, 0);
switch (parse_short_opt(ctx, options)) {
case -1:
- return parse_options_usage(usagestr, options, arg + 1, 1);
+ return parse_options_usage(usagestr, options, arg, 1);
case -2:
goto unknown;
+ case -3:
+ goto exclusive;
default:
break;
}
if (ctx->opt)
- check_typos(arg + 1, options);
+ check_typos(arg, options);
while (ctx->opt) {
if (internal_help && *ctx->opt == 'h')
return usage_with_options_internal(usagestr, options, 0);
@@ -389,6 +409,8 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
ctx->argv[0] = strdup(ctx->opt - 1);
*(char *)ctx->argv[0] = '-';
goto unknown;
+ case -3:
+ goto exclusive;
default:
break;
}
@@ -404,19 +426,23 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
break;
}

- if (internal_help && !strcmp(arg + 2, "help-all"))
+ arg += 2;
+ if (internal_help && !strcmp(arg, "help-all"))
return usage_with_options_internal(usagestr, options, 1);
- if (internal_help && !strcmp(arg + 2, "help"))
+ if (internal_help && !strcmp(arg, "help"))
return usage_with_options_internal(usagestr, options, 0);
- if (!strcmp(arg + 2, "list-opts"))
+ if (!strcmp(arg, "list-opts"))
return PARSE_OPT_LIST_OPTS;
- if (!strcmp(arg + 2, "list-cmds"))
+ if (!strcmp(arg, "list-cmds"))
return PARSE_OPT_LIST_SUBCMDS;
- switch (parse_long_opt(ctx, arg + 2, options)) {
+ switch (parse_long_opt(ctx, arg, options)) {
case -1:
- return parse_options_usage(usagestr, options, arg + 2, 0);
+ return parse_options_usage(usagestr, options, arg, 0);
case -2:
goto unknown;
+ case -3:
+ excl_short_opt = 0;
+ goto exclusive;
default:
break;
}
@@ -428,6 +454,17 @@ unknown:
ctx->opt = NULL;
}
return PARSE_OPT_DONE;
+
+exclusive:
+ parse_options_usage(usagestr, options, arg, excl_short_opt);
+ if ((excl_short_opt && ctx->excl_opt->short_name) ||
+ ctx->excl_opt->long_name == NULL) {
+ char opt = ctx->excl_opt->short_name;
+ parse_options_usage(NULL, options, &opt, 1);
+ } else {
+ parse_options_usage(NULL, options, ctx->excl_opt->long_name, 0);
+ }
+ return PARSE_OPT_HELP;
}

int parse_options_end(struct parse_opt_ctx_t *ctx)
diff --git a/tools/perf/util/parse-options.h b/tools/perf/util/parse-options.h
index b7c80dbc7627..97b153fb4999 100644
--- a/tools/perf/util/parse-options.h
+++ b/tools/perf/util/parse-options.h
@@ -39,6 +39,7 @@ enum parse_opt_option_flags {
PARSE_OPT_HIDDEN = 8,
PARSE_OPT_LASTARG_DEFAULT = 16,
PARSE_OPT_DISABLED = 32,
+ PARSE_OPT_EXCLUSIVE = 64,
};

struct option;
@@ -174,6 +175,7 @@ struct parse_opt_ctx_t {
const char **out;
int argc, cpidx;
const char *opt;
+ const struct option *excl_opt;
int flags;
};

--
2.0.0

2014-10-22 15:16:24

by Namhyung Kim

Subject: [PATCH 5/5] perf probe: Use PARSE_OPT_EXCLUSIVE flag

The perf probe has some exclusive options. Use new PARSE_OPT_EXCLUSIVE
flag to simplify the code and show more compact usage.

$ perf probe -l -a foo
Error: switch `a' cannot be used with switch `l'

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'

-a, --add <[EVENT=]FUNC[@SRC][+OFF|%return|:RL|;PT]|SRC:AL|SRC;PT [[NAME=]ARG ...]>
probe point definition, where
GROUP: Group name (optional)
EVENT: Event name
FUNC: Function name
OFF: Offset from function entry (in byte)
%return: Put the probe at function return
SRC: Source code path
RL: Relative line number from function entry.
AL: Absolute line number in file.
PT: Lazy expression of line code.
ARG: Probe argument (local variable name or
kprobe-tracer argument format.)

-l, --list list up current probe events

Cc: Masami Hiramatsu <[email protected]>
Cc: Hemant Kumar <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/builtin-probe.c | 54 ++++++++--------------------------------------
1 file changed, 9 insertions(+), 45 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 04412b4770a2..23d6e7f03cf1 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -312,7 +312,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
#endif
NULL
};
- const struct option options[] = {
+ struct option options[] = {
OPT_INCR('v', "verbose", &verbose,
"be more verbose (show parsed arguments, etc)"),
OPT_BOOLEAN('l', "list", &params.list_events,
@@ -382,6 +382,14 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
};
int ret;

+ set_option_flag(options, 'a', "add", PARSE_OPT_EXCLUSIVE);
+ set_option_flag(options, 'd', "del", PARSE_OPT_EXCLUSIVE);
+ set_option_flag(options, 'l', "list", PARSE_OPT_EXCLUSIVE);
+#ifdef HAVE_DWARF_SUPPORT
+ set_option_flag(options, 'L', "line", PARSE_OPT_EXCLUSIVE);
+ set_option_flag(options, 'V', "vars", PARSE_OPT_EXCLUSIVE);
+#endif
+
argc = parse_options(argc, argv, options, probe_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
if (argc > 0) {
@@ -409,22 +417,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);

if (params.list_events) {
- if (params.mod_events) {
- pr_err(" Error: Don't use --list with --add/--del.\n");
- usage_with_options(probe_usage, options);
- }
- if (params.show_lines) {
- pr_err(" Error: Don't use --list with --line.\n");
- usage_with_options(probe_usage, options);
- }
- if (params.show_vars) {
- pr_err(" Error: Don't use --list with --vars.\n");
- usage_with_options(probe_usage, options);
- }
- if (params.show_funcs) {
- pr_err(" Error: Don't use --list with --funcs.\n");
- usage_with_options(probe_usage, options);
- }
if (params.uprobes) {
pr_warning(" Error: Don't use --list with --exec.\n");
usage_with_options(probe_usage, options);
@@ -435,19 +427,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
return ret;
}
if (params.show_funcs) {
- if (params.nevents != 0 || params.dellist) {
- pr_err(" Error: Don't use --funcs with"
- " --add/--del.\n");
- usage_with_options(probe_usage, options);
- }
- if (params.show_lines) {
- pr_err(" Error: Don't use --funcs with --line.\n");
- usage_with_options(probe_usage, options);
- }
- if (params.show_vars) {
- pr_err(" Error: Don't use --funcs with --vars.\n");
- usage_with_options(probe_usage, options);
- }
if (!params.filter)
params.filter = strfilter__new(DEFAULT_FUNC_FILTER,
NULL);
@@ -462,16 +441,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)

#ifdef HAVE_DWARF_SUPPORT
if (params.show_lines) {
- if (params.mod_events) {
- pr_err(" Error: Don't use --line with"
- " --add/--del.\n");
- usage_with_options(probe_usage, options);
- }
- if (params.show_vars) {
- pr_err(" Error: Don't use --line with --vars.\n");
- usage_with_options(probe_usage, options);
- }
-
ret = show_line_range(&params.line_range, params.target,
params.uprobes);
if (ret < 0)
@@ -479,11 +448,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
return ret;
}
if (params.show_vars) {
- if (params.mod_events) {
- pr_err(" Error: Don't use --vars with"
- " --add/--del.\n");
- usage_with_options(probe_usage, options);
- }
if (!params.filter)
params.filter = strfilter__new(DEFAULT_VAR_FILTER,
NULL);
--
2.0.0

2014-10-22 15:17:35

by Namhyung Kim

Subject: [PATCH 2/5] perf tools: Export usage string and option table of perf record

Those are shared with other builtin commands like kvm, script. So
make it accessable from them. This is a preparation of later change
that limiting possible options.

Cc: Masami Hiramatsu <[email protected]>
Cc: Hemant Kumar <[email protected]>
Cc: Alexander Yarygin <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/builtin-record.c | 7 +++++--
tools/perf/builtin-script.c | 1 -
tools/perf/builtin-timechart.c | 7 ++++---
tools/perf/perf.h | 3 +++
4 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 2583a9b04317..5091a27e6d28 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -680,11 +680,12 @@ static int perf_record_config(const char *var, const char *value, void *cb)
return perf_default_config(var, value, cb);
}

-static const char * const record_usage[] = {
+static const char * const __record_usage[] = {
"perf record [<options>] [<command>]",
"perf record [<options>] -- <command> [<options>]",
NULL
};
+const char * const *record_usage = __record_usage;

/*
* XXX Ideally would be local to cmd_record() and passed to a record__new
@@ -725,7 +726,7 @@ const char record_callchain_help[] = CALLCHAIN_HELP "fp";
* perf_evlist__prepare_workload, etc instead of fork+exec'in 'perf record',
* using pipes, etc.
*/
-const struct option record_options[] = {
+struct option __record_options[] = {
OPT_CALLBACK('e', "event", &record.evlist, "event",
"event selector. use 'perf list' to list available events",
parse_events_option),
@@ -802,6 +803,8 @@ const struct option record_options[] = {
OPT_END()
};

+struct option *record_options = __record_options;
+
int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
{
int err = -ENOMEM;
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 9708a1290571..28727c4fdc5d 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -23,7 +23,6 @@ static char const *generate_script_lang;
static bool debug_mode;
static u64 last_timestamp;
static u64 nr_unordered;
-extern const struct option record_options[];
static bool no_callchain;
static bool latency_format;
static bool system_wide;
diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index 35b425b6293f..9a68fda17cb5 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -1963,7 +1963,7 @@ int cmd_timechart(int argc, const char **argv,
NULL
};

- const struct option record_options[] = {
+ const struct option timechart_record_options[] = {
OPT_BOOLEAN('P', "power-only", &tchart.power_only, "output power data only"),
OPT_BOOLEAN('T', "tasks-only", &tchart.tasks_only,
"output processes data only"),
@@ -1972,7 +1972,7 @@ int cmd_timechart(int argc, const char **argv,
OPT_BOOLEAN('g', "callchain", &tchart.with_backtrace, "record callchain"),
OPT_END()
};
- const char * const record_usage[] = {
+ const char * const timechart_record_usage[] = {
"perf timechart record [<options>]",
NULL
};
@@ -1985,7 +1985,8 @@ int cmd_timechart(int argc, const char **argv,
}

if (argc && !strncmp(argv[0], "rec", 3)) {
- argc = parse_options(argc, argv, record_options, record_usage,
+ argc = parse_options(argc, argv, timechart_record_options,
+ timechart_record_usage,
PARSE_OPT_STOP_AT_NON_OPTION);

if (tchart.power_only && tchart.tasks_only) {
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 220d44e44c1b..511c2831aa81 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -62,4 +62,7 @@ struct record_opts {
unsigned initial_delay;
};

+struct option;
+extern const char * const *record_usage;
+extern struct option *record_options;
#endif
--
2.0.0

2014-10-23 03:06:59

by Hemant Kumar

Subject: Re: [PATCHSET 0/5] perf tools: option parsing improvement

Hi Namhyung,
On 10/22/2014 08:45 PM, Namhyung Kim wrote:
> Hello,
>
> This patchset tries to enhance option parser a bit. Patch 1-3 are to
> reuse existing perf record options for other commands like perf kvm
> stat record. Patch 4-5 are to support exclusive options that cannot
> be used at the same time. The perf probe has such options and upcoming
> sdt-cache command also.

Thank you for working on this.

[SNIP]

--
Thanks,
Hemant Kumar

2014-10-23 05:00:46

by 平松雅巳 / HIRAMATU，MASAMI

Subject: Re: [PATCH 5/5] perf probe: Use PARSE_OPT_EXCLUSIVE flag

Hi Namhyng,


(2014/10/23 0:15), Namhyung Kim wrote:
> The perf probe has some exclusive options. Use new PARSE_OPT_EXCLUSIVE
> flag to simplify the code and show more compact usage.
>
> $ perf probe -l -a foo
> Error: switch `a' cannot be used with switch `l'
>
> 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'
>
> -a, --add <[EVENT=]FUNC[@SRC][+OFF|%return|:RL|;PT]|SRC:AL|SRC;PT [[NAME=]ARG ...]>
> probe point definition, where
> GROUP: Group name (optional)
> EVENT: Event name
> FUNC: Function name
> OFF: Offset from function entry (in byte)
> %return: Put the probe at function return
> SRC: Source code path
> RL: Relative line number from function entry.
> AL: Absolute line number in file.
> PT: Lazy expression of line code.
> ARG: Probe argument (local variable name or
> kprobe-tracer argument format.)
>
> -l, --list list up current probe events
>

Thanks! this looks very good to me:)

Acked-by: Masami Hiramatsu <[email protected]>

> Cc: Masami Hiramatsu <[email protected]>
> Cc: Hemant Kumar <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> tools/perf/builtin-probe.c | 54 ++++++++--------------------------------------
> 1 file changed, 9 insertions(+), 45 deletions(-)
>
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index 04412b4770a2..23d6e7f03cf1 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -312,7 +312,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> #endif
> NULL
> };
> - const struct option options[] = {
> + struct option options[] = {
> OPT_INCR('v', "verbose", &verbose,
> "be more verbose (show parsed arguments, etc)"),
> OPT_BOOLEAN('l', "list", &params.list_events,
> @@ -382,6 +382,14 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> };
> int ret;
>
> + set_option_flag(options, 'a', "add", PARSE_OPT_EXCLUSIVE);
> + set_option_flag(options, 'd', "del", PARSE_OPT_EXCLUSIVE);
> + set_option_flag(options, 'l', "list", PARSE_OPT_EXCLUSIVE);
> +#ifdef HAVE_DWARF_SUPPORT
> + set_option_flag(options, 'L', "line", PARSE_OPT_EXCLUSIVE);
> + set_option_flag(options, 'V', "vars", PARSE_OPT_EXCLUSIVE);
> +#endif
> +
> argc = parse_options(argc, argv, options, probe_usage,
> PARSE_OPT_STOP_AT_NON_OPTION);
> if (argc > 0) {
> @@ -409,22 +417,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);
>
> if (params.list_events) {
> - if (params.mod_events) {
> - pr_err(" Error: Don't use --list with --add/--del.\n");
> - usage_with_options(probe_usage, options);
> - }
> - if (params.show_lines) {
> - pr_err(" Error: Don't use --list with --line.\n");
> - usage_with_options(probe_usage, options);
> - }
> - if (params.show_vars) {
> - pr_err(" Error: Don't use --list with --vars.\n");
> - usage_with_options(probe_usage, options);
> - }
> - if (params.show_funcs) {
> - pr_err(" Error: Don't use --list with --funcs.\n");
> - usage_with_options(probe_usage, options);
> - }
> if (params.uprobes) {
> pr_warning(" Error: Don't use --list with --exec.\n");
> usage_with_options(probe_usage, options);
> @@ -435,19 +427,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> return ret;
> }
> if (params.show_funcs) {
> - if (params.nevents != 0 || params.dellist) {
> - pr_err(" Error: Don't use --funcs with"
> - " --add/--del.\n");
> - usage_with_options(probe_usage, options);
> - }
> - if (params.show_lines) {
> - pr_err(" Error: Don't use --funcs with --line.\n");
> - usage_with_options(probe_usage, options);
> - }
> - if (params.show_vars) {
> - pr_err(" Error: Don't use --funcs with --vars.\n");
> - usage_with_options(probe_usage, options);
> - }
> if (!params.filter)
> params.filter = strfilter__new(DEFAULT_FUNC_FILTER,
> NULL);
> @@ -462,16 +441,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>
> #ifdef HAVE_DWARF_SUPPORT
> if (params.show_lines) {
> - if (params.mod_events) {
> - pr_err(" Error: Don't use --line with"
> - " --add/--del.\n");
> - usage_with_options(probe_usage, options);
> - }
> - if (params.show_vars) {
> - pr_err(" Error: Don't use --line with --vars.\n");
> - usage_with_options(probe_usage, options);
> - }
> -
> ret = show_line_range(&params.line_range, params.target,
> params.uprobes);
> if (ret < 0)
> @@ -479,11 +448,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> return ret;
> }
> if (params.show_vars) {
> - if (params.mod_events) {
> - pr_err(" Error: Don't use --vars with"
> - " --add/--del.\n");
> - usage_with_options(probe_usage, options);
> - }
> if (!params.filter)
> params.filter = strfilter__new(DEFAULT_VAR_FILTER,
> NULL);
>


--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]

2014-10-23 05:05:19

by 平松雅巳 / HIRAMATU，MASAMI

Subject: Re: [PATCH 4/5] perf tools: Add support for exclusive option

(2014/10/23 0:15), Namhyung Kim wrote:
> Some options cannot be used at the same time. To handle such options
> add a new PARSE_OPT_EXCLUSIVE flag and show error message if more than
> one of them is used.

Looks useful for me :)

Reviewed-by: Masami Hiramatsu <[email protected]>

I just have a comment below;

> @@ -360,19 +378,21 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
> }
>
> if (arg[1] != '-') {
> - ctx->opt = arg + 1;
> + ctx->opt = ++arg;
> if (internal_help && *ctx->opt == 'h')
> return usage_with_options_internal(usagestr, options, 0);
> switch (parse_short_opt(ctx, options)) {
> case -1:
> - return parse_options_usage(usagestr, options, arg + 1, 1);
> + return parse_options_usage(usagestr, options, arg, 1);
> case -2:
> goto unknown;
> + case -3:
> + goto exclusive;

BTW, it may be a time to define return error codes.

Thank you,


--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]

2014-10-23 20:29:53

by Arnaldo Carvalho de Melo

Subject: Re: [PATCH 4/5] perf tools: Add support for exclusive option

Em Thu, Oct 23, 2014 at 02:05:08PM +0900, Masami Hiramatsu escreveu:
> (2014/10/23 0:15), Namhyung Kim wrote:
> > Some options cannot be used at the same time. To handle such options
> > add a new PARSE_OPT_EXCLUSIVE flag and show error message if more than
> > one of them is used.
>
> Looks useful for me :)
>
> Reviewed-by: Masami Hiramatsu <[email protected]>
>
> I just have a comment below;
>
> > @@ -360,19 +378,21 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
> > }
> >
> > if (arg[1] != '-') {
> > - ctx->opt = arg + 1;
> > + ctx->opt = ++arg;
> > if (internal_help && *ctx->opt == 'h')
> > return usage_with_options_internal(usagestr, options, 0);
> > switch (parse_short_opt(ctx, options)) {
> > case -1:
> > - return parse_options_usage(usagestr, options, arg + 1, 1);
> > + return parse_options_usage(usagestr, options, arg, 1);
> > case -2:
> > goto unknown;
> > + case -3:
> > + goto exclusive;
>
> BTW, it may be a time to define return error codes.

Yeah, this thing looks unnecessarily complicated :-\

2014-10-23 20:33:30

by Arnaldo Carvalho de Melo

Subject: Re: [PATCHSET 0/5] perf tools: option parsing improvement

Em Thu, Oct 23, 2014 at 12:15:44AM +0900, Namhyung Kim escreveu:
> Hello,
>
> This patchset tries to enhance option parser a bit. Patch 1-3 are to
> reuse existing perf record options for other commands like perf kvm
> stat record. Patch 4-5 are to support exclusive options that cannot
> be used at the same time. The perf probe has such options and upcoming
> sdt-cache command also.
>
> You can get it from 'perf/option-share-v2' branch on my tree:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>
> Any comments are welcome, thanks
> Namhyung

Thanks, applied the series.

- Arnaldo

>
> Cc: Masami Hiramatsu <[email protected]>
> Cc: Hemant Kumar <[email protected]>
> Cc: Alexander Yarygin <[email protected]>
>
>
> Namhyung Kim (5):
> perf tools: Add PARSE_OPT_DISABLED flag
> perf tools: Export usage string and option table of perf record
> perf kvm: Print kvm specific --help output
> perf tools: Add support for exclusive option
> perf probe: Use PARSE_OPT_EXCLUSIVE flag
>
> tools/perf/builtin-kvm.c | 25 +++++++++++++
> tools/perf/builtin-probe.c | 54 +++++-----------------------
> tools/perf/builtin-record.c | 7 ++--
> tools/perf/builtin-script.c | 1 -
> tools/perf/builtin-timechart.c | 7 ++--
> tools/perf/perf.h | 3 ++
> tools/perf/util/parse-options.c | 78 ++++++++++++++++++++++++++++++++++-------
> tools/perf/util/parse-options.h | 4 +++
> 8 files changed, 116 insertions(+), 63 deletions(-)
>
> --
> 2.0.0

2014-10-24 00:40:09

by Namhyung Kim

Subject: Re: [PATCH 4/5] perf tools: Add support for exclusive option

Hi Arnaldo,

On Thu, 23 Oct 2014 17:29:48 -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Oct 23, 2014 at 02:05:08PM +0900, Masami Hiramatsu escreveu:
>> (2014/10/23 0:15), Namhyung Kim wrote:
>> > Some options cannot be used at the same time. To handle such options
>> > add a new PARSE_OPT_EXCLUSIVE flag and show error message if more than
>> > one of them is used.
>>
>> Looks useful for me :)
>>
>> Reviewed-by: Masami Hiramatsu <[email protected]>
>>
>> I just have a comment below;
>>
>> > @@ -360,19 +378,21 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
>> > }
>> >
>> > if (arg[1] != '-') {
>> > - ctx->opt = arg + 1;
>> > + ctx->opt = ++arg;
>> > if (internal_help && *ctx->opt == 'h')
>> > return usage_with_options_internal(usagestr, options, 0);
>> > switch (parse_short_opt(ctx, options)) {
>> > case -1:
>> > - return parse_options_usage(usagestr, options, arg + 1, 1);
>> > + return parse_options_usage(usagestr, options, arg, 1);
>> > case -2:
>> > goto unknown;
>> > + case -3:
>> > + goto exclusive;
>>
>> BTW, it may be a time to define return error codes.
>
> Yeah, this thing looks unnecessarily complicated :-\

OK, will do it as a separate patch later.

Thanks,
Namhyung

2014-10-30 06:44:51

by tip-bot for Vasyl Gomonovych

Subject: [tip:perf/core] perf tools: Add PARSE_OPT_DISABLED flag

Commit-ID: d152d1be5962ace0706066db71b4f05dff8764eb
Gitweb: http://git.kernel.org/tip/d152d1be5962ace0706066db71b4f05dff8764eb
Author: Namhyung Kim <[email protected]>
AuthorDate: Thu, 23 Oct 2014 00:15:45 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 29 Oct 2014 10:32:47 -0200

perf tools: Add PARSE_OPT_DISABLED flag

In some cases, we need to reuse exising options with some of them
disabled. To do that, add PARSE_OPT_DISABLED flag and
set_option_flag() function.

Signed-off-by: Namhyung Kim <[email protected]>
Acked-by: Hemant Kumar <[email protected]>
Cc: Alexander Yarygin <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Hemant Kumar <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/parse-options.c | 17 +++++++++++++++++
tools/perf/util/parse-options.h | 2 ++
2 files changed, 19 insertions(+)

diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c
index bf48092..b601610 100644
--- a/tools/perf/util/parse-options.c
+++ b/tools/perf/util/parse-options.c
@@ -42,6 +42,8 @@ static int get_value(struct parse_opt_ctx_t *p,
return opterror(opt, "takes no value", flags);
if (unset && (opt->flags & PARSE_OPT_NONEG))
return opterror(opt, "isn't available", flags);
+ if (opt->flags & PARSE_OPT_DISABLED)
+ return opterror(opt, "is not usable", flags);

if (!(flags & OPT_SHORT) && p->opt) {
switch (opt->type) {
@@ -509,6 +511,8 @@ static void print_option_help(const struct option *opts, int full)
}
if (!full && (opts->flags & PARSE_OPT_HIDDEN))
return;
+ if (opts->flags & PARSE_OPT_DISABLED)
+ return;

pos = fprintf(stderr, " ");
if (opts->short_name)
@@ -679,3 +683,16 @@ int parse_opt_verbosity_cb(const struct option *opt,
}
return 0;
}
+
+void set_option_flag(struct option *opts, int shortopt, const char *longopt,
+ int flag)
+{
+ for (; opts->type != OPTION_END; opts++) {
+ if ((shortopt && opts->short_name == shortopt) ||
+ (opts->long_name && longopt &&
+ !strcmp(opts->long_name, longopt))) {
+ opts->flags |= flag;
+ break;
+ }
+ }
+}
diff --git a/tools/perf/util/parse-options.h b/tools/perf/util/parse-options.h
index b59ba85..b7c80db 100644
--- a/tools/perf/util/parse-options.h
+++ b/tools/perf/util/parse-options.h
@@ -38,6 +38,7 @@ enum parse_opt_option_flags {
PARSE_OPT_NONEG = 4,
PARSE_OPT_HIDDEN = 8,
PARSE_OPT_LASTARG_DEFAULT = 16,
+ PARSE_OPT_DISABLED = 32,
};

struct option;
@@ -211,4 +212,5 @@ extern int parse_opt_verbosity_cb(const struct option *, const char *, int);

extern const char *parse_options_fix_filename(const char *prefix, const char *file);

+void set_option_flag(struct option *opts, int sopt, const char *lopt, int flag);
#endif /* __PERF_PARSE_OPTIONS_H */

2014-10-30 06:45:08

by tip-bot for Vasyl Gomonovych

Subject: [tip:perf/core] perf tools: Export usage string and option table of perf record

Commit-ID: e5b2c20755d37d781bb6e1e733faec5c39bd087a
Gitweb: http://git.kernel.org/tip/e5b2c20755d37d781bb6e1e733faec5c39bd087a
Author: Namhyung Kim <[email protected]>
AuthorDate: Thu, 23 Oct 2014 00:15:46 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 29 Oct 2014 10:32:47 -0200

perf tools: Export usage string and option table of perf record

Those are shared with other builtin commands like kvm, script. So
make it accessable from them. This is a preparation of later change
that limiting possible options.

Signed-off-by: Namhyung Kim <[email protected]>
Acked-by: Hemant Kumar <[email protected]>
Cc: Alexander Yarygin <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Hemant Kumar <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-record.c | 7 +++++--
tools/perf/builtin-script.c | 1 -
tools/perf/builtin-timechart.c | 7 ++++---
tools/perf/perf.h | 3 +++
4 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 2583a9b..5091a27 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -680,11 +680,12 @@ static int perf_record_config(const char *var, const char *value, void *cb)
return perf_default_config(var, value, cb);
}

-static const char * const record_usage[] = {
+static const char * const __record_usage[] = {
"perf record [<options>] [<command>]",
"perf record [<options>] -- <command> [<options>]",
NULL
};
+const char * const *record_usage = __record_usage;

/*
* XXX Ideally would be local to cmd_record() and passed to a record__new
@@ -725,7 +726,7 @@ const char record_callchain_help[] = CALLCHAIN_HELP "fp";
* perf_evlist__prepare_workload, etc instead of fork+exec'in 'perf record',
* using pipes, etc.
*/
-const struct option record_options[] = {
+struct option __record_options[] = {
OPT_CALLBACK('e', "event", &record.evlist, "event",
"event selector. use 'perf list' to list available events",
parse_events_option),
@@ -802,6 +803,8 @@ const struct option record_options[] = {
OPT_END()
};

+struct option *record_options = __record_options;
+
int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
{
int err = -ENOMEM;
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index b35517f..ce304df 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -23,7 +23,6 @@ static char const *generate_script_lang;
static bool debug_mode;
static u64 last_timestamp;
static u64 nr_unordered;
-extern const struct option record_options[];
static bool no_callchain;
static bool latency_format;
static bool system_wide;
diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index f5fb256..f3bb1a4 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -1963,7 +1963,7 @@ int cmd_timechart(int argc, const char **argv,
NULL
};

- const struct option record_options[] = {
+ const struct option timechart_record_options[] = {
OPT_BOOLEAN('P', "power-only", &tchart.power_only, "output power data only"),
OPT_BOOLEAN('T', "tasks-only", &tchart.tasks_only,
"output processes data only"),
@@ -1972,7 +1972,7 @@ int cmd_timechart(int argc, const char **argv,
OPT_BOOLEAN('g', "callchain", &tchart.with_backtrace, "record callchain"),
OPT_END()
};
- const char * const record_usage[] = {
+ const char * const timechart_record_usage[] = {
"perf timechart record [<options>]",
NULL
};
@@ -1985,7 +1985,8 @@ int cmd_timechart(int argc, const char **argv,
}

if (argc && !strncmp(argv[0], "rec", 3)) {
- argc = parse_options(argc, argv, record_options, record_usage,
+ argc = parse_options(argc, argv, timechart_record_options,
+ timechart_record_usage,
PARSE_OPT_STOP_AT_NON_OPTION);

if (tchart.power_only && tchart.tasks_only) {
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 220d44e..511c2831 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -62,4 +62,7 @@ struct record_opts {
unsigned initial_delay;
};

+struct option;
+extern const char * const *record_usage;
+extern struct option *record_options;
#endif

2014-10-30 06:45:22

by tip-bot for Vasyl Gomonovych

Subject: [tip:perf/core] perf kvm: Print kvm specific --help output

Commit-ID: f45d20ffb654f4559648da402b1608e747d46942
Gitweb: http://git.kernel.org/tip/f45d20ffb654f4559648da402b1608e747d46942
Author: Namhyung Kim <[email protected]>
AuthorDate: Thu, 23 Oct 2014 00:15:47 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 29 Oct 2014 10:32:47 -0200

perf kvm: Print kvm specific --help output

The 'perf kvm stat record' tool is an alias of 'perf record' with
predefined kvm related options. All options that passed to 'perf kvm
stat record' are processed by the 'perf record' tool. So, 'perf kvm
stat record --help' prints help of usage for the 'perf record'
command. There are a few options useful for 'perf kvm stat record',
the rest either break kvm related output or don't change it.

Let's print safe for 'perf kvm stat record' options in addition to
general 'perf record' --help output.

With this patch, new output looks like below:

$ perf kvm stat record -h

usage: perf kvm stat record [<options>]

-p, --pid <pid> record events on existing process id
-t, --tid <tid> record events on existing thread id
-r, --realtime <n> collect data with this RT SCHED_FIFO priority
--no-buffering collect data without buffering
-a, --all-cpus system-wide collection from all CPUs
-C, --cpu <cpu> list of cpus to monitor
-c, --count <n> event period to sample
-o, --output <file> output file name
-i, --no-inherit child tasks do not inherit counters
-m, --mmap-pages <pages>
number of mmap data pages
-v, --verbose be more verbose (show counter open errors, etc)
-q, --quiet don't print any message
-s, --stat per thread counts
-D, --delay <n> ms to wait before starting measurement after program start
-u, --uid <user> user to profile
--per-thread use per-thread mmaps

$ perf kvm stat record -n sleep 1
Error: switch `n' is not usable

usage: perf kvm stat record [<options>]

Signed-off-by: Namhyung Kim <[email protected]>
Acked-by: Hemant Kumar <[email protected]>
Cc: Alexander Yarygin <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-kvm.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index b65eb050..3c0f3d4 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -1132,6 +1132,10 @@ kvm_events_record(struct perf_kvm_stat *kvm, int argc, const char **argv)
"-m", "1024",
"-c", "1",
};
+ const char * const kvm_stat_record_usage[] = {
+ "perf kvm stat record [<options>]",
+ NULL
+ };
const char * const *events_tp;
events_tp_size = 0;

@@ -1159,6 +1163,27 @@ kvm_events_record(struct perf_kvm_stat *kvm, int argc, const char **argv)
for (j = 1; j < (unsigned int)argc; j++, i++)
rec_argv[i] = argv[j];

+ set_option_flag(record_options, 'e', "event", PARSE_OPT_HIDDEN);
+ set_option_flag(record_options, 0, "filter", PARSE_OPT_HIDDEN);
+ set_option_flag(record_options, 'R', "raw-samples", PARSE_OPT_HIDDEN);
+
+ set_option_flag(record_options, 'F', "freq", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 0, "group", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 'g', NULL, PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 0, "call-graph", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 'd', "data", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 'T', "timestamp", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 'P', "period", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 'n', "no-samples", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 'N', "no-buildid-cache", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 'B', "no-buildid", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 'G', "cgroup", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 'b', "branch-any", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 'j', "branch-filter", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 'W', "weight", PARSE_OPT_DISABLED);
+ set_option_flag(record_options, 0, "transaction", PARSE_OPT_DISABLED);
+
+ record_usage = kvm_stat_record_usage;
return cmd_record(i, rec_argv, NULL);
}

2014-10-30 06:45:42

by tip-bot for Vasyl Gomonovych

Subject: [tip:perf/core] perf tools: Add support for exclusive option

Commit-ID: 42bd71d0812ecd955cf65a14375ebe6a3195d979
Gitweb: http://git.kernel.org/tip/42bd71d0812ecd955cf65a14375ebe6a3195d979
Author: Namhyung Kim <[email protected]>
AuthorDate: Thu, 23 Oct 2014 00:15:48 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 29 Oct 2014 10:32:47 -0200

perf tools: Add support for exclusive option

Some options cannot be used at the same time. To handle such options
add a new PARSE_OPT_EXCLUSIVE flag and show error message if more than
one of them is used.

Signed-off-by: Namhyung Kim <[email protected]>
Reviewed-by: Masami Hiramatsu <[email protected]>
Acked-by: Hemant Kumar <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Hemant Kumar <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/parse-options.c | 59 +++++++++++++++++++++++++++++++++--------
tools/perf/util/parse-options.h | 2 ++
2 files changed, 50 insertions(+), 11 deletions(-)

diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c
index b601610..f62dee7 100644
--- a/tools/perf/util/parse-options.c
+++ b/tools/perf/util/parse-options.c
@@ -45,6 +45,23 @@ static int get_value(struct parse_opt_ctx_t *p,
if (opt->flags & PARSE_OPT_DISABLED)
return opterror(opt, "is not usable", flags);

+ if (opt->flags & PARSE_OPT_EXCLUSIVE) {
+ if (p->excl_opt) {
+ char msg[128];
+
+ if (((flags & OPT_SHORT) && p->excl_opt->short_name) ||
+ p->excl_opt->long_name == NULL) {
+ scnprintf(msg, sizeof(msg), "cannot be used with switch `%c'",
+ p->excl_opt->short_name);
+ } else {
+ scnprintf(msg, sizeof(msg), "cannot be used with %s",
+ p->excl_opt->long_name);
+ }
+ opterror(opt, msg, flags);
+ return -3;
+ }
+ p->excl_opt = opt;
+ }
if (!(flags & OPT_SHORT) && p->opt) {
switch (opt->type) {
case OPTION_CALLBACK:
@@ -345,13 +362,14 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
const char * const usagestr[])
{
int internal_help = !(ctx->flags & PARSE_OPT_NO_INTERNAL_HELP);
+ int excl_short_opt = 1;
+ const char *arg;

/* we must reset ->opt, unknown short option leave it dangling */
ctx->opt = NULL;

for (; ctx->argc; ctx->argc--, ctx->argv++) {
- const char *arg = ctx->argv[0];
-
+ arg = ctx->argv[0];
if (*arg != '-' || !arg[1]) {
if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
break;
@@ -360,19 +378,21 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
}

if (arg[1] != '-') {
- ctx->opt = arg + 1;
+ ctx->opt = ++arg;
if (internal_help && *ctx->opt == 'h')
return usage_with_options_internal(usagestr, options, 0);
switch (parse_short_opt(ctx, options)) {
case -1:
- return parse_options_usage(usagestr, options, arg + 1, 1);
+ return parse_options_usage(usagestr, options, arg, 1);
case -2:
goto unknown;
+ case -3:
+ goto exclusive;
default:
break;
}
if (ctx->opt)
- check_typos(arg + 1, options);
+ check_typos(arg, options);
while (ctx->opt) {
if (internal_help && *ctx->opt == 'h')
return usage_with_options_internal(usagestr, options, 0);
@@ -389,6 +409,8 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
ctx->argv[0] = strdup(ctx->opt - 1);
*(char *)ctx->argv[0] = '-';
goto unknown;
+ case -3:
+ goto exclusive;
default:
break;
}
@@ -404,19 +426,23 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
break;
}

- if (internal_help && !strcmp(arg + 2, "help-all"))
+ arg += 2;
+ if (internal_help && !strcmp(arg, "help-all"))
return usage_with_options_internal(usagestr, options, 1);
- if (internal_help && !strcmp(arg + 2, "help"))
+ if (internal_help && !strcmp(arg, "help"))
return usage_with_options_internal(usagestr, options, 0);
- if (!strcmp(arg + 2, "list-opts"))
+ if (!strcmp(arg, "list-opts"))
return PARSE_OPT_LIST_OPTS;
- if (!strcmp(arg + 2, "list-cmds"))
+ if (!strcmp(arg, "list-cmds"))
return PARSE_OPT_LIST_SUBCMDS;
- switch (parse_long_opt(ctx, arg + 2, options)) {
+ switch (parse_long_opt(ctx, arg, options)) {
case -1:
- return parse_options_usage(usagestr, options, arg + 2, 0);
+ return parse_options_usage(usagestr, options, arg, 0);
case -2:
goto unknown;
+ case -3:
+ excl_short_opt = 0;
+ goto exclusive;
default:
break;
}
@@ -428,6 +454,17 @@ unknown:
ctx->opt = NULL;
}
return PARSE_OPT_DONE;
+
+exclusive:
+ parse_options_usage(usagestr, options, arg, excl_short_opt);
+ if ((excl_short_opt && ctx->excl_opt->short_name) ||
+ ctx->excl_opt->long_name == NULL) {
+ char opt = ctx->excl_opt->short_name;
+ parse_options_usage(NULL, options, &opt, 1);
+ } else {
+ parse_options_usage(NULL, options, ctx->excl_opt->long_name, 0);
+ }
+ return PARSE_OPT_HELP;
}

int parse_options_end(struct parse_opt_ctx_t *ctx)
diff --git a/tools/perf/util/parse-options.h b/tools/perf/util/parse-options.h
index b7c80db..97b153f 100644
--- a/tools/perf/util/parse-options.h
+++ b/tools/perf/util/parse-options.h
@@ -39,6 +39,7 @@ enum parse_opt_option_flags {
PARSE_OPT_HIDDEN = 8,
PARSE_OPT_LASTARG_DEFAULT = 16,
PARSE_OPT_DISABLED = 32,
+ PARSE_OPT_EXCLUSIVE = 64,
};

struct option;
@@ -174,6 +175,7 @@ struct parse_opt_ctx_t {
const char **out;
int argc, cpidx;
const char *opt;
+ const struct option *excl_opt;
int flags;
};

2014-10-30 06:45:57

by tip-bot for Vasyl Gomonovych

Subject: [tip:perf/core] perf probe: Use PARSE_OPT_EXCLUSIVE flag

Commit-ID: 13dcbbc0222f9768394b0a58ab84adcd630f48d6
Gitweb: http://git.kernel.org/tip/13dcbbc0222f9768394b0a58ab84adcd630f48d6
Author: Namhyung Kim <[email protected]>
AuthorDate: Thu, 23 Oct 2014 00:15:49 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 29 Oct 2014 10:32:47 -0200

perf probe: Use PARSE_OPT_EXCLUSIVE flag

The perf probe command has some exclusive options. Use new PARSE_OPT_EXCLUSIVE
flag to simplify the code and show more compact usage.

$ perf probe -l -a foo
Error: switch `a' cannot be used with switch `l'

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'

-a, --add <[EVENT=]FUNC[@SRC][+OFF|%return|:RL|;PT]|SRC:AL|SRC;PT [[NAME=]ARG ...]>
probe point definition, where
GROUP: Group name (optional)
EVENT: Event name
FUNC: Function name
OFF: Offset from function entry (in byte)
%return: Put the probe at function return
SRC: Source code path
RL: Relative line number from function entry.
AL: Absolute line number in file.
PT: Lazy expression of line code.
ARG: Probe argument (local variable name or
kprobe-tracer argument format.)

-l, --list list up current probe events

Signed-off-by: Namhyung Kim <[email protected]>
Acked-by: Hemant Kumar <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Hemant Kumar <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-probe.c | 54 ++++++++--------------------------------------
1 file changed, 9 insertions(+), 45 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 7af26ac..2d3577d 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -312,7 +312,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
#endif
NULL
};
- const struct option options[] = {
+ struct option options[] = {
OPT_INCR('v', "verbose", &verbose,
"be more verbose (show parsed arguments, etc)"),
OPT_BOOLEAN('l', "list", &params.list_events,
@@ -382,6 +382,14 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
};
int ret;

+ set_option_flag(options, 'a', "add", PARSE_OPT_EXCLUSIVE);
+ set_option_flag(options, 'd', "del", PARSE_OPT_EXCLUSIVE);
+ set_option_flag(options, 'l', "list", PARSE_OPT_EXCLUSIVE);
+#ifdef HAVE_DWARF_SUPPORT
+ set_option_flag(options, 'L', "line", PARSE_OPT_EXCLUSIVE);
+ set_option_flag(options, 'V', "vars", PARSE_OPT_EXCLUSIVE);
+#endif
+
argc = parse_options(argc, argv, options, probe_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
if (argc > 0) {
@@ -409,22 +417,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);

if (params.list_events) {
- if (params.mod_events) {
- pr_err(" Error: Don't use --list with --add/--del.\n");
- usage_with_options(probe_usage, options);
- }
- if (params.show_lines) {
- pr_err(" Error: Don't use --list with --line.\n");
- usage_with_options(probe_usage, options);
- }
- if (params.show_vars) {
- pr_err(" Error: Don't use --list with --vars.\n");
- usage_with_options(probe_usage, options);
- }
- if (params.show_funcs) {
- pr_err(" Error: Don't use --list with --funcs.\n");
- usage_with_options(probe_usage, options);
- }
if (params.uprobes) {
pr_warning(" Error: Don't use --list with --exec.\n");
usage_with_options(probe_usage, options);
@@ -435,19 +427,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
return ret;
}
if (params.show_funcs) {
- if (params.nevents != 0 || params.dellist) {
- pr_err(" Error: Don't use --funcs with"
- " --add/--del.\n");
- usage_with_options(probe_usage, options);
- }
- if (params.show_lines) {
- pr_err(" Error: Don't use --funcs with --line.\n");
- usage_with_options(probe_usage, options);
- }
- if (params.show_vars) {
- pr_err(" Error: Don't use --funcs with --vars.\n");
- usage_with_options(probe_usage, options);
- }
if (!params.filter)
params.filter = strfilter__new(DEFAULT_FUNC_FILTER,
NULL);
@@ -462,16 +441,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)

#ifdef HAVE_DWARF_SUPPORT
if (params.show_lines) {
- if (params.mod_events) {
- pr_err(" Error: Don't use --line with"
- " --add/--del.\n");
- usage_with_options(probe_usage, options);
- }
- if (params.show_vars) {
- pr_err(" Error: Don't use --line with --vars.\n");
- usage_with_options(probe_usage, options);
- }
-
ret = show_line_range(&params.line_range, params.target,
params.uprobes);
if (ret < 0)
@@ -479,11 +448,6 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
return ret;
}
if (params.show_vars) {
- if (params.mod_events) {
- pr_err(" Error: Don't use --vars with"
- " --add/--del.\n");
- usage_with_options(probe_usage, options);
- }
if (!params.filter)
params.filter = strfilter__new(DEFAULT_VAR_FILTER,
NULL);