diff mbox

[1/2] perf tools: Always give options even it not compiled

Message ID 1447941793-118639-2-git-send-email-wangnan0@huawei.com
State New
Headers show

Commit Message

Wang Nan Nov. 19, 2015, 2:03 p.m. UTC 
This patch keeps options of perf builtins same in all condition. If the
option is disabled because of compiling options, users should be
notified.

This patch does it by introducing a series of new option macros, flags
and field in struct options. For those options disabled by compiling,
OPT_NOTBUILT_NOARG, OPT_NOTBUILT_OPTARG and OPT_NOTBUILT can be used
to record the help messages and the reason why not built them.

Options in 'perf record' and 'perf probe' are fixed by those new macros.

Test result:

- Normal result:
 # ./perf probe -L sys_write
 <SyS_write@/path/to/kernel/fs/read_write.c:0>
       0  SYSCALL_DEFINE3(write, unsigned int, fd, const char __user *, buf,
 ...

- Build with NO_DWARF=1:

 # ./perf probe -L sys_write
 ERROR: option '--line' is not built (disabled by NO_DWARF)

  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 [GROUP:]EVENT ...
     or: perf probe [<options>] --funcs

     -L, --line <FUNC[:RLN[+NUM|-RLN2]]|SRC:ALN[+NUM|-ALN2]>
                           Show source code lines.
                           (disabled by NO_DWARF)

 # ./perf probe -k /tmp/vmlinux sys_write
 WARNING: option '--vmlinux' is not built (disabled by NO_DWARF)
 Added new event:
   probe:sys_write      (on sys_write)

 You can now use it in all perf tools, such as:

 	perf record -e probe:sys_write -aR sleep 1

 # ./perf probe -k sys_write

  Usage: perf probe [<options>] 'PROBEDEF' ['PROBEDEF' ...]
     or: perf probe [<options>] --add 'PROBEDEF' [--add 'PROBEDEF' ...]
  ...

- Help messages:

 # ./perf probe -h

  Usage: perf probe [<options>] 'PROBEDEF' ['PROBEDEF' ...]
     or: perf probe [<options>] --add 'PROBEDEF' [--add 'PROBEDEF' ...]
  ...
     -k, --vmlinux <file>  vmlinux pathname
                           (disabled by NO_DWARF)
     -L, --line <FUNC[:RLN[+NUM|-RLN2]]|SRC:ALN[+NUM|-ALN2]>
                           Show source code lines.
                           (disabled by NO_DWARF)
     -l, --list <[GROUP:]EVENT>
                           list up probe events
  ...
     -s, --source <directory>
                           path to kernel source
                           (disabled by NO_DWARF)
     -V, --vars <FUNC[@SRC][+OFF|%return|:RL|;PT]|SRC:AL|SRC;PT>
                           Show accessible variables on PROBEDEF
                           (disabled by NO_DWARF)
  ...
         --no-inlines      Don't search inlined functions
                           (disabled by NO_DWARF)
         --range           Show variables location range in scope (with --vars only)
                           (disabled by NO_DWARF)

Signed-off-by: Wang Nan <wangnan0@huawei.com>

Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
---
 tools/perf/builtin-probe.c      | 23 +++++++++++++++++++++++
 tools/perf/builtin-record.c     |  7 +++++++
 tools/perf/util/parse-options.c | 30 +++++++++++++++++++++++++++++-
 tools/perf/util/parse-options.h | 16 ++++++++++++++++
 4 files changed, 75 insertions(+), 1 deletion(-)

-- 
1.8.3.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Comments

Wang Nan Nov. 26, 2015, 8:05 a.m. UTC | #1 
On 2015/11/20 18:54, 平松雅巳 / HIRAMATU，MASAMI wrote:
>> From: Wang Nan [mailto:wangnan0@huawei.com]

>>

>> This patch keeps options of perf builtins same in all condition. If the

>> option is disabled because of compiling options, users should be

>> notified.

>>

>> This patch does it by introducing a series of new option macros, flags

>> and field in struct options. For those options disabled by compiling,

>> OPT_NOTBUILT_NOARG, OPT_NOTBUILT_OPTARG and OPT_NOTBUILT can be used

>> to record the help messages and the reason why not built them.

>>

>> Options in 'perf record' and 'perf probe' are fixed by those new macros.

> Hmm, OK, I agree the reason why this is useful. Could you reconsider

> the implementation, because just cloning the code is ugly and not

> maintainable?

>

> It will be better if we can replace OPT_BOOLEAN with;

>

>     OPT_DEPENDS(HAVE_DWARF_SUPPORT, BOOLEAN, '\0', "no-inlines", &probe_conf.no_inlines, ...

>

> This may be done by following macros ;

>

> ----

> #define OPTMSG_HAVE_DWARF_SUPPORT "NO_DWARF"

> #ifdef HAVE_DWARF_SUPPORT

> #define OPTVAL_HAVE_DWARF_SUPPORT 1

> #else

> #define OPTVAL_HAVE_DWARF_SUPPORT 0

> #endif

>

> #define __OPT_DEPENDS(val, msg, opt, ...)	\

> 	{.type = OPTION_NEXT_DEPENDS, .value = (void *)val, .data = msg, }, opt(__VA_ARGS__)

>

> #define _OPT_DEPENDS(val, msg, opt, ...)	\

> 	__OPT_DEPENDS(val, msg, opt, __VA_ARGS__)

>

> #define OPT_DEPENDS(flag, opt, ...)	\

> 	_OPT_DEPENDS(OPTVAL_ ## flag, OPTMSG_ ## flag, OPT_ ## opt, __VA_ARGS__)

> ----

>

> And if the parser find OPTION_NEXT_DEPENDS and .value != NULL, it skips next entry.

>


That would be good, but don't forget we have a options__order() which
reorder the options array. To archive our goal I think we must preprocess
the options array to 'merge' information in the OPTION_NEXT_DEPENDS into
the real option it decorates.

Thank you.

> Thank you,

>



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Wang Nan Nov. 26, 2015, 9:06 a.m. UTC | #2 
On 2015/11/26 16:05, Wangnan (F) wrote:
>

>

> On 2015/11/20 18:54, 平松雅巳 / HIRAMATU，MASAMI wrote:

>>> From: Wang Nan [mailto:wangnan0@huawei.com]

>>>

>>> This patch keeps options of perf builtins same in all condition. If the

>>> option is disabled because of compiling options, users should be

>>> notified.

>>>

>>> This patch does it by introducing a series of new option macros, flags

>>> and field in struct options. For those options disabled by compiling,

>>> OPT_NOTBUILT_NOARG, OPT_NOTBUILT_OPTARG and OPT_NOTBUILT can be used

>>> to record the help messages and the reason why not built them.

>>>

>>> Options in 'perf record' and 'perf probe' are fixed by those new 

>>> macros.

>> Hmm, OK, I agree the reason why this is useful. Could you reconsider

>> the implementation, because just cloning the code is ugly and not

>> maintainable?

>>

>> It will be better if we can replace OPT_BOOLEAN with;

>>

>>     OPT_DEPENDS(HAVE_DWARF_SUPPORT, BOOLEAN, '\0', "no-inlines", 

>> &probe_conf.no_inlines, ...

>>

>> This may be done by following macros ;

>>

>> ----

>> #define OPTMSG_HAVE_DWARF_SUPPORT "NO_DWARF"

>> #ifdef HAVE_DWARF_SUPPORT

>> #define OPTVAL_HAVE_DWARF_SUPPORT 1

>> #else

>> #define OPTVAL_HAVE_DWARF_SUPPORT 0

>> #endif

>>

>> #define __OPT_DEPENDS(val, msg, opt, ...)    \

>>     {.type = OPTION_NEXT_DEPENDS, .value = (void *)val, .data = msg, 

>> }, opt(__VA_ARGS__)

>>

>> #define _OPT_DEPENDS(val, msg, opt, ...)    \

>>     __OPT_DEPENDS(val, msg, opt, __VA_ARGS__)

>>

>> #define OPT_DEPENDS(flag, opt, ...)    \

>>     _OPT_DEPENDS(OPTVAL_ ## flag, OPTMSG_ ## flag, OPT_ ## opt, 

>> __VA_ARGS__)

>> ----

>>

>> And if the parser find OPTION_NEXT_DEPENDS and .value != NULL, it 

>> skips next entry.

>>

>

> That would be good, but don't forget we have a options__order() which

> reorder the options array. To archive our goal I think we must preprocess

> the options array to 'merge' information in the OPTION_NEXT_DEPENDS into

> the real option it decorates.

>


Doing such merging in parse_options_subcommand() is natually but require 
removing all
'const' decorators from struct options. Too much code changing for such 
a small
feature...

Will think another solution...

> Thank you.

>

>> Thank you,

>>

>



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
diff mbox

Patch

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 132afc9..4d7cf57 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -490,6 +490,29 @@  __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 		   "directory", "path to kernel source"),
 	OPT_BOOLEAN('\0', "no-inlines", &probe_conf.no_inlines,
 		"Don't search inlined functions"),
+#else
+#define DISABLE_MSG	"disabled by NO_DWARF"
+	OPT_NOTBUILT('L', "line",
+		     "FUNC[:RLN[+NUM|-RLN2]]|SRC:ALN[+NUM|-ALN2]",
+		     "Show source code lines.",
+		     DISABLE_MSG, false),
+	OPT_NOTBUILT('V', "vars",
+		     "FUNC[@SRC][+OFF|%return|:RL|;PT]|SRC:AL|SRC;PT",
+		     "Show accessible variables on PROBEDEF",
+		     DISABLE_MSG, false),
+	OPT_NOTBUILT_NOARG('\0', "externs",
+			   "Show external variables too (with --vars only)",
+			   DISABLE_MSG, false),
+	OPT_NOTBUILT_NOARG('\0', "range",
+			   "Show variables location range in scope (with --vars only)",
+			   DISABLE_MSG, false),
+	OPT_NOTBUILT('k', "vmlinux", "file", "vmlinux pathname",
+		     DISABLE_MSG, true),
+	OPT_NOTBUILT('s', "source", "directory", "path to kernel source",
+		     DISABLE_MSG, true),
+	OPT_NOTBUILT_NOARG('\0', "no-inlines", "Don't search inlined functions",
+			   DISABLE_MSG, true),
+#undef DISABLE_MSG
 #endif
 	OPT__DRY_RUN(&probe_event_dry_run),
 	OPT_INTEGER('\0', "max-probes", &probe_conf.max_probes,
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 199fc31..b02d1ee 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1118,6 +1118,13 @@  struct option __record_options[] = {
 		   "clang binary to use for compiling BPF scriptlets"),
 	OPT_STRING(0, "clang-opt", &llvm_param.clang_opt, "clang options",
 		   "options passed to clang when compiling BPF scriptlets"),
+#else
+	OPT_NOTBUILT(0, "clang-path", "clang path",
+		     "clang binary to use for compiling BPF scriptlets",
+		     "disabled by NO_LIBBPF", true),
+	OPT_NOTBUILT(0, "clang-opt", "clang options",
+		     "options passed to clang when compiling BPF scriptlets",
+		     "disabled by NO_LIBBPF", true),
 #endif
 	OPT_END()
 };
diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c
index 9fca092..5c3eb7a 100644
--- a/tools/perf/util/parse-options.c
+++ b/tools/perf/util/parse-options.c
@@ -1,4 +1,5 @@ 
 #include "util.h"
+#include "debug.h"
 #include "parse-options.h"
 #include "cache.h"
 #include "header.h"
@@ -69,6 +70,7 @@  static int get_value(struct parse_opt_ctx_t *p,
 	if (!(flags & OPT_SHORT) && p->opt) {
 		switch (opt->type) {
 		case OPTION_CALLBACK:
+		case OPTION_NOTBUILT:
 			if (!(opt->flags & PARSE_OPT_NOARG))
 				break;
 			/* FALLTHROUGH */
@@ -152,7 +154,28 @@  static int get_value(struct parse_opt_ctx_t *p,
 		if (get_arg(p, opt, flags, &arg))
 			return -1;
 		return (*opt->callback)(opt, arg, 0) ? (-1) : 0;
-
+	case OPTION_NOTBUILT: {
+		char short_str[2] = {opt->short_name, '\0'};
+		int ret = -1;
+
+		pr_warning("%s: option '-%s%s' is not built (%s)\n",
+			   opt->flags & PARSE_OPT_CANSKIP ?
+				"WARNING" : "ERROR",
+			   opt->long_name ? "-" : short_str,
+			   opt->long_name ? : "",
+			   opt->notbuild_reason);
+		if (opt->flags & PARSE_OPT_CANSKIP)
+			ret = 0;
+		if (unset)
+			return ret;
+		if (opt->flags & PARSE_OPT_NOARG)
+			return ret;
+		if (opt->flags & PARSE_OPT_OPTARG && !p->opt)
+			return ret;
+		if (get_arg(p, opt, flags, &arg))
+			return -1;
+		return ret;
+	}
 	case OPTION_INTEGER:
 		if (unset) {
 			*(int *)opt->value = 0;
@@ -607,6 +630,7 @@  static void print_option_help(const struct option *opts, int full)
 			pos += fprintf(stderr, " <n>");
 		break;
 	case OPTION_CALLBACK:
+	case OPTION_NOTBUILT:
 		if (opts->flags & PARSE_OPT_NOARG)
 			break;
 		/* FALLTHROUGH */
@@ -647,6 +671,10 @@  static void print_option_help(const struct option *opts, int full)
 		pad = USAGE_OPTS_WIDTH;
 	}
 	fprintf(stderr, "%*s%s\n", pad + USAGE_GAP, "", opts->help);
+	if (opts->type == OPTION_NOTBUILT)
+		fprintf(stderr, "%*s(%s)\n",
+			USAGE_OPTS_WIDTH + USAGE_GAP, "",
+			opts->notbuild_reason);
 }
 
 static int option__cmp(const void *va, const void *vb)
diff --git a/tools/perf/util/parse-options.h b/tools/perf/util/parse-options.h
index a8e407b..08cc9b5 100644
--- a/tools/perf/util/parse-options.h
+++ b/tools/perf/util/parse-options.h
@@ -22,6 +22,7 @@  enum parse_opt_type {
 	OPTION_CALLBACK,
 	OPTION_U64,
 	OPTION_UINTEGER,
+	OPTION_NOTBUILT,
 };
 
 enum parse_opt_flags {
@@ -41,6 +42,7 @@  enum parse_opt_option_flags {
 	PARSE_OPT_DISABLED = 32,
 	PARSE_OPT_EXCLUSIVE = 64,
 	PARSE_OPT_NOEMPTY  = 128,
+	PARSE_OPT_CANSKIP  = 256,
 };
 
 struct option;
@@ -96,6 +98,7 @@  struct option {
 	void *value;
 	const char *argh;
 	const char *help;
+	const char *notbuild_reason;
 
 	int flags;
 	parse_opt_cb *callback;
@@ -146,6 +149,19 @@  struct option {
 	  .value = (v), (a), .help = (h), .callback = (f), \
 	  .flags = PARSE_OPT_OPTARG, .data = (d) }
 
+#define OPT_NOTBUILT_NOARG(s, l, h, r, skip) \
+	{ .type = OPTION_NOTBUILT, .short_name = (s), .long_name = (l), \
+	  .help = (h), .notbuild_reason = (r), \
+	 .flags = PARSE_OPT_NOARG | (skip ? PARSE_OPT_CANSKIP : 0)}
+#define OPT_NOTBUILT_OPTARG(s, l, a, h, r, skip) \
+	{ .type = OPTION_NOTBUILT, .short_name = (s), .long_name = (l), \
+	  .argh = (a), .help = (h), .notbuild_reason = (r), \
+	  .flags = PARSE_OPT_OPTARG | (skip ? PARSE_OPT_CANSKIP : 0)}
+#define OPT_NOTBUILT(s, l, a, h, r, skip) \
+	{ .type = OPTION_NOTBUILT, .short_name = (s), .long_name = (l), \
+	  .argh = (a), .help = (h), .notbuild_reason = (r), \
+	  .flags = skip ? PARSE_OPT_CANSKIP : 0}
+
 /* parse_options() will filter out the processed options and leave the
  * non-option argments in argv[].
  * Returns the number of arguments left in argv[].