2017-06-02 15:48:25

by Andi Kleen

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

From: Andi Kleen <[email protected]>

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 <[email protected]>
Signed-off-by: Andi Kleen <[email protected]>
---
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


2017-06-08 13:00:02

by Milian Wolff

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

On Friday, June 2, 2017 5:48:10 PM CEST Andi Kleen wrote:
> From: Andi Kleen <[email protected]>
>
> 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 <[email protected]>

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 | [email protected] | Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts


Attachments:
smime.p7s (3.74 kB)

2017-06-08 14:34:29

by Arnaldo Carvalho de Melo

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

Em Fri, Jun 02, 2017 at 08:48:10AM -0700, Andi Kleen escreveu:
> From: Andi Kleen <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Andi Kleen <[email protected]>
> ---
> 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

2017-06-09 02:52:45

by Andi Kleen

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

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

2017-06-09 09:13:33

by Milian Wolff

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

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 | [email protected] | Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts

2017-06-11 19:06:35

by Andi Kleen

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

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

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

Commit-ID: 36ce565114b4e7e3b83f40309675f6b1720957e4
Gitweb: http://git.kernel.org/tip/36ce565114b4e7e3b83f40309675f6b1720957e4
Author: Andi Kleen <[email protected]>
AuthorDate: Fri, 2 Jun 2017 08:48:10 -0700
Committer: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
Tested-by: Arnaldo Carvalho de Melo <[email protected]>
Tested-by: Milian Wolff <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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,"