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

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

[Namhyung Kim]

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 <masami.hiramatsu.pt@hitachi.com>
Cc: Hemant Kumar <hemant@linux.vnet.ibm.com>
Cc: Alexander Yarygin <yarygin@linux.vnet.ibm.com>


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

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

[Namhyung Kim]

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 <masami.hiramatsu.pt@hitachi.com>
Cc: Hemant Kumar <hemant@linux.vnet.ibm.com>
Cc: Alexander Yarygin <yarygin@linux.vnet.ibm.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 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

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

[tip-bot for Namhyung Kim]

Commit-ID:  d152d1be5962ace0706066db71b4f05dff8764eb
Gitweb:     http://git.kernel.org/tip/d152d1be5962ace0706066db71b4f05dff8764eb
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 23 Oct 2014 00:15:45 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <namhyung@kernel.org>
Acked-by: Hemant Kumar <hemant@linux.vnet.ibm.com>
Cc: Alexander Yarygin <yarygin@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Hemant Kumar <hemant@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1413990949-13953-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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 */

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

[Namhyung Kim]

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 <masami.hiramatsu.pt@hitachi.com>
Cc: Hemant Kumar <hemant@linux.vnet.ibm.com>
Cc: Alexander Yarygin <yarygin@linux.vnet.ibm.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 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

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

[tip-bot for Namhyung Kim]

Commit-ID:  e5b2c20755d37d781bb6e1e733faec5c39bd087a
Gitweb:     http://git.kernel.org/tip/e5b2c20755d37d781bb6e1e733faec5c39bd087a
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 23 Oct 2014 00:15:46 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <namhyung@kernel.org>
Acked-by: Hemant Kumar <hemant@linux.vnet.ibm.com>
Cc: Alexander Yarygin <yarygin@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Hemant Kumar <hemant@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1413990949-13953-3-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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

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

[Namhyung Kim]

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 <yarygin@linux.vnet.ibm.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 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

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

[tip-bot for Namhyung Kim]

Commit-ID:  f45d20ffb654f4559648da402b1608e747d46942
Gitweb:     http://git.kernel.org/tip/f45d20ffb654f4559648da402b1608e747d46942
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 23 Oct 2014 00:15:47 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <namhyung@kernel.org>
Acked-by: Hemant Kumar <hemant@linux.vnet.ibm.com>
Cc: Alexander Yarygin <yarygin@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1413990949-13953-4-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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);
 }
 

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

[Namhyung Kim]

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 <masami.hiramatsu.pt@hitachi.com>
Cc: Hemant Kumar <hemant@linux.vnet.ibm.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 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

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

[Masami Hiramatsu]

(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 <masami.hiramatsu.pt@hitachi.com>

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: masami.hiramatsu.pt@hitachi.com

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

[Arnaldo Carvalho de Melo]

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 <masami.hiramatsu.pt@hitachi.com>
> 
> 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 :-\

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

[Namhyung Kim]

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 <masami.hiramatsu.pt@hitachi.com>
>> 
>> 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

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

[tip-bot for Namhyung Kim]

Commit-ID:  42bd71d0812ecd955cf65a14375ebe6a3195d979
Gitweb:     http://git.kernel.org/tip/42bd71d0812ecd955cf65a14375ebe6a3195d979
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 23 Oct 2014 00:15:48 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <namhyung@kernel.org>
Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Acked-by: Hemant Kumar <hemant@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Hemant Kumar <hemant@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1413990949-13953-5-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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;
 };
 

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

[Namhyung Kim]

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 <masami.hiramatsu.pt@hitachi.com>
Cc: Hemant Kumar <hemant@linux.vnet.ibm.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 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

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

[Masami Hiramatsu]

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 <masami.hiramatsu.pt@hitachi.com>

> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Hemant Kumar <hemant@linux.vnet.ibm.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  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: masami.hiramatsu.pt@hitachi.com

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

[tip-bot for Namhyung Kim]

Commit-ID:  13dcbbc0222f9768394b0a58ab84adcd630f48d6
Gitweb:     http://git.kernel.org/tip/13dcbbc0222f9768394b0a58ab84adcd630f48d6
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 23 Oct 2014 00:15:49 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <namhyung@kernel.org>
Acked-by: Hemant Kumar <hemant@linux.vnet.ibm.com>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Hemant Kumar <hemant@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1413990949-13953-6-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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);

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

[Hemant Kumar]

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

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

[Arnaldo Carvalho de Melo]

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 <masami.hiramatsu.pt@hitachi.com>
> Cc: Hemant Kumar <hemant@linux.vnet.ibm.com>
> Cc: Alexander Yarygin <yarygin@linux.vnet.ibm.com>
> 
> 
> 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