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.

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

But I cannot do:

$ perf record -e "topdown-*" ls
event syntax error: 'topdown-*'

Quoted 10 lines · Milian Wolff

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.

Quoted 4 lines

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.

Quoted 7 lines

Quoted 4 lines

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?

Quoted 4 lines

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

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(-)

tools/perf/Documentation/perf-script.txt +8 -0

index cb0eda3925e6..4547120c6ad3 100644
@@ -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.:
 

tools/perf/builtin-script.c +34 -3

index d05aec491cff..85c83cc3e994 100644
@@ -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,"

Signature · 1 line

2.9.4