[PATCH] perf, tools, script: Allow adding and removing fields

[PATCH] perf, tools, script: Allow adding and removing fields

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

With perf script it is common that we just want to add or remove a field.
Currently this requires figuring out the long list of default fields and
specifying them first, and then adding/removing the new field.

This patch adds a new + - syntax to merely add or remove fields,
that allows more succint and clearer command lines

For example to remove the comm field from PMU samples:

Previously

perf script -F tid,cpu,time,event,sym,ip,dso,period
	    0 [000] 504345.383126:          1 cycles:  ffffffff90060c66 native_write_msr ([kernel.kallsyms])

with the new syntax

perf script -F -comm
	    0 [000] 504345.383126:          1 cycles:  ffffffff90060c66 native_write_msr ([kernel.kallsyms])

The new syntax cannot be mixed with normal overriding.

v2: Fix example in description. Use tid vs pid. No functional
changes.
v3: Don't skip initialization when user specified explicit type.
v4: Rebase. Remove empty line.
Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/Documentation/perf-script.txt |  8 +++++++
 tools/perf/builtin-script.c              | 37 +++++++++++++++++++++++++++++---
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
index cb0eda3925e6..4547120c6ad3 100644
--- a/tools/perf/Documentation/perf-script.txt
+++ b/tools/perf/Documentation/perf-script.txt
@@ -130,6 +130,14 @@ OPTIONS
 	i.e., the specified fields apply to all event types if the type string
 	is not given.
 
+	In addition to overriding fields, it is also possible to add or remove
+	fields from the defaults. For example
+
+		-F -cpu,+insn
+
+	removes the cpu field and adds the insn field. Adding/removing fields
+	cannot be mixed with normal overriding.
+
 	The arguments are processed in the order received. A later usage can
 	reset a prior request. e.g.:
 
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index d05aec491cff..85c83cc3e994 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1727,6 +1727,7 @@ static int parse_output_fields(const struct option *opt __maybe_unused,
 	int rc = 0;
 	char *str = strdup(arg);
 	int type = -1;
+	enum { DEFAULT, SET, ADD, REMOVE } change = DEFAULT;
 
 	if (!str)
 		return -ENOMEM;
@@ -1772,6 +1773,10 @@ static int parse_output_fields(const struct option *opt __maybe_unused,
 			goto out;
 		}
 
+		/* Don't override defaults for +- */
+		if (strchr(str, '+') || strchr(str, '-'))
+			goto parse;
+
 		if (output_set_by_user())
 			pr_warning("Overriding previous field request for all events.\n");
 
@@ -1782,13 +1787,30 @@ static int parse_output_fields(const struct option *opt __maybe_unused,
 		}
 	}
 
+parse:
 	for (tok = strtok_r(tok, ",", &strtok_saveptr); tok; tok = strtok_r(NULL, ",", &strtok_saveptr)) {
+		if (*tok == '+') {
+			if (change == SET)
+				goto out_badmix;
+			change = ADD;
+			tok++;
+		} else if (*tok == '-') {
+			if (change == SET)
+				goto out_badmix;
+			change = REMOVE;
+			tok++;
+		} else {
+			if (change != SET && change != DEFAULT)
+				goto out_badmix;
+			change = SET;
+		}
+
 		for (i = 0; i < imax; ++i) {
 			if (strcmp(tok, all_output_options[i].str) == 0)
 				break;
 		}
 		if (i == imax && strcmp(tok, "flags") == 0) {
-			print_flags = true;
+			print_flags = change == REMOVE ? false : true;
 			continue;
 		}
 		if (i == imax) {
@@ -1805,8 +1827,12 @@ static int parse_output_fields(const struct option *opt __maybe_unused,
 				if (output[j].invalid_fields & all_output_options[i].field) {
 					pr_warning("\'%s\' not valid for %s events. Ignoring.\n",
 						   all_output_options[i].str, event_type(j));
-				} else
-					output[j].fields |= all_output_options[i].field;
+				} else {
+					if (change == REMOVE)
+						output[j].fields &= ~all_output_options[i].field;
+					else
+						output[j].fields |= all_output_options[i].field;
+				}
 			}
 		} else {
 			if (output[type].invalid_fields & all_output_options[i].field) {
@@ -1826,7 +1852,11 @@ static int parse_output_fields(const struct option *opt __maybe_unused,
 				 "Events will not be displayed.\n", event_type(type));
 		}
 	}
+	goto out;
 
+out_badmix:
+	fprintf(stderr, "Cannot mix +-field with overridden fields\n");
+	rc = -EINVAL;
 out:
 	free(str);
 	return rc;
@@ -2444,6 +2474,7 @@ int cmd_script(int argc, const char **argv)
 		     symbol__config_symfs),
 	OPT_CALLBACK('F', "fields", NULL, "str",
 		     "comma separated output fields prepend with 'type:'. "
+		     "+field to add and -field to remove."
 		     "Valid types: hw,sw,trace,raw. "
 		     "Fields: comm,tid,pid,time,cpu,event,trace,ip,sym,dso,"
 		     "addr,symoff,period,iregs,brstack,brstacksym,flags,"
-- 
2.9.4

Re: [PATCH] perf, tools, script: Allow adding and removing fields

[Milian Wolff]

[-- Attachment #1: Type: text/plain, Size: 2410 bytes --]

On Friday, June 2, 2017 5:48:10 PM CEST Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> With perf script it is common that we just want to add or remove a field.
> Currently this requires figuring out the long list of default fields and
> specifying them first, and then adding/removing the new field.
> 
> This patch adds a new + - syntax to merely add or remove fields,
> that allows more succint and clearer command lines
> 
> For example to remove the comm field from PMU samples:
> 
> Previously
> 
> perf script -F tid,cpu,time,event,sym,ip,dso,period
> 	    0 [000] 504345.383126:          1 cycles:  ffffffff90060c66
> native_write_msr ([kernel.kallsyms])
> 
> with the new syntax
> 
> perf script -F -comm
> 	    0 [000] 504345.383126:          1 cycles:  ffffffff90060c66
> native_write_msr ([kernel.kallsyms])
> 
> The new syntax cannot be mixed with normal overriding.

Tested-by: Milian Wolff <milian.wolff@kdab.com>

Works a charm.

But I notice that this functionality is missing in other places too. Most 
notably, I would like to be able to configure `perf stat` in a similar way. 
Such that one could do:

perf stat -e +cache-misses

Instead of

perf stat -e <whatever the defaults are>,cache-misses

While at it, I also think it would be tremendously useful to get wildcard 
matching behavior in all perf tools. Currently, I can do:

perf list "topdown-*"

List of pre-defined events (to be used in -e):

  topdown-fetch-bubbles OR cpu/topdown-fetch-bubbles/ [Kernel PMU event]
  topdown-recovery-bubbles OR cpu/topdown-recovery-bubbles/ [Kernel PMU event]
  topdown-slots-issued OR cpu/topdown-slots-issued/  [Kernel PMU event]
  topdown-slots-retired OR cpu/topdown-slots-retired/ [Kernel PMU event]
  topdown-total-slots OR cpu/topdown-total-slots/    [Kernel PMU event]

But I cannot do:

$ perf record -e "topdown-*" ls
event syntax error: 'topdown-*'
                            \___ parser error
Run 'perf list' for a list of valid events

But of course I'm aware that these are all separate issues. But Andi, you seem 
to be working a lot on polishing perf. I'd really appreciate it if you could 
also implement the above two suggestions. Otherwise I'll try to see when I get 
the time to do it myself.

Cheers

-- 
Milian Wolff | milian.wolff@kdab.com | Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3826 bytes --]

Re: [PATCH] perf, tools, script: Allow adding and removing fields

[Arnaldo Carvalho de Melo]

Em Fri, Jun 02, 2017 at 08:48:10AM -0700, Andi Kleen escreveu:
> From: Andi Kleen <ak@linux.intel.com>
> 
> With perf script it is common that we just want to add or remove a field.
> Currently this requires figuring out the long list of default fields and
> specifying them first, and then adding/removing the new field.
> 
> This patch adds a new + - syntax to merely add or remove fields,
> that allows more succint and clearer command lines
> 
> For example to remove the comm field from PMU samples:
> 
> Previously
> 
> perf script -F tid,cpu,time,event,sym,ip,dso,period
> 	    0 [000] 504345.383126:          1 cycles:  ffffffff90060c66 native_write_msr ([kernel.kallsyms])
> 
> with the new syntax
> 
> perf script -F -comm
> 	    0 [000] 504345.383126:          1 cycles:  ffffffff90060c66 native_write_msr ([kernel.kallsyms])

Humm, its a nice feature, but the output used as an example doesn't show
any diff, cut'n'paste error?

Anyway, I tested it and will update that example with this:

---------------

  # perf record -a usleep 1
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 1.748 MB perf.data (14 samples) ]

Without a explicit field list specified via -F, defaults to:

  # perf script | head -2
              perf  6338 [000] 18467.058607:          1 cycles:  ffffffff89060c36 native_write_msr (/lib/modules/4.11.0-rc8+/build/vmlinux)
           swapper     0 [001] 18467.058617:          1 cycles:  ffffffff89060c36 native_write_msr (/lib/modules/4.11.0-rc8+/build/vmlinux)

Which is equivalent to:

  # perf script -F comm,tid,cpu,time,period,event,ip,sym,dso | head -2
              perf  6338 [000] 18467.058607:          1 cycles:  ffffffff89060c36 native_write_msr (/lib/modules/4.11.0-rc8+/build/vmlinux)
           swapper     0 [001] 18467.058617:          1 cycles:  ffffffff89060c36 native_write_msr (/lib/modules/4.11.0-rc8+/build/vmlinux)
  #

So if we want to remove the comm, as in your original example, we would have to 
figure out the default field list and remove ' comm' from it:

  # perf script -F tid,cpu,time,period,event,ip,sym,dso | head -2
   6338 [000] 18467.058607:          1 cycles:  ffffffff89060c36 native_write_msr (/lib/modules/4.11.0-rc8+/build/vmlinux)
      0 [001] 18467.058617:          1 cycles:  ffffffff89060c36 native_write_msr (/lib/modules/4.11.0-rc8+/build/vmlinux)
  # 

With your patch this becomes simpler, one can remove fields by prefixing them
with '-':

  # perf script -F -comm | head -2
  6338 [000] 18467.058607:          1 cycles:  ffffffff89060c36 native_write_msr (/lib/modules/4.11.0-rc8+/build/vmlinux)
     0 [001] 18467.058617:          1 cycles:  ffffffff89060c36 native_write_msr (/lib/modules/4.11.0-rc8+/build/vmlinux)
  # 

-------------------

BTW, the '+' syntax is present in 'perf report -F', but we lack handling
'-', and it works for the whole list, i.e. it is not possible to add
some fields and remove others, would be good to look at both and
consolidate at some time, using your syntax, which is more powerful.

- Arnaldo
 
> The new syntax cannot be mixed with normal overriding.
> 
> v2: Fix example in description. Use tid vs pid. No functional
> changes.
> v3: Don't skip initialization when user specified explicit type.
> v4: Rebase. Remove empty line.
> Acked-by: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  tools/perf/Documentation/perf-script.txt |  8 +++++++
>  tools/perf/builtin-script.c              | 37 +++++++++++++++++++++++++++++---
>  2 files changed, 42 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
> index cb0eda3925e6..4547120c6ad3 100644
> --- a/tools/perf/Documentation/perf-script.txt
> +++ b/tools/perf/Documentation/perf-script.txt
> @@ -130,6 +130,14 @@ OPTIONS
>  	i.e., the specified fields apply to all event types if the type string
>  	is not given.
>  
> +	In addition to overriding fields, it is also possible to add or remove
> +	fields from the defaults. For example
> +
> +		-F -cpu,+insn
> +
> +	removes the cpu field and adds the insn field. Adding/removing fields
> +	cannot be mixed with normal overriding.
> +
>  	The arguments are processed in the order received. A later usage can
>  	reset a prior request. e.g.:
>  
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index d05aec491cff..85c83cc3e994 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -1727,6 +1727,7 @@ static int parse_output_fields(const struct option *opt __maybe_unused,
>  	int rc = 0;
>  	char *str = strdup(arg);
>  	int type = -1;
> +	enum { DEFAULT, SET, ADD, REMOVE } change = DEFAULT;
>  
>  	if (!str)
>  		return -ENOMEM;
> @@ -1772,6 +1773,10 @@ static int parse_output_fields(const struct option *opt __maybe_unused,
>  			goto out;
>  		}
>  
> +		/* Don't override defaults for +- */
> +		if (strchr(str, '+') || strchr(str, '-'))
> +			goto parse;
> +
>  		if (output_set_by_user())
>  			pr_warning("Overriding previous field request for all events.\n");
>  
> @@ -1782,13 +1787,30 @@ static int parse_output_fields(const struct option *opt __maybe_unused,
>  		}
>  	}
>  
> +parse:
>  	for (tok = strtok_r(tok, ",", &strtok_saveptr); tok; tok = strtok_r(NULL, ",", &strtok_saveptr)) {
> +		if (*tok == '+') {
> +			if (change == SET)
> +				goto out_badmix;
> +			change = ADD;
> +			tok++;
> +		} else if (*tok == '-') {
> +			if (change == SET)
> +				goto out_badmix;
> +			change = REMOVE;
> +			tok++;
> +		} else {
> +			if (change != SET && change != DEFAULT)
> +				goto out_badmix;
> +			change = SET;
> +		}
> +
>  		for (i = 0; i < imax; ++i) {
>  			if (strcmp(tok, all_output_options[i].str) == 0)
>  				break;
>  		}
>  		if (i == imax && strcmp(tok, "flags") == 0) {
> -			print_flags = true;
> +			print_flags = change == REMOVE ? false : true;
>  			continue;
>  		}
>  		if (i == imax) {
> @@ -1805,8 +1827,12 @@ static int parse_output_fields(const struct option *opt __maybe_unused,
>  				if (output[j].invalid_fields & all_output_options[i].field) {
>  					pr_warning("\'%s\' not valid for %s events. Ignoring.\n",
>  						   all_output_options[i].str, event_type(j));
> -				} else
> -					output[j].fields |= all_output_options[i].field;
> +				} else {
> +					if (change == REMOVE)
> +						output[j].fields &= ~all_output_options[i].field;
> +					else
> +						output[j].fields |= all_output_options[i].field;
> +				}
>  			}
>  		} else {
>  			if (output[type].invalid_fields & all_output_options[i].field) {
> @@ -1826,7 +1852,11 @@ static int parse_output_fields(const struct option *opt __maybe_unused,
>  				 "Events will not be displayed.\n", event_type(type));
>  		}
>  	}
> +	goto out;
>  
> +out_badmix:
> +	fprintf(stderr, "Cannot mix +-field with overridden fields\n");
> +	rc = -EINVAL;
>  out:
>  	free(str);
>  	return rc;
> @@ -2444,6 +2474,7 @@ int cmd_script(int argc, const char **argv)
>  		     symbol__config_symfs),
>  	OPT_CALLBACK('F', "fields", NULL, "str",
>  		     "comma separated output fields prepend with 'type:'. "
> +		     "+field to add and -field to remove."
>  		     "Valid types: hw,sw,trace,raw. "
>  		     "Fields: comm,tid,pid,time,cpu,event,trace,ip,sym,dso,"
>  		     "addr,symoff,period,iregs,brstack,brstacksym,flags,"
> -- 
> 2.9.4

Re: [PATCH] perf, tools, script: Allow adding and removing fields

[Andi Kleen]

On Thu, Jun 08, 2017 at 02:59:58PM +0200, Milian Wolff wrote:
> But I notice that this functionality is missing in other places too. Most 
> notably, I would like to be able to configure `perf stat` in a similar way. 
> Such that one could do:
> 
> perf stat -e +cache-misses
> 
> Instead of
> 
> perf stat -e <whatever the defaults are>,cache-misses

The defaults are not great, so I'm not sure that is super useful.

It's probably better to assemble reasonable groups, perhaps
with groups of metrics.

> But I cannot do:
> 
> $ perf record -e "topdown-*" ls
> event syntax error: 'topdown-*'

That's actually good because the current topdown events are not useful to sample

Usually you need to have at least some idea about the events you're collecting,
and also for non trivial collections you need groups to get good results.

I've been thinking about adding MetricGroups to the json files, that
would allow to assemble reasonable groups. But it still wouldn't be wildcard.

For a few things wildcards are useful, e.g. I implemented it recently
for PMUs so that uncore PMUs are easier to handle.

-Andi

Re: [PATCH] perf, tools, script: Allow adding and removing fields

[Milian Wolff]

On Freitag, 9. Juni 2017 04:52:43 CEST Andi Kleen wrote:
> On Thu, Jun 08, 2017 at 02:59:58PM +0200, Milian Wolff wrote:
> > But I notice that this functionality is missing in other places too. Most
> > notably, I would like to be able to configure `perf stat` in a similar
> > way.
> > Such that one could do:
> > 
> > perf stat -e +cache-misses
> > 
> > Instead of
> > 
> > perf stat -e <whatever the defaults are>,cache-misses
> 
> The defaults are not great, so I'm not sure that is super useful.
> 
> It's probably better to assemble reasonable groups, perhaps
> with groups of metrics.
> 
> > But I cannot do:
> > 
> > $ perf record -e "topdown-*" ls
> > event syntax error: 'topdown-*'
> 
> That's actually good because the current topdown events are not useful to
> sample

Can you elaborate? I assume it's because you actually want to sample on 
instructions, and then group it together with the topdown events and 
potentially other counters like instructions?

> Usually you need to have at least some idea about the events you're
> collecting, and also for non trivial collections you need groups to get
> good results.

Yes, sure. But replace `record` with `stat` in the above and my point still 
stands.

> I've been thinking about adding MetricGroups to the json files, that
> would allow to assemble reasonable groups. But it still wouldn't be
> wildcard.
> 
> For a few things wildcards are useful, e.g. I implemented it recently
> for PMUs so that uncore PMUs are easier to handle.

I just noticed that I can actually use wildcards for tracepoints:

perf trace --no-syscalls --event "ext4:*"

And I think the same should be doable for PMU events with perf stat, but 
currently isn't:

$ perf stat -e "topdown*" ls
invalid or unsupported event: 'topdown*'
$ perf stat -e "branch*" ls
invalid or unsupported event: 'branch*'
$ perf stat -e "cache*" ls
invalid or unsupported event: 'cache*'

Bye

-- 
Milian Wolff | milian.wolff@kdab.com | Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts

Re: [PATCH] perf, tools, script: Allow adding and removing fields

[Andi Kleen]

On Fri, Jun 09, 2017 at 11:13:11AM +0200, Milian Wolff wrote:
> > > But I cannot do:
> > > 
> > > $ perf record -e "topdown-*" ls
> > > event syntax error: 'topdown-*'
> > 
> > That's actually good because the current topdown events are not useful to
> > sample
> 
> Can you elaborate? I assume it's because you actually want to sample on 
> instructions, and then group it together with the topdown events and 
> potentially other counters like instructions?

The topdown-* events are inputs to a formula. But you cannot directly
sample for the formula.

What you can do is to compute the formuals from counts, determine 
the bottlenecks and then sample for events which look for the
bottlebeck. For example FRONTEND_* for Frontend Bound.
These events are generally different.

toplev in pmu-tools implements this automatically, but it's a bit
too complicated for standard perf.

> 
> > Usually you need to have at least some idea about the events you're
> > collecting, and also for non trivial collections you need groups to get
> > good results.
> 
> Yes, sure. But replace `record` with `stat` in the above and my point still 
> stands

The comment was for stat.

-Andi

[tip:perf/core] perf script: Allow adding and removing fields

[tip-bot for Andi Kleen]

Commit-ID:  36ce565114b4e7e3b83f40309675f6b1720957e4
Gitweb:     http://git.kernel.org/tip/36ce565114b4e7e3b83f40309675f6b1720957e4
Author:     Andi Kleen <ak@linux.intel.com>
AuthorDate: Fri, 2 Jun 2017 08:48:10 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 19 Jun 2017 15:14:58 -0300

perf script: Allow adding and removing fields

With 'perf script' it is common that we just want to add or remove a field.

Currently this requires figuring out the long list of default fields and
specifying them first, and then adding/removing the new field.

This patch adds a new + - syntax to merely add or remove fields,
that allows more succint and clearer command lines

For example to remove the comm field from PMU samples:

Previously

  $ perf script -F tid,cpu,time,event,sym,ip,dso,period | head -1
  swapper  0 [000] 504345.383126:          1 cycles:  ffffffff90060c66 native_write_msr ([kernel.kallsyms])

with the new syntax

  perf script -F -comm | head -1
  0 [000] 504345.383126:          1 cycles:  ffffffff90060c66 native_write_msr ([kernel.kallsyms])

The new syntax cannot be mixed with normal overriding.

v2: Fix example in description. Use tid vs pid. No functional changes.
v3: Don't skip initialization when user specified explicit type.
v4: Rebase. Remove empty line.

Committer testing:

  # perf record -a usleep 1
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 1.748 MB perf.data (14 samples) ]

Without a explicit field list specified via -F, defaults to:

  # perf script | head -2
      perf 6338 [000] 18467.058607: 1 cycles: ffffffff89060c36 native_write_msr (/lib/modules/4.11.0-rc8+/build/vmlinux)
   swapper    0 [001] 18467.058617: 1 cycles: ffffffff89060c36 native_write_msr (/lib/modules/4.11.0-rc8+/build/vmlinux)
  #

Which is equivalent to:

  # perf script -F comm,tid,cpu,time,period,event,ip,sym,dso | head -2
      perf 6338 [000] 18467.058607: 1 cycles: ffffffff89060c36 native_write_msr (/lib/modules/4.11.0-rc8+/build/vmlinux)
   swapper    0 [001] 18467.058617: 1 cycles: ffffffff89060c36 native_write_msr (/lib/modules/4.11.0-rc8+/build/vmlinux)
  #

So if we want to remove the comm, as in your original example, we would have to
figure out the default field list and remove ' comm' from it:

  # perf script -F tid,cpu,time,period,event,ip,sym,dso | head -2
   6338 [000] 18467.058607: 1 cycles: ffffffff89060c36 native_write_msr (/lib/modules/4.11.0-rc8+/build/vmlinux)
      0 [001] 18467.058617: 1 cycles: ffffffff89060c36 native_write_msr (/lib/modules/4.11.0-rc8+/build/vmlinux)
  #

With your patch this becomes simpler, one can remove fields by prefixing them
with '-':

  # perf script -F -comm | head -2
  6338 [000] 18467.058607: 1 cycles: ffffffff89060c36 native_write_msr (/lib/modules/4.11.0-rc8+/build/vmlinux)
     0 [001] 18467.058617: 1 cycles: ffffffff89060c36 native_write_msr (/lib/modules/4.11.0-rc8+/build/vmlinux)
  #

Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Tested-by: Milian Wolff <milian.wolff@kdab.com>
Link: http://lkml.kernel.org/r/20170602154810.15875-1-andi@firstfloor.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-script.txt |  8 +++++++
 tools/perf/builtin-script.c              | 37 +++++++++++++++++++++++++++++---
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
index 3517e20..3eca8c0 100644
--- a/tools/perf/Documentation/perf-script.txt
+++ b/tools/perf/Documentation/perf-script.txt
@@ -130,6 +130,14 @@ OPTIONS
 	i.e., the specified fields apply to all event types if the type string
 	is not given.
 
+	In addition to overriding fields, it is also possible to add or remove
+	fields from the defaults. For example
+
+		-F -cpu,+insn
+
+	removes the cpu field and adds the insn field. Adding/removing fields
+	cannot be mixed with normal overriding.
+
 	The arguments are processed in the order received. A later usage can
 	reset a prior request. e.g.:
 
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 4761b0d..afa84de 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1727,6 +1727,7 @@ static int parse_output_fields(const struct option *opt __maybe_unused,
 	int rc = 0;
 	char *str = strdup(arg);
 	int type = -1;
+	enum { DEFAULT, SET, ADD, REMOVE } change = DEFAULT;
 
 	if (!str)
 		return -ENOMEM;
@@ -1772,6 +1773,10 @@ static int parse_output_fields(const struct option *opt __maybe_unused,
 			goto out;
 		}
 
+		/* Don't override defaults for +- */
+		if (strchr(str, '+') || strchr(str, '-'))
+			goto parse;
+
 		if (output_set_by_user())
 			pr_warning("Overriding previous field request for all events.\n");
 
@@ -1782,13 +1787,30 @@ static int parse_output_fields(const struct option *opt __maybe_unused,
 		}
 	}
 
+parse:
 	for (tok = strtok_r(tok, ",", &strtok_saveptr); tok; tok = strtok_r(NULL, ",", &strtok_saveptr)) {
+		if (*tok == '+') {
+			if (change == SET)
+				goto out_badmix;
+			change = ADD;
+			tok++;
+		} else if (*tok == '-') {
+			if (change == SET)
+				goto out_badmix;
+			change = REMOVE;
+			tok++;
+		} else {
+			if (change != SET && change != DEFAULT)
+				goto out_badmix;
+			change = SET;
+		}
+
 		for (i = 0; i < imax; ++i) {
 			if (strcmp(tok, all_output_options[i].str) == 0)
 				break;
 		}
 		if (i == imax && strcmp(tok, "flags") == 0) {
-			print_flags = true;
+			print_flags = change == REMOVE ? false : true;
 			continue;
 		}
 		if (i == imax) {
@@ -1805,8 +1827,12 @@ static int parse_output_fields(const struct option *opt __maybe_unused,
 				if (output[j].invalid_fields & all_output_options[i].field) {
 					pr_warning("\'%s\' not valid for %s events. Ignoring.\n",
 						   all_output_options[i].str, event_type(j));
-				} else
-					output[j].fields |= all_output_options[i].field;
+				} else {
+					if (change == REMOVE)
+						output[j].fields &= ~all_output_options[i].field;
+					else
+						output[j].fields |= all_output_options[i].field;
+				}
 			}
 		} else {
 			if (output[type].invalid_fields & all_output_options[i].field) {
@@ -1826,7 +1852,11 @@ static int parse_output_fields(const struct option *opt __maybe_unused,
 				 "Events will not be displayed.\n", event_type(type));
 		}
 	}
+	goto out;
 
+out_badmix:
+	fprintf(stderr, "Cannot mix +-field with overridden fields\n");
+	rc = -EINVAL;
 out:
 	free(str);
 	return rc;
@@ -2444,6 +2474,7 @@ int cmd_script(int argc, const char **argv)
 		     symbol__config_symfs),
 	OPT_CALLBACK('F', "fields", NULL, "str",
 		     "comma separated output fields prepend with 'type:'. "
+		     "+field to add and -field to remove."
 		     "Valid types: hw,sw,trace,raw. "
 		     "Fields: comm,tid,pid,time,cpu,event,trace,ip,sym,dso,"
 		     "addr,symoff,period,iregs,brstack,brstacksym,flags,"