diff mbox

[2/2] perf record: Add --dry-run option to check cmdline options

Message ID 1466064161-48553-3-git-send-email-wangnan0@huawei.com
State Accepted
Commit 0aab21363ffa66d6e7340bc50cc5bfae865fd1a6
Headers show

Commit Message

Wang Nan June 16, 2016, 8:02 a.m. UTC
With '--dry-run', 'perf record' doesn't do reall recording. Combine with
llvm.dump-obj option, --dry-run can be used to help compile BPF objects for
embedded platform.

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

Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/Documentation/perf-record.txt | 7 +++++++
 tools/perf/builtin-record.c              | 7 +++++++
 2 files changed, 14 insertions(+)

-- 
1.8.3.4

Comments

Wang Nan June 20, 2016, 3:29 a.m. UTC | #1
On 2016/6/17 0:48, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jun 16, 2016 at 08:02:41AM +0000, Wang Nan escreveu:

>> With '--dry-run', 'perf record' doesn't do reall recording. Combine with

>> llvm.dump-obj option, --dry-run can be used to help compile BPF objects for

>> embedded platform.

> So these are nice and have value, but can we have a subcommand to do all

> this with an expressive name, Something like:

>

>    perf bpfcc foo.c -o foo

>

> or shorter:

>

>    perf bcc foo.c -o foo

>

> Just like one would use gcc or some other compiler to generate something

> for later use?


I'll try it today. I thought a subcommand require a bigger feature,
and wrapping clang is not big enough.

> That if called as:

>

>    perf bcc foo.c

>

> Would default to generating a foo.o file.

>

>    Then, later, one could use this as a event name, i.e.

>

>    trace --event foo

>

> Would, knowing that there is no event named "foo", look at the current

> directory (and in some other places perhaps) for a file named "foo" that

> was a bpf object file to use as it would a foo.c, shortcircuiting the

> bpf compilation code.

> If this was done instead:

>

>    trace --event foo.c

>

> And foo.c wasn't present, it would fallback to the behaviour described

> in the previous paragraph: look for a foo.o or foo bpf object file, etc.

>

> What do you think?


I'm not sure how many people can be benified from this feature. The only
advantage I can understand is we can skip the '.c', '.o' or '.bpf' suffix.

I guess what you really want is introducing something like buildid-cache for
BPF object. One can compile his/her BPF scriptlets into .o using 'perf 
bcc' and
insert it into cache, then he/her can use the resuling object without 
remembering
the path of it.

About fallback, if user explicitly uses '.o' or '.bpf' as suffix our 
parser can
be easier. Technically we need a boundary to split event name and 
configuration.
'.c', '.o' and '.bpf' are boundaries. In addition, is there any 
difference between
'-e mybpf' and '-e mybpf.bpf'? We can define that, when using '-e mybpf'
the search path whould be the BPF object cache, when using '-e 
mybpf.bpf' the
search path is current directory. It is acceptable, but why not make '-e 
mybpf.bpf'
search BPF object cache also?

Thank you.
Wang Nan June 21, 2016, 1:57 a.m. UTC | #2
On 2016/6/20 22:38, Arnaldo Carvalho de Melo wrote:
> Em Mon, Jun 20, 2016 at 11:29:13AM +0800, Wangnan (F) escreveu:

>> On 2016/6/17 0:48, Arnaldo Carvalho de Melo wrote:

>>> Em Thu, Jun 16, 2016 at 08:02:41AM +0000, Wang Nan escreveu:


[SNIP]

>> About fallback, if user explicitly uses '.o' or '.bpf' as suffix our

>> parser can be easier. Technically we need a boundary to split event

>> name and configuration.  '.c', '.o' and '.bpf' are boundaries. In

>> addition, is there any difference between '-e mybpf' and '-e

>> mybpf.bpf'? We can define that, when using '-e mybpf' the search path

>> whould be the BPF object cache, when using '-e mybpf.bpf' the search

>> path is current directory. It is acceptable, but why not make '-e

>> mybpf.bpf' search BPF object cache also?

> Well there is a namespace issue here, if we say:

>

> 	perf record -e cycles

>

> then this is well known, we want PERF_TYPE_HARDWARE,

> PERF_COUNT_HW_CPU_CYCLES. If we instead use:

>

> 	perf record -e cycles.c

>

> Then this also is well known, we need to build this somehow, and right

> now the only way to do this is to use the llvm/clang infrastructure and

> then load it into the kernel via sys_bpf.

>

> If we say:

>

> 	perf record -e cycles.bpf

>

> Then we don't have anything associated with this and may go on trying to

> map it to a PERF_TYPE_HARDWARE, PERF_TYPE_SOFTWARE, etc till we find a

> suitable event, i.e. if it doesn't match anything, we would end up

> looking at a file in the current directory, figure out it is an ELF file

> and that its contents are a BPF proggie, that we would load via sys_bpf,

> etc.


cycles.bpf is not a good example. See tools/perf/util/parse-events.l:
   ...
   bpf_object      .*\.(o|bpf)
   ...

currently '.o' equals to '.bpf'.

Thank you.
Wang Nan June 21, 2016, 6:12 a.m. UTC | #3
On 2016/6/21 0:22, Alexei Starovoitov wrote:
> On Mon, Jun 20, 2016 at 11:38:18AM -0300, Arnaldo Carvalho de Melo wrote:

>> Em Mon, Jun 20, 2016 at 11:29:13AM +0800, Wangnan (F) escreveu:

>>> On 2016/6/17 0:48, Arnaldo Carvalho de Melo wrote:

>>>> Em Thu, Jun 16, 2016 at 08:02:41AM +0000, Wang Nan escreveu:

>>>>> With '--dry-run', 'perf record' doesn't do reall recording. Combine with

>>>>> llvm.dump-obj option, --dry-run can be used to help compile BPF objects for

>>>>> embedded platform.

>>>> So these are nice and have value, but can we have a subcommand to do all

>>>> this with an expressive name, Something like:

>>>>    perf bpfcc foo.c -o foo

>>>> or shorter:

>>>>    perf bcc foo.c -o foo

>>>> Just like one would use gcc or some other compiler to generate something

>>>> for later use?

>>> I'll try it today. I thought a subcommand require a bigger feature,

>>> and wrapping clang is not big enough.

>> Not really, we may have as many as we like, given that they provide

>> something useful, like I think is the case here.

>>

>> Having to edit ~/.perfconfig, create a new section, a variable in it

>> with a boolean value (at first, just reading the changeset comment, I

>> thought I had to provide a directory where to store the objects

>> "dumped"), to then use a tool to record a .c event, but not recording

>> (use dry-run, which is useful to test the command line, etc), to then

>> get, on the current directory, the end result looked to me a convoluted

>> way to ask perf to compile the given .c file into a .o for later use.

>>

>> Doing:

>>

>> 	perf bcc -c foo.c

>>

>> Looks so much simpler and similar to an existing compile source code

>> into object file workflow (gcc's, any C compiler) that I think it would

>> fit in the workflow being discussed really nicely.

> I'm hopeful that eventually we'll be able merge iovisor/bcc project

> with perf, so would be good to reserve 'perf bcc' command for that

> future use. Also picking a different name for compiling would be less

> confusing to users who already familiar with bcc. Instead we can use:

> perf bpfcc foo.c -o foo.o

> perf cc foo.c

> perf compile foo.c

>


I think finally we should make perf independent with LLVM runtime.
I suggest 'perf bpf' subcommand to deal with all BPF related things,
include compiling, configuration and potential cache.

Thank you.
diff mbox

Patch

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 8dbee83..5b46b1d 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -360,6 +360,13 @@  particular perf.data snapshot should be kept or not.
 
 Implies --timestamp-filename, --no-buildid and --no-buildid-cache.
 
+--dry-run::
+Parse options then exit. --dry-run can be used to detect errors in cmdline
+options.
+
+'perf record --dry-run -e' can act as a BPF script compiler if llvm.dump-obj
+in config file is set to true.
+
 SEE ALSO
 --------
 linkperf:perf-stat[1], linkperf:perf-list[1]
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index d4cf1b0..b1304eb 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1274,6 +1274,8 @@  static struct record record = {
 const char record_callchain_help[] = CALLCHAIN_RECORD_HELP
 	"\n\t\t\t\tDefault: fp";
 
+static bool dry_run;
+
 /*
  * XXX Will stay a global variable till we fix builtin-script.c to stop messing
  * with it and switch to use the library functions in perf_evlist that came
@@ -1393,6 +1395,8 @@  struct option __record_options[] = {
 		    "append timestamp to output filename"),
 	OPT_BOOLEAN(0, "switch-output", &record.switch_output,
 		    "Switch output when receive SIGUSR2"),
+	OPT_BOOLEAN(0, "dry-run", &dry_run,
+		    "Parse options then exit"),
 	OPT_END()
 };
 
@@ -1462,6 +1466,9 @@  int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (err)
 		return err;
 
+	if (dry_run)
+		return 0;
+
 	err = bpf__setup_stdout(rec->evlist);
 	if (err) {
 		bpf__strerror_setup_stdout(rec->evlist, err, errbuf, sizeof(errbuf));