Skip to contentSkip to conversation

perf/core improvements and fixes

GIT PULL In review Arnaldo Carvalho de Melo 3 participants 41 messages Aug 4, 2014 – Aug 13, 2014

Subsystems (3)
lore
Share & tools
lore

Arnaldo Carvalho de Melo

author maintainer
#0 lore raw

Hi Ingo,

Mostly fixes plus some refactoring work, more to come as I process
Adrian Hunter big patch set and other that missed this pull request.

Please consider pulling,

- Arnaldo

The following changes since commit f9b9f812235d53f774a083e88a5a23b517a69752:

Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf into perf/core (2014-07-30 14:51:06 +0200)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-core-for-mingo

for you to fetch changes up to 55b7e5fcf4782335c1ee5b4a2076697527c36407:

perf tools: Default to python version 2 (2014-08-04 12:21:37 -0300)

----------------------------------------------------------------
perf/core improvements and fixes:

User visible:

. Show better error message in case we fail to open counters due to EBUSY error,
for instance, when oprofile is running. (Jiri Olsa)

. Honour -w in the report tools (report, top), allowing to specify the widths
for the histogram entries columns. (Namhyung Kim)

. Don't run workload if not told to, as happens when the user has no
permission for profiling and even then the specified workload ends up running (Arnaldo Carvalho de Melo)

. Do not ignore mmap events in 'perf kmem report'. This tool was using
the kernel mmaps in the running machine instead of processing the mmap records from the perf.data file. (Namhyung Kim)

. Properly show submicrosecond times in 'perf kvm stat' (Christian Borntraeger)

. Allow the user to disable time stamps in 'perf record' (Andi Kleen)

. Make sure --symfs usage includes the path separator (Arnaldo Carvalho de Melo)

Developer Stuff:

. Fix arm64 build error (Mark Salter)

. Fix make PYTHON override (Namhyung Kim)

. Rename ordered_samples to ordered_events and allow setting a queue
size for ordering events (Jiri Olsa)

. Default to python version 2 (Thomas Ilsche)

----------------------------------------------------------------
Andi Kleen (1):
perf record: Allow the user to disable time stamps

Arnaldo Carvalho de Melo (2):
perf evlist: Don't run workload if not told to
perf symbols: Make sure --symfs usage includes the path separator

Christian Borntraeger (1):
perf kvm stat: Properly show submicrosecond times

Jiri Olsa (18):
perf tools: Rename ordered_samples bool to ordered_events
perf tools: Rename ordered_samples struct to ordered_events
perf tools: Rename ordered_events members
perf tools: Add ordered_events__(new|delete) interface
perf tools: Factor ordered_events__flush to be more generic
perf tools: Limit ordered events queue size
perf tools: Flush ordered events in case of allocation failure
perf tools: Make perf_session__deliver_event global
perf tools: Create ordered-events object
perf tools: Use list_move in ordered_events_delete function
perf tools: Add ordered_events__init function
perf tools: Add ordered_events__free function
perf tools: Add perf_config_u64 function
perf tools: Add report.queue-size config file option
perf tools: Add debug prints for ordered events queue
perf tools: Allow out of order messages in forced flush
perf tools: Show better error message in case we fail to open counters due to EBUSY error perf tools: Fix PERF_FLAG_FD_CLOEXEC flag probing event type open counters due to EBUSY error

Mark Salter (1):
perf tools: Fix arm64 build error

Namhyung Kim (9):
perf kmem: Do not ignore mmap events
perf tools: Fix make PYTHON override
perf tools: Left-align output contents
perf tools: Make __hpp__fmt() receive an additional len argument perf tools: Save column length in perf_hpp_fmt
perf report: Honor column width setting
perf top: Add -w option for setting column width
perf tools: Add name field into perf_hpp_fmt
perf tools: Fix column alignment when headers aren't shown on TUI

Thomas Ilsche (1):
perf tools: Default to python version 2

 tools/perf/Documentation/perf-report.txt  |   2 +-
 tools/perf/Documentation/perf-top.txt     |   6 +
 tools/perf/Makefile.perf                  |   2 +
 tools/perf/arch/arm64/include/perf_regs.h |   2 +
 tools/perf/builtin-annotate.c             |   2 +-
 tools/perf/builtin-diff.c                 |   2 +-
 tools/perf/builtin-inject.c               |   2 +-
 tools/perf/builtin-kmem.c                 |   7 +-
 tools/perf/builtin-kvm.c                  |  16 +-
 tools/perf/builtin-lock.c                 |   2 +-
 tools/perf/builtin-mem.c                  |   2 +-
 tools/perf/builtin-record.c               |   1 +
 tools/perf/builtin-report.c               |  15 +-
 tools/perf/builtin-sched.c                |   2 +-
 tools/perf/builtin-script.c               |   2 +-
 tools/perf/builtin-timechart.c            |   2 +-
 tools/perf/builtin-top.c                  |   3 +
 tools/perf/builtin-trace.c                |   2 +-
 tools/perf/config/Makefile                |  40 +++--
 tools/perf/config/feature-checks/Makefile |  18 +-
 tools/perf/ui/browsers/hists.c            |  31 ++--
 tools/perf/ui/gtk/hists.c                 |  18 +-
 tools/perf/ui/hist.c                      | 280 ++++++++++++++++++------------
 tools/perf/ui/stdio/hist.c                |   4 +-
 tools/perf/util/annotate.c                |   9 +-
 tools/perf/util/cache.h                   |   1 +
 tools/perf/util/cloexec.c                 |   6 +-
 tools/perf/util/color.c                   |  16 ++
 tools/perf/util/color.h                   |   1 +
 tools/perf/util/config.c                  |  24 +++
 tools/perf/util/debug.c                   |  36 +++-
 tools/perf/util/debug.h                   |   8 +
 tools/perf/util/dso.c                     |  28 ++-
 tools/perf/util/evlist.c                  |  20 ++-
 tools/perf/util/evsel.c                   |  15 +-
 tools/perf/util/hist.h                    |  17 +-
 tools/perf/util/ordered-events.c          | 245 ++++++++++++++++++++++++++
 tools/perf/util/ordered-events.h          |  51 ++++++
 tools/perf/util/session.c                 | 219 +++++------------------
 tools/perf/util/session.h                 |  25 +--
 tools/perf/util/sort.c                    |  58 ++++---
 tools/perf/util/symbol.c                  |   3 +-
 tools/perf/util/symbol.h                  |   9 +
 tools/perf/util/tool.h                    |   2 +-
 tools/perf/util/util.c                    |  36 ++++
 tools/perf/util/util.h                    |  17 ++
 46 files changed, 869 insertions(+), 440 deletions(-)
 create mode 100644 tools/perf/util/ordered-events.c
 create mode 100644 tools/perf/util/ordered-events.h
author maintainer
#1 lore raw

PATCH 1/33 perf tools: Fix arm64 build error

From: Mark Salter <[email protected]>

I'm seeing the following build error on arm64:

In file included from util/event.c:3:0:
util/event.h:95:17: error: 'PERF_REGS_MAX' undeclared here (not in a function)

    u64 cache_regs[PERF_REGS_MAX];
                 ^

This patch adds a PERF_REGS_MAX definition for arm64.

---
 tools/perf/arch/arm64/include/perf_regs.h | 2 ++
 1 file changed, 2 insertions(+)
tools/perf/arch/arm64/include/perf_regs.h +2 -0
index e9441b9e2a30..1d3f39c3aa56 100644
@@ -6,6 +6,8 @@
 #include <asm/perf_regs.h>
 
 #define PERF_REGS_MASK	((1ULL << PERF_REG_ARM64_MAX) - 1)
+#define PERF_REGS_MAX	PERF_REG_ARM64_MAX
+
 #define PERF_REG_IP	PERF_REG_ARM64_PC
 #define PERF_REG_SP	PERF_REG_ARM64_SP
 
Signature · 1 line
1.9.3
author maintainer
#2 lore raw

PATCH 2/33 perf evlist: Don't run workload if not told to

In reply to Arnaldo Carvalho de Melo

From: Arnaldo Carvalho de Melo <[email protected]>

The perf_evlist__prepare_workload() method works by forking and then waiting on a fd that must be written to to allow the workload to be exec()ed.

But if the tool calling it fails to, say, set up the events with which it wants to sample the workload for, it will not call perf_evlist__start_workload(), but even in this case the workload ended up running:

[acme@zoo linux]$ trace /bin/echo workload ends up running, it should not... Couldn't mmap the events: Operation not permitted
workload ends up running, it should not...
[acme@zoo linux]$

So check if at least one byte was written before letting exec() be called.

Now the expected behaviour:

[acme@zoo linux]$ trace /bin/echo workload ends up running, it should not... Couldn't mmap the events: Operation not permitted
[acme@zoo linux]$

Cc Adrian Hunter <[email protected]>
Cc David Ahern <[email protected]>
Cc Don Zickus <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Mike Galbraith <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/evlist.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)
tools/perf/util/evlist.c +18 -2
index 814e954c1318..3b366c085021 100644
@@ -1061,6 +1061,8 @@ int perf_evlist__prepare_workload(struct perf_evlist *evlist, struct target *tar
 	}
 
 	if (!evlist->workload.pid) {
+		int ret;
+
 		if (pipe_output)
 			dup2(2, 1);
 
@@ -1078,8 +1080,22 @@ int perf_evlist__prepare_workload(struct perf_evlist *evlist, struct target *tar
 		/*
 		 * Wait until the parent tells us to go.
 		 */
-		if (read(go_pipe[0], &bf, 1) == -1)
-			perror("unable to read pipe");
+		ret = read(go_pipe[0], &bf, 1);
+		/*
+		 * The parent will ask for the execvp() to be performed by
+		 * writing exactly one byte, in workload.cork_fd, usually via
+		 * perf_evlist__start_workload().
+		 *
+		 * For cancelling the workload without actuallin running it,
+		 * the parent will just close workload.cork_fd, without writing
+		 * anything, i.e. read will return zero and we just exit()
+		 * here.
+		 */
+		if (ret != 1) {
+			if (ret == -1)
+				perror("unable to read pipe");
+			exit(ret);
+		}
 
 		execvp(argv[0], (char **)argv);
 
Signature · 1 line
1.9.3
author maintainer
#3 lore raw

PATCH 3/33 perf symbols: Make sure --symfs usage includes the path separator

In reply to Arnaldo Carvalho de Melo

Minchan reported that perf failed to load vmlinux if --symfs argument doesn't end with '/' character.

Fix it by making sure that the '/' path separator is used when composing pathnames with a --symfs provided directory name.

---
 tools/perf/util/annotate.c |  9 +++------
 tools/perf/util/dso.c      | 28 +++++++++++++---------------
 tools/perf/util/symbol.c   |  3 +--
 tools/perf/util/symbol.h   |  9 +++++++++
 tools/perf/util/util.h     | 16 ++++++++++++++++
 5 files changed, 42 insertions(+), 23 deletions(-)
tools/perf/util/annotate.c +3 -6
index 809b4c50beae..7745fec01a6b 100644
@@ -899,10 +899,8 @@ int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize)
 	struct kcore_extract kce;
 	bool delete_extract = false;
 
-	if (filename) {
-		snprintf(symfs_filename, sizeof(symfs_filename), "%s%s",
-			 symbol_conf.symfs, filename);
-	}
+	if (filename)
+		symbol__join_symfs(symfs_filename, filename);
 
 	if (filename == NULL) {
 		if (dso->has_build_id) {
@@ -922,8 +920,7 @@ fallback:
 		 * DSO is the same as when 'perf record' ran.
 		 */
 		filename = (char *)dso->long_name;
-		snprintf(symfs_filename, sizeof(symfs_filename), "%s%s",
-			 symbol_conf.symfs, filename);
+		symbol__join_symfs(symfs_filename, filename);
 		free_filename = false;
 	}
 
tools/perf/util/dso.c +13 -15
index 90d02c661dd4..bdafd306fb52 100644
@@ -37,6 +37,7 @@ int dso__read_binary_type_filename(const struct dso *dso,
 {
 	char build_id_hex[BUILD_ID_SIZE * 2 + 1];
 	int ret = 0;
+	size_t len;
 
 	switch (type) {
 	case DSO_BINARY_TYPE__DEBUGLINK: {
@@ -60,26 +61,25 @@ int dso__read_binary_type_filename(const struct dso *dso,
 		break;
 
 	case DSO_BINARY_TYPE__FEDORA_DEBUGINFO:
-		snprintf(filename, size, "%s/usr/lib/debug%s.debug",
-			 symbol_conf.symfs, dso->long_name);
+		len = __symbol__join_symfs(filename, size, "/usr/lib/debug");
+		snprintf(filename + len, size - len, "%s.debug", dso->long_name);
 		break;
 
 	case DSO_BINARY_TYPE__UBUNTU_DEBUGINFO:
-		snprintf(filename, size, "%s/usr/lib/debug%s",
-			 symbol_conf.symfs, dso->long_name);
+		len = __symbol__join_symfs(filename, size, "/usr/lib/debug");
+		snprintf(filename + len, size - len, "%s", dso->long_name);
 		break;
 
 	case DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO:
 	{
 		const char *last_slash;
-		size_t len;
 		size_t dir_size;
 
 		last_slash = dso->long_name + dso->long_name_len;
 		while (last_slash != dso->long_name && *last_slash != '/')
 			last_slash--;
 
-		len = scnprintf(filename, size, "%s", symbol_conf.symfs);
+		len = __symbol__join_symfs(filename, size, "");
 		dir_size = last_slash - dso->long_name + 2;
 		if (dir_size > (size - len)) {
 			ret = -1;
@@ -100,26 +100,24 @@ int dso__read_binary_type_filename(const struct dso *dso,
 		build_id__sprintf(dso->build_id,
 				  sizeof(dso->build_id),
 				  build_id_hex);
-		snprintf(filename, size,
-			 "%s/usr/lib/debug/.build-id/%.2s/%s.debug",
-			 symbol_conf.symfs, build_id_hex, build_id_hex + 2);
+		len = __symbol__join_symfs(filename, size, "/usr/lib/debug/.build-id/");
+		snprintf(filename + len, size - len, "%.2s/%s.debug",
+			 build_id_hex, build_id_hex + 2);
 		break;
 
 	case DSO_BINARY_TYPE__VMLINUX:
 	case DSO_BINARY_TYPE__GUEST_VMLINUX:
 	case DSO_BINARY_TYPE__SYSTEM_PATH_DSO:
-		snprintf(filename, size, "%s%s",
-			 symbol_conf.symfs, dso->long_name);
+		__symbol__join_symfs(filename, size, dso->long_name);
 		break;
 
 	case DSO_BINARY_TYPE__GUEST_KMODULE:
-		snprintf(filename, size, "%s%s%s", symbol_conf.symfs,
-			 root_dir, dso->long_name);
+		path__join3(filename, size, symbol_conf.symfs,
+			    root_dir, dso->long_name);
 		break;
 
 	case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE:
-		snprintf(filename, size, "%s%s", symbol_conf.symfs,
-			 dso->long_name);
+		__symbol__join_symfs(filename, size, dso->long_name);
 		break;
 
 	case DSO_BINARY_TYPE__KCORE:
tools/perf/util/symbol.c +1 -2
index eb06746b06b2..f134ec138934 100644
@@ -1468,8 +1468,7 @@ int dso__load_vmlinux(struct dso *dso, struct map *map,
 	if (vmlinux[0] == '/')
 		snprintf(symfs_vmlinux, sizeof(symfs_vmlinux), "%s", vmlinux);
 	else
-		snprintf(symfs_vmlinux, sizeof(symfs_vmlinux), "%s%s",
-			 symbol_conf.symfs, vmlinux);
+		symbol__join_symfs(symfs_vmlinux, vmlinux);
 
 	if (dso->kernel == DSO_TYPE_GUEST_KERNEL)
 		symtab_type = DSO_BINARY_TYPE__GUEST_VMLINUX;
tools/perf/util/symbol.h +9 -0
index e7295e93cff9..196b29104276 100644
@@ -13,6 +13,7 @@
 #include <libgen.h>
 #include "build-id.h"
 #include "event.h"
+#include "util.h"
 
 #ifdef HAVE_LIBELF_SUPPORT
 #include <libelf.h>
@@ -143,6 +144,14 @@ struct symbol_conf {
 };
 
 extern struct symbol_conf symbol_conf;
+
+static inline int __symbol__join_symfs(char *bf, size_t size, const char *path)
+{
+	return path__join(bf, size, symbol_conf.symfs, path);
+}
+
+#define symbol__join_symfs(bf, path) __symbol__join_symfs(bf, sizeof(bf), path)
+
 extern int vmlinux_path__nr_entries;
 extern char **vmlinux_path;
 
tools/perf/util/util.h +16 -0
index 66864364ccb4..38af77550c75 100644
@@ -68,6 +68,7 @@
 #include <sys/socket.h>
 #include <sys/ioctl.h>
 #include <inttypes.h>
+#include <linux/kernel.h>
 #include <linux/magic.h>
 #include <linux/types.h>
 #include <sys/ttydefaults.h>
@@ -317,6 +318,21 @@ unsigned long parse_tag_value(const char *str, struct parse_tag *tags);
 
 #define SRCLINE_UNKNOWN  ((char *) "??:0")
 
+static inline int path__join(char *bf, size_t size,
+			     const char *path1, const char *path2)
+{
+	return scnprintf(bf, size, "%s%s%s", path1, path1[0] ? "/" : "", path2);
+}
+
+static inline int path__join3(char *bf, size_t size,
+			      const char *path1, const char *path2,
+			      const char *path3)
+{
+	return scnprintf(bf, size, "%s%s%s%s%s",
+			 path1, path1[0] ? "/" : "",
+			 path2, path2[0] ? "/" : "", path3);
+}
+
 struct dso;
 
 char *get_srcline(struct dso *dso, unsigned long addr);
Signature · 1 line
1.9.3
author maintainer
#4 lore raw

PATCH 4/33 perf kvm stat: Properly show submicrosecond times

In reply to Arnaldo Carvalho de Melo

From: Christian Borntraeger <[email protected]>

For lots of exits the min time (and sometimes max) is 0 or 1. Lets increase the accurancy similar to what the average field alread does.

---
 tools/perf/builtin-kvm.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
tools/perf/builtin-kvm.c +4 -4
index 43367eb00510..fe92dfdeab46 100644
@@ -592,8 +592,8 @@ static void print_result(struct perf_kvm_stat *kvm)
 	pr_info("%9s ", "Samples%");
 
 	pr_info("%9s ", "Time%");
-	pr_info("%10s ", "Min Time");
-	pr_info("%10s ", "Max Time");
+	pr_info("%11s ", "Min Time");
+	pr_info("%11s ", "Max Time");
 	pr_info("%16s ", "Avg time");
 	pr_info("\n\n");
 
@@ -610,8 +610,8 @@ static void print_result(struct perf_kvm_stat *kvm)
 		pr_info("%10llu ", (unsigned long long)ecount);
 		pr_info("%8.2f%% ", (double)ecount / kvm->total_count * 100);
 		pr_info("%8.2f%% ", (double)etime / kvm->total_time * 100);
-		pr_info("%8" PRIu64 "us ", min / 1000);
-		pr_info("%8" PRIu64 "us ", max / 1000);
+		pr_info("%9.2fus ", (double)min / 1e3);
+		pr_info("%9.2fus ", (double)max / 1e3);
 		pr_info("%9.2fus ( +-%7.2f%% )", (double)etime / ecount/1e3,
 			kvm_event_rel_stddev(vcpu, event));
 		pr_info("\n");
Signature · 1 line
1.9.3
author maintainer
#5 lore raw

PATCH 5/33 perf record: Allow the user to disable time stamps

In reply to Arnaldo Carvalho de Melo

From: Andi Kleen <[email protected]>

Time stamps are always implicitely enabled for record currently. The old --time/-T option is a nop.

Allow the user to disable timestamps by using --no-time

This can cause some minor misaccounting (by missing mmaps), but significantly lowers the size of perf.data

The defaults are unchanged.

---
 tools/perf/builtin-record.c | 1 +
 tools/perf/util/evsel.c     | 9 ++++++---
 2 files changed, 7 insertions(+), 3 deletions(-)
tools/perf/builtin-record.c +1 -0
index 4869050e7194..ca0251e8470d 100644
@@ -781,6 +781,7 @@ static const char * const record_usage[] = {
  */
 static struct record record = {
 	.opts = {
+		.sample_time	     = true,
 		.mmap_pages	     = UINT_MAX,
 		.user_freq	     = UINT_MAX,
 		.user_interval	     = ULLONG_MAX,
tools/perf/util/evsel.c +6 -3
index 21a373ebea22..92e5235f5377 100644
@@ -633,9 +633,12 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
 	if (opts->period)
 		perf_evsel__set_sample_bit(evsel, PERIOD);
 
-	if (!perf_missing_features.sample_id_all &&
-	    (opts->sample_time || !opts->no_inherit ||
-	     target__has_cpu(&opts->target) || per_cpu))
+	/*
+	 * When the user explicitely disabled time don't force it here.
+	 */
+	if (opts->sample_time &&
+	    (!perf_missing_features.sample_id_all &&
+	    (!opts->no_inherit || target__has_cpu(&opts->target) || per_cpu)))
 		perf_evsel__set_sample_bit(evsel, TIME);
 
 	if (opts->raw_samples && !evsel->no_aux_samples) {
Signature · 1 line
1.9.3
#6 lore raw

Re: [PATCH 05/33] perf record: Allow the user to disable time stamps

Quoted 2 lines · Arnaldo Carvalho de Melo

From: Andi Kleen <[email protected]>

This patch looks dangerous and misleading to me.

Quoted 7 lines

Time stamps are always implicitely enabled for record currently. The old --time/-T option is a nop.

Allow the user to disable timestamps by using --no-time

This can cause some minor misaccounting (by missing mmaps), but significantly lowers the size of perf.data

I'm not any big change in size:

-rw------- 1 mingo mingo 384768 Aug 5 08:01 perf.data.timestamps -rw------- 1 mingo mingo 336952 Aug 5 08:00 perf.data.notimestamps

That's a ~14% reduction - it's certainly noticeable but not earth-shattering.

And what is called 'minor misaccounting' in the description is actually a potentially broken profile that people might rely on.

So the patch description misleads twice: by exaggerating the advantages and also by downplaying the disadvantages.

So either remove the --time option altogether, or fix its 'misaccounting' so that the profile can be relied on.

Thanks,

Ingo

author maintainer
#7 lore raw

Re: [PATCH 05/33] perf record: Allow the user to disable time stamps

Em Tue, Aug 05, 2014 at 08:08:56AM +0200, Ingo Molnar escreveu:

Quoted 4 lines · Arnaldo Carvalho de Melo
Quoted 1 line

From: Andi Kleen <[email protected]>

This patch looks dangerous and misleading to me.

I took it more from the angle: hey, it fixes a regression, i.e. -T/--time somehow became a noop, and also it doesn't change any default.

It also states that you will get misaccounting, why that would happen, althought wording could've been better, as mmaps won't be "missing", i.e. they will be there as this governs just PERF_SAMPLE_TIME, not perf_event_attr.mmap, so it MMAPs may come after samples for its maps.

So, I took it.

How should we proceed?

- Arnaldo

Quoted 8 lines
Quoted 7 lines

Time stamps are always implicitely enabled for record currently. The old --time/-T option is a nop.

Allow the user to disable timestamps by using --no-time

This can cause some minor misaccounting (by missing mmaps), but significantly lowers the size of perf.data

Quoted 7 lines

I'm not any big change in size:

-rw------- 1 mingo mingo 384768 Aug 5 08:01 perf.data.timestamps -rw------- 1 mingo mingo 336952 Aug 5 08:00 perf.data.notimestamps

That's a ~14% reduction - it's certainly noticeable but not earth-shattering.

I wonder if this is for things like intel PT or some other super high volume scenario where 14% indeed looks like huge, but then not using PERF_SAMPLE_TIME there may have implications, but then, this is one of those use-only-if-you-know-what-are-the-implications knob, where the default is the sane one.

Quoted 12 lines

And what is called 'minor misaccounting' in the description is actually a potentially broken profile that people might rely on.

So the patch description misleads twice: by exaggerating the advantages and also by downplaying the disadvantages.

So either remove the --time option altogether, or fix its 'misaccounting' so that the profile can be relied on.

Thanks,

Ingo

#8 lore raw

Re: [PATCH 05/33] perf record: Allow the user to disable time stamps

Quoted 16 lines · Arnaldo Carvalho de Melo

Em Tue, Aug 05, 2014 at 08:08:56AM +0200, Ingo Molnar escreveu:

Quoted 4 lines · Arnaldo Carvalho de Melo
Quoted 1 line

From: Andi Kleen <[email protected]>

This patch looks dangerous and misleading to me.

I took it more from the angle: hey, it fixes a regression, i.e. -T/--time somehow became a noop, and also it doesn't change any default.

It also states that you will get misaccounting, why that would happen, althought wording could've been better, as mmaps won't be "missing", i.e. they will be there as this governs just PERF_SAMPLE_TIME, not perf_event_attr.mmap, so it MMAPs may come after samples for its maps.

Quoted 3 lines

So, I took it.

How should we proceed?

I'd be fine with applying the patch and fixing the misleading changelog.

Thanks,

Ingo

author maintainer
#9 lore raw

Re: [PATCH 05/33] perf record: Allow the user to disable time stamps

Em Tue, Aug 12, 2014 at 04:58:19PM +0200, Ingo Molnar escreveu:

Quoted 14 lines · Arnaldo Carvalho de Melo
Quoted 12 lines

Em Tue, Aug 05, 2014 at 08:08:56AM +0200, Ingo Molnar escreveu:

Quoted 1 line

This patch looks dangerous and misleading to me.

I took it more from the angle: hey, it fixes a regression, i.e. -T/--time somehow became a noop, and also it doesn't change any default.

It also states that you will get misaccounting, why that would happen, althought wording could've been better, as mmaps won't be "missing", i.e. they will be there as this governs just PERF_SAMPLE_TIME, not perf_event_attr.mmap, so it MMAPs may come after samples for its maps.

Quoted 6 lines
Quoted 3 lines

So, I took it.

How should we proceed?

I'd be fine with applying the patch and fixing the misleading changelog.

Done:

https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=8affc2b8c27bfc2d6e70827b746f490b62c44eaa

Consider now pulling the perf-core-for-mingo tag there, I updated it after updating the changelog for this specific commit, all the rest is the same as in the previous pull req.

Thanks,

- Arnaldo

#10 lore raw

Re: [PATCH 05/33] perf record: Allow the user to disable time stamps

Quoted 22 lines · Arnaldo Carvalho de Melo

Em Tue, Aug 12, 2014 at 04:58:19PM +0200, Ingo Molnar escreveu:

Quoted 14 lines · Arnaldo Carvalho de Melo
Quoted 12 lines

Em Tue, Aug 05, 2014 at 08:08:56AM +0200, Ingo Molnar escreveu:

Quoted 1 line

This patch looks dangerous and misleading to me.

I took it more from the angle: hey, it fixes a regression, i.e. -T/--time somehow became a noop, and also it doesn't change any default.

It also states that you will get misaccounting, why that would happen, althought wording could've been better, as mmaps won't be "missing", i.e. they will be there as this governs just PERF_SAMPLE_TIME, not perf_event_attr.mmap, so it MMAPs may come after samples for its maps.

Quoted 6 lines
Quoted 3 lines

So, I took it.

How should we proceed?

I'd be fine with applying the patch and fixing the misleading changelog.

Quoted 7 lines

Done:

https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=8affc2b8c27bfc2d6e70827b746f490b62c44eaa

Consider now pulling the perf-core-for-mingo tag there, I updated it after updating the changelog for this specific commit, all the rest is the same as in the previous pull req.

Pulled into tip:perf/core, thanks a lot Arnaldo!

Ingo

#11 lore raw

Re: [PATCH 05/33] perf record: Allow the user to disable time stamps

In reply to Ingo Molnar
Quoted 5 lines
Quoted 5 lines

Time stamps are always implicitely enabled for record currently. The old --time/-T option is a nop.

Allow the user to disable timestamps by using --no-time

Quoted 7 lines
Quoted 2 lines

This can cause some minor misaccounting (by missing mmaps), but significantly lowers the size of perf.data

I'm not any big change in size:

-rw------- 1 mingo mingo 384768 Aug 5 08:01 perf.data.timestamps -rw------- 1 mingo mingo 336952 Aug 5 08:00 perf.data.notimestamps

It will depend on your workload. What period did you use (or did it automatically use) and what kind of workload was it?

The smaller the period, the higher the benefit.

There are some classes of workloads where using a smaller period is especially beneficial, essentially anything with lots of small events, instead of long loops.

You also get a higher benefit if you use -c or -F, because without that each sample is smaller (no period reported)

You also get higher benefit for longer traces, and traces that do not start a lot of programs, as those tend to be dominated by MMAP events and other overhead.

Quoted 2 lines

So either remove the --time option altogether, or fix its 'misaccounting' so that the profile can be relied on.

I don't know how to fix it. Do you?

I guess one possible way to mitigate would be to lower the perf buffer sizes, then the worst case out of ordering would be less (I believe any potential problem
just comes from out of order events). However that may impact performance.

-Andi

author maintainer
#12 lore raw

Re: [PATCH 05/33] perf record: Allow the user to disable time stamps

Em Tue, Aug 05, 2014 at 07:17:33AM -0700, Andi Kleen escreveu:

Quoted 23 lines
Quoted 5 lines
Quoted 4 lines

Time stamps are always implicitely enabled for record currently. The old --time/-T option is a nop.

Allow the user to disable timestamps by using --no-time

Quoted 7 lines
Quoted 2 lines

This can cause some minor misaccounting (by missing mmaps), but significantly lowers the size of perf.data

I'm not any big change in size:

-rw------- 1 mingo mingo 384768 Aug 5 08:01 perf.data.timestamps -rw------- 1 mingo mingo 336952 Aug 5 08:00 perf.data.notimestamps

It will depend on your workload. What period did you use (or did it automatically use) and what kind of workload was it?

The smaller the period, the higher the benefit.

There are some classes of workloads where using a smaller period is especially beneficial, essentially anything with lots of small events, instead of long loops.

Quoted 6 lines

You also get a higher benefit if you use -c or -F, because without that each sample is smaller (no period reported)

You also get higher benefit for longer traces, and traces that do not start a lot of programs, as those tend to be dominated by MMAP events and other overhead.

I guess it would have been great if you had put the above paragraphs in the documentation for 'perf record' :-\

- Arnaldo

Quoted 10 lines
Quoted 2 lines

So either remove the --time option altogether, or fix its 'misaccounting' so that the profile can be relied on.

I don't know how to fix it. Do you?

I guess one possible way to mitigate would be to lower the perf buffer sizes, then the worst case out of ordering would be less (I believe any potential problem
just comes from out of order events). However that may impact performance.

Mentioning that OOO is the main problem here, i.e. that some samples may be misaccounted, should've also been added to the documentation.

- Arnaldo

author maintainer
#13 lore raw

PATCH 6/33 perf tools: Rename ordered_samples bool to ordered_events

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

The time ordering is generic for all kinds of events, so using generic name 'ordered_events' for ordered_samples bool in perf_tool struct.

No functional change was intended.

---
 tools/perf/builtin-annotate.c  |  2 +-
 tools/perf/builtin-diff.c      |  2 +-
 tools/perf/builtin-inject.c    |  2 +-
 tools/perf/builtin-kmem.c      |  2 +-
 tools/perf/builtin-kvm.c       |  4 ++--
 tools/perf/builtin-lock.c      |  2 +-
 tools/perf/builtin-mem.c       |  2 +-
 tools/perf/builtin-report.c    |  2 +-
 tools/perf/builtin-sched.c     |  2 +-
 tools/perf/builtin-script.c    |  2 +-
 tools/perf/builtin-timechart.c |  2 +-
 tools/perf/builtin-trace.c     |  2 +-
 tools/perf/util/session.c      | 10 +++++-----
 tools/perf/util/tool.h         |  2 +-
 14 files changed, 19 insertions(+), 19 deletions(-)
tools/perf/builtin-annotate.c +1 -1
index 1ec429fef2be..952b5ece6740 100644
@@ -297,7 +297,7 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
 			.comm	= perf_event__process_comm,
 			.exit	= perf_event__process_exit,
 			.fork	= perf_event__process_fork,
-			.ordered_samples = true,
+			.ordered_events = true,
 			.ordering_requires_timestamps = true,
 		},
 	};
tools/perf/builtin-diff.c +1 -1
index 9a5a035cb426..c10cc44bae19 100644
@@ -360,7 +360,7 @@ static struct perf_tool tool = {
 	.exit	= perf_event__process_exit,
 	.fork	= perf_event__process_fork,
 	.lost	= perf_event__process_lost,
-	.ordered_samples = true,
+	.ordered_events = true,
 	.ordering_requires_timestamps = true,
 };
 
tools/perf/builtin-inject.c +1 -1
index 9a02807387d6..ee875cca13b1 100644
@@ -366,7 +366,7 @@ static int __cmd_inject(struct perf_inject *inject)
 	} else if (inject->sched_stat) {
 		struct perf_evsel *evsel;
 
-		inject->tool.ordered_samples = true;
+		inject->tool.ordered_events = true;
 
 		evlist__for_each(session->evlist, evsel) {
 			const char *name = perf_evsel__name(evsel);
tools/perf/builtin-kmem.c +1 -1
index bef3376bfaf3..b5721116713b 100644
@@ -256,7 +256,7 @@ static int process_sample_event(struct perf_tool *tool __maybe_unused,
 static struct perf_tool perf_kmem = {
 	.sample		 = process_sample_event,
 	.comm		 = perf_event__process_comm,
-	.ordered_samples = true,
+	.ordered_events	 = true,
 };
 
 static double fragmentation(unsigned long n_req, unsigned long n_alloc)
tools/perf/builtin-kvm.c +2 -2
index fe92dfdeab46..a05f43592e0a 100644
@@ -1058,7 +1058,7 @@ static int read_events(struct perf_kvm_stat *kvm)
 	struct perf_tool eops = {
 		.sample			= process_sample_event,
 		.comm			= perf_event__process_comm,
-		.ordered_samples	= true,
+		.ordered_events		= true,
 	};
 	struct perf_data_file file = {
 		.path = kvm->file_name,
@@ -1311,7 +1311,7 @@ static int kvm_events_live(struct perf_kvm_stat *kvm,
 	kvm->tool.exit   = perf_event__process_exit;
 	kvm->tool.fork   = perf_event__process_fork;
 	kvm->tool.lost   = process_lost_event;
-	kvm->tool.ordered_samples = true;
+	kvm->tool.ordered_events = true;
 	perf_tool__fill_defaults(&kvm->tool);
 
 	/* set defaults */
tools/perf/builtin-lock.c +1 -1
index 6148afc995c6..c8122d323621 100644
@@ -852,7 +852,7 @@ static int __cmd_report(bool display_info)
 	struct perf_tool eops = {
 		.sample		 = process_sample_event,
 		.comm		 = perf_event__process_comm,
-		.ordered_samples = true,
+		.ordered_events	 = true,
 	};
 	struct perf_data_file file = {
 		.path = input_name,
tools/perf/builtin-mem.c +1 -1
index 4a1a6c94a5eb..80e57c84adef 100644
@@ -194,7 +194,7 @@ int cmd_mem(int argc, const char **argv, const char *prefix __maybe_unused)
 			.lost		= perf_event__process_lost,
 			.fork		= perf_event__process_fork,
 			.build_id	= perf_event__process_build_id,
-			.ordered_samples = true,
+			.ordered_events	= true,
 		},
 		.input_name		 = "perf.data",
 	};
tools/perf/builtin-report.c +1 -1
index 21d830bafff3..c72cc5a12144 100644
@@ -578,7 +578,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 			.attr		 = perf_event__process_attr,
 			.tracing_data	 = perf_event__process_tracing_data,
 			.build_id	 = perf_event__process_build_id,
-			.ordered_samples = true,
+			.ordered_events	 = true,
 			.ordering_requires_timestamps = true,
 		},
 		.max_stack		 = PERF_MAX_STACK_DEPTH,
tools/perf/builtin-sched.c +1 -1
index f83c08c0dd87..7c16aeb6b675 100644
@@ -1662,7 +1662,7 @@ int cmd_sched(int argc, const char **argv, const char *prefix __maybe_unused)
 			.comm		 = perf_event__process_comm,
 			.lost		 = perf_event__process_lost,
 			.fork		 = perf_sched__process_fork_event,
-			.ordered_samples = true,
+			.ordered_events = true,
 		},
 		.cmp_pid	      = LIST_HEAD_INIT(sched.cmp_pid),
 		.sort_list	      = LIST_HEAD_INIT(sched.sort_list),
tools/perf/builtin-script.c +1 -1
index f57035b89c15..868c17d09762 100644
@@ -1488,7 +1488,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
 			.attr		 = process_attr,
 			.tracing_data	 = perf_event__process_tracing_data,
 			.build_id	 = perf_event__process_build_id,
-			.ordered_samples = true,
+			.ordered_events	 = true,
 			.ordering_requires_timestamps = true,
 		},
 	};
tools/perf/builtin-timechart.c +1 -1
index 2f1a5220c090..912e3b5bb22b 100644
@@ -1920,7 +1920,7 @@ int cmd_timechart(int argc, const char **argv,
 			.fork		 = process_fork_event,
 			.exit		 = process_exit_event,
 			.sample		 = process_sample_event,
-			.ordered_samples = true,
+			.ordered_events	 = true,
 		},
 		.proc_num = 15,
 		.min_time = 1000000,
tools/perf/builtin-trace.c +1 -1
index a6c375224f46..36ae51d61be2 100644
@@ -2209,7 +2209,7 @@ static int trace__replay(struct trace *trace)
 	trace->tool.tracing_data = perf_event__process_tracing_data;
 	trace->tool.build_id	  = perf_event__process_build_id;
 
-	trace->tool.ordered_samples = true;
+	trace->tool.ordered_events = true;
 	trace->tool.ordering_requires_timestamps = true;
 
 	/* add tid to output */
tools/perf/util/session.c +5 -5
index 88dfef70c13d..a2c97ff1aa6a 100644
@@ -104,9 +104,9 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
 	}
 
 	if (tool && tool->ordering_requires_timestamps &&
-	    tool->ordered_samples && !perf_evlist__sample_id_all(session->evlist)) {
+	    tool->ordered_events && !perf_evlist__sample_id_all(session->evlist)) {
 		dump_printf("WARNING: No sample_id_all support, falling back to unordered processing\n");
-		tool->ordered_samples = false;
+		tool->ordered_events = false;
 	}
 
 	return session;
@@ -238,7 +238,7 @@ void perf_tool__fill_defaults(struct perf_tool *tool)
 	if (tool->build_id == NULL)
 		tool->build_id = process_finished_round_stub;
 	if (tool->finished_round == NULL) {
-		if (tool->ordered_samples)
+		if (tool->ordered_events)
 			tool->finished_round = process_finished_round;
 		else
 			tool->finished_round = process_finished_round_stub;
@@ -483,7 +483,7 @@ static int flush_sample_queue(struct perf_session *s,
 	struct ui_progress prog;
 	int ret;
 
-	if (!tool->ordered_samples || !limit)
+	if (!tool->ordered_events || !limit)
 		return 0;
 
 	if (show_progress)
@@ -1062,7 +1062,7 @@ static s64 perf_session__process_event(struct perf_session *session,
 	if (ret)
 		return ret;
 
-	if (tool->ordered_samples) {
+	if (tool->ordered_events) {
 		ret = perf_session_queue_event(session, event, &sample,
 					       file_offset);
 		if (ret != -ETIME)
tools/perf/util/tool.h +1 -1
index 4385816d3d49..f11636966a0f 100644
@@ -40,7 +40,7 @@ struct perf_tool {
 	event_op2	tracing_data;
 	event_op2	finished_round,
 			build_id;
-	bool		ordered_samples;
+	bool		ordered_events;
 	bool		ordering_requires_timestamps;
 };
 
Signature · 1 line
1.9.3
author maintainer
#14 lore raw

PATCH 7/33 perf tools: Rename ordered_samples struct to ordered_events

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

Following up with ordered_samples rename for ordered_samples and sample_queue structs to ordered_events and ordered_event structs respectively.

Also changing flush_sample_queue function name to ordered_events_flush.

No functional change was intended.

---
 tools/perf/builtin-kvm.c  |   2 +-
 tools/perf/util/session.c | 118 +++++++++++++++++++++++-----------------------
 tools/perf/util/session.h |  10 ++--
 3 files changed, 65 insertions(+), 65 deletions(-)
tools/perf/builtin-kvm.c +1 -1
index a05f43592e0a..258a5274099d 100644
@@ -785,7 +785,7 @@ static int perf_kvm__mmap_read(struct perf_kvm_stat *kvm)
 
 	/* flush queue after each round in which we processed events */
 	if (ntotal) {
-		kvm->session->ordered_samples.next_flush = flush_time;
+		kvm->session->ordered_events.next_flush = flush_time;
 		err = kvm->tool.finished_round(&kvm->tool, NULL, kvm->session);
 		if (err) {
 			if (kvm->lost_events)
tools/perf/util/session.c +59 -59

233 lines not on this page · raw message

tools/perf/util/session.h +5 -5
index 0321013bd9fd..f6baf935917a 100644
@@ -12,19 +12,19 @@
 #include <linux/rbtree.h>
 #include <linux/perf_event.h>
 
-struct sample_queue;
+struct ordered_event;
 struct ip_callchain;
 struct thread;
 
-struct ordered_samples {
+struct ordered_events {
 	u64			last_flush;
 	u64			next_flush;
 	u64			max_timestamp;
 	struct list_head	samples;
 	struct list_head	sample_cache;
 	struct list_head	to_free;
-	struct sample_queue	*sample_buffer;
-	struct sample_queue	*last_sample;
+	struct ordered_event	*sample_buffer;
+	struct ordered_event	*last_sample;
 	int			sample_buffer_idx;
 	unsigned int		nr_samples;
 };
@@ -39,7 +39,7 @@ struct perf_session {
 	bool			one_mmap;
 	void			*one_mmap_addr;
 	u64			one_mmap_offset;
-	struct ordered_samples	ordered_samples;
+	struct ordered_events	ordered_events;
 	struct perf_data_file	*file;
 };
 
Signature · 1 line
1.9.3
author maintainer
#15 lore raw

PATCH 8/33 perf tools: Rename ordered_events members

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

Rename 'struct ordered_events' members to fit better the ordered events style.

No functional change was intended.

---
 tools/perf/util/session.c | 86 +++++++++++++++++++++++------------------------
 tools/perf/util/session.h | 12 +++----
 2 files changed, 48 insertions(+), 50 deletions(-)
tools/perf/util/session.c +42 -44
index 6570282a7625..619778ef5b58 100644
@@ -75,8 +75,8 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
 		goto out;
 
 	session->repipe = repipe;
-	INIT_LIST_HEAD(&session->ordered_events.samples);
-	INIT_LIST_HEAD(&session->ordered_events.sample_cache);
+	INIT_LIST_HEAD(&session->ordered_events.events);
+	INIT_LIST_HEAD(&session->ordered_events.cache);
 	INIT_LIST_HEAD(&session->ordered_events.to_free);
 	machines__init(&session->machines);
 
@@ -474,11 +474,11 @@ static int ordered_events__flush(struct perf_session *s,
 				 struct perf_tool *tool)
 {
 	struct ordered_events *oe = &s->ordered_events;
-	struct list_head *head = &oe->samples;
+	struct list_head *head = &oe->events;
 	struct ordered_event *tmp, *iter;
 	struct perf_sample sample;
 	u64 limit = oe->next_flush;
-	u64 last_ts = oe->last_sample ? oe->last_sample->timestamp : 0ULL;
+	u64 last_ts = oe->last ? oe->last->timestamp : 0ULL;
 	bool show_progress = limit == ULLONG_MAX;
 	struct ui_progress prog;
 	int ret;
@@ -487,7 +487,7 @@ static int ordered_events__flush(struct perf_session *s,
 		return 0;
 
 	if (show_progress)
-		ui_progress__init(&prog, oe->nr_samples, "Processing time ordered events...");
+		ui_progress__init(&prog, oe->nr_events, "Processing time ordered events...");
 
 	list_for_each_entry_safe(iter, tmp, head, list) {
 		if (session_done())
@@ -508,19 +508,17 @@ static int ordered_events__flush(struct perf_session *s,
 
 		oe->last_flush = iter->timestamp;
 		list_del(&iter->list);
-		list_add(&iter->list, &oe->sample_cache);
-		oe->nr_samples--;
+		list_add(&iter->list, &oe->cache);
+		oe->nr_events--;
 
 		if (show_progress)
 			ui_progress__update(&prog, 1);
 	}
 
-	if (list_empty(head)) {
-		oe->last_sample = NULL;
-	} else if (last_ts <= limit) {
-		oe->last_sample =
-			list_entry(head->prev, struct ordered_event, list);
-	}
+	if (list_empty(head))
+		oe->last = NULL;
+	else if (last_ts <= limit)
+		oe->last = list_entry(head->prev, struct ordered_event, list);
 
 	return 0;
 }
@@ -579,45 +577,45 @@ static int process_finished_round(struct perf_tool *tool,
 static void __queue_event(struct ordered_event *new, struct perf_session *s)
 {
 	struct ordered_events *oe = &s->ordered_events;
-	struct ordered_event *sample = oe->last_sample;
+	struct ordered_event *last = oe->last;
 	u64 timestamp = new->timestamp;
 	struct list_head *p;
 
-	++oe->nr_samples;
-	oe->last_sample = new;
+	++oe->nr_events;
+	oe->last = new;
 
-	if (!sample) {
-		list_add(&new->list, &oe->samples);
+	if (!last) {
+		list_add(&new->list, &oe->events);
 		oe->max_timestamp = timestamp;
 		return;
 	}
 
 	/*
-	 * last_sample might point to some random place in the list as it's
+	 * last event might point to some random place in the list as it's
 	 * the last queued event. We expect that the new event is close to
 	 * this.
 	 */
-	if (sample->timestamp <= timestamp) {
-		while (sample->timestamp <= timestamp) {
-			p = sample->list.next;
-			if (p == &oe->samples) {
-				list_add_tail(&new->list, &oe->samples);
+	if (last->timestamp <= timestamp) {
+		while (last->timestamp <= timestamp) {
+			p = last->list.next;
+			if (p == &oe->events) {
+				list_add_tail(&new->list, &oe->events);
 				oe->max_timestamp = timestamp;
 				return;
 			}
-			sample = list_entry(p, struct ordered_event, list);
+			last = list_entry(p, struct ordered_event, list);
 		}
-		list_add_tail(&new->list, &sample->list);
+		list_add_tail(&new->list, &last->list);
 	} else {
-		while (sample->timestamp > timestamp) {
-			p = sample->list.prev;
-			if (p == &oe->samples) {
-				list_add(&new->list, &oe->samples);
+		while (last->timestamp > timestamp) {
+			p = last->list.prev;
+			if (p == &oe->events) {
+				list_add(&new->list, &oe->events);
 				return;
 			}
-			sample = list_entry(p, struct ordered_event, list);
+			last = list_entry(p, struct ordered_event, list);
 		}
-		list_add(&new->list, &sample->list);
+		list_add(&new->list, &last->list);
 	}
 }
 
@@ -627,7 +625,7 @@ int perf_session_queue_event(struct perf_session *s, union perf_event *event,
 				    struct perf_sample *sample, u64 file_offset)
 {
 	struct ordered_events *oe = &s->ordered_events;
-	struct list_head *sc = &oe->sample_cache;
+	struct list_head *cache = &oe->cache;
 	u64 timestamp = sample->time;
 	struct ordered_event *new;
 
@@ -639,20 +637,20 @@ int perf_session_queue_event(struct perf_session *s, union perf_event *event,
 		return -EINVAL;
 	}
 
-	if (!list_empty(sc)) {
-		new = list_entry(sc->next, struct ordered_event, list);
+	if (!list_empty(cache)) {
+		new = list_entry(cache->next, struct ordered_event, list);
 		list_del(&new->list);
-	} else if (oe->sample_buffer) {
-		new = oe->sample_buffer + oe->sample_buffer_idx;
-		if (++oe->sample_buffer_idx == MAX_SAMPLE_BUFFER)
-			oe->sample_buffer = NULL;
+	} else if (oe->buffer) {
+		new = oe->buffer + oe->buffer_idx;
+		if (++oe->buffer_idx == MAX_SAMPLE_BUFFER)
+			oe->buffer = NULL;
 	} else {
-		oe->sample_buffer = malloc(MAX_SAMPLE_BUFFER * sizeof(*new));
-		if (!oe->sample_buffer)
+		oe->buffer = malloc(MAX_SAMPLE_BUFFER * sizeof(*new));
+		if (!oe->buffer)
 			return -ENOMEM;
-		list_add(&oe->sample_buffer->list, &oe->to_free);
-		oe->sample_buffer_idx = 2;
-		new = oe->sample_buffer + 1;
+		list_add(&oe->buffer->list, &oe->to_free);
+		oe->buffer_idx = 2;
+		new = oe->buffer + 1;
 	}
 
 	new->timestamp = timestamp;
tools/perf/util/session.h +6 -6
index f6baf935917a..419eb50e1cd3 100644
@@ -20,13 +20,13 @@ struct ordered_events {
 	u64			last_flush;
 	u64			next_flush;
 	u64			max_timestamp;
-	struct list_head	samples;
-	struct list_head	sample_cache;
+	struct list_head	events;
+	struct list_head	cache;
 	struct list_head	to_free;
-	struct ordered_event	*sample_buffer;
-	struct ordered_event	*last_sample;
-	int			sample_buffer_idx;
-	unsigned int		nr_samples;
+	struct ordered_event	*buffer;
+	struct ordered_event	*last;
+	int			buffer_idx;
+	unsigned int		nr_events;
 };
 
 struct perf_session {
Signature · 1 line
1.9.3
author maintainer
#16 lore raw

PATCH 9/33 perf tools: Add ordered_events__(new|delete) interface

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

Adding new ordered events interface to new|delete event buffer:

ordered_events__new - allocate event buffer from the cache
ordered_events__delete - return event buffer to the cache

---
 tools/perf/util/session.c | 169 +++++++++++++++++++++++++++-------------------
 1 file changed, 98 insertions(+), 71 deletions(-)
tools/perf/util/session.c +98 -71
index 619778ef5b58..ff0188c65783 100644
@@ -464,6 +464,100 @@ static void perf_session_free_sample_buffers(struct perf_session *session)
 	}
 }
 
+/* The queue is ordered by time */
+static void queue_event(struct ordered_events *oe, struct ordered_event *new)
+{
+	struct ordered_event *last = oe->last;
+	u64 timestamp = new->timestamp;
+	struct list_head *p;
+
+	++oe->nr_events;
+	oe->last = new;
+
+	if (!last) {
+		list_add(&new->list, &oe->events);
+		oe->max_timestamp = timestamp;
+		return;
+	}
+
+	/*
+	 * last event might point to some random place in the list as it's
+	 * the last queued event. We expect that the new event is close to
+	 * this.
+	 */
+	if (last->timestamp <= timestamp) {
+		while (last->timestamp <= timestamp) {
+			p = last->list.next;
+			if (p == &oe->events) {
+				list_add_tail(&new->list, &oe->events);
+				oe->max_timestamp = timestamp;
+				return;
+			}
+			last = list_entry(p, struct ordered_event, list);
+		}
+		list_add_tail(&new->list, &last->list);
+	} else {
+		while (last->timestamp > timestamp) {
+			p = last->list.prev;
+			if (p == &oe->events) {
+				list_add(&new->list, &oe->events);
+				return;
+			}
+			last = list_entry(p, struct ordered_event, list);
+		}
+		list_add(&new->list, &last->list);
+	}
+}
+
+#define MAX_SAMPLE_BUFFER	(64 * 1024 / sizeof(struct ordered_event))
+static struct ordered_event *alloc_event(struct ordered_events *oe)
+{
+	struct list_head *cache = &oe->cache;
+	struct ordered_event *new;
+
+	if (!list_empty(cache)) {
+		new = list_entry(cache->next, struct ordered_event, list);
+		list_del(&new->list);
+	} else if (oe->buffer) {
+		new = oe->buffer + oe->buffer_idx;
+		if (++oe->buffer_idx == MAX_SAMPLE_BUFFER)
+			oe->buffer = NULL;
+	} else {
+		oe->buffer = malloc(MAX_SAMPLE_BUFFER * sizeof(*new));
+		if (!oe->buffer)
+			return NULL;
+		list_add(&oe->buffer->list, &oe->to_free);
+
+		/* First entry is abused to maintain the to_free list. */
+		oe->buffer_idx = 2;
+		new = oe->buffer + 1;
+	}
+
+	return new;
+}
+
+static struct ordered_event *
+ordered_events__new(struct ordered_events *oe, u64 timestamp)
+{
+	struct ordered_event *new;
+
+	new = alloc_event(oe);
+	if (new) {
+		new->timestamp = timestamp;
+		queue_event(oe, new);
+	}
+
+	return new;
+}
+
+static void
+ordered_events__delete(struct ordered_events *oe, struct ordered_event *event)
+{
+	list_del(&event->list);
+	list_add(&event->list, &oe->cache);
+	oe->nr_events--;
+}
+
 static int perf_session_deliver_event(struct perf_session *session,
 				      union perf_event *event,
 				      struct perf_sample *sample,
@@ -506,10 +600,8 @@ static int ordered_events__flush(struct perf_session *s,
 				return ret;
 		}
 
+		ordered_events__delete(oe, iter);
 		oe->last_flush = iter->timestamp;
-		list_del(&iter->list);
-		list_add(&iter->list, &oe->cache);
-		oe->nr_events--;
 
 		if (show_progress)
 			ui_progress__update(&prog, 1);
@@ -573,59 +665,10 @@ static int process_finished_round(struct perf_tool *tool,
 	return ret;
 }
 
-/* The queue is ordered by time */
-static void __queue_event(struct ordered_event *new, struct perf_session *s)
-{
-	struct ordered_events *oe = &s->ordered_events;
-	struct ordered_event *last = oe->last;
-	u64 timestamp = new->timestamp;
-	struct list_head *p;
-
-	++oe->nr_events;
-	oe->last = new;
-
-	if (!last) {
-		list_add(&new->list, &oe->events);
-		oe->max_timestamp = timestamp;
-		return;
-	}
-
-	/*
-	 * last event might point to some random place in the list as it's
-	 * the last queued event. We expect that the new event is close to
-	 * this.
-	 */
-	if (last->timestamp <= timestamp) {
-		while (last->timestamp <= timestamp) {
-			p = last->list.next;
-			if (p == &oe->events) {
-				list_add_tail(&new->list, &oe->events);
-				oe->max_timestamp = timestamp;
-				return;
-			}
-			last = list_entry(p, struct ordered_event, list);
-		}
-		list_add_tail(&new->list, &last->list);
-	} else {
-		while (last->timestamp > timestamp) {
-			p = last->list.prev;
-			if (p == &oe->events) {
-				list_add(&new->list, &oe->events);
-				return;
-			}
-			last = list_entry(p, struct ordered_event, list);
-		}
-		list_add(&new->list, &last->list);
-	}
-}
-
-#define MAX_SAMPLE_BUFFER	(64 * 1024 / sizeof(struct ordered_event))
-
 int perf_session_queue_event(struct perf_session *s, union perf_event *event,
 				    struct perf_sample *sample, u64 file_offset)
 {
 	struct ordered_events *oe = &s->ordered_events;
-	struct list_head *cache = &oe->cache;
 	u64 timestamp = sample->time;
 	struct ordered_event *new;
 
@@ -637,28 +680,12 @@ int perf_session_queue_event(struct perf_session *s, union perf_event *event,
 		return -EINVAL;
 	}
 
-	if (!list_empty(cache)) {
-		new = list_entry(cache->next, struct ordered_event, list);
-		list_del(&new->list);
-	} else if (oe->buffer) {
-		new = oe->buffer + oe->buffer_idx;
-		if (++oe->buffer_idx == MAX_SAMPLE_BUFFER)
-			oe->buffer = NULL;
-	} else {
-		oe->buffer = malloc(MAX_SAMPLE_BUFFER * sizeof(*new));
-		if (!oe->buffer)
-			return -ENOMEM;
-		list_add(&oe->buffer->list, &oe->to_free);
-		oe->buffer_idx = 2;
-		new = oe->buffer + 1;
-	}
+	new = ordered_events__new(oe, timestamp);
+	if (!new)
+		return -ENOMEM;
 
-	new->timestamp = timestamp;
 	new->file_offset = file_offset;
 	new->event = event;
-
-	__queue_event(new, s);
-
 	return 0;
 }
 
Signature · 1 line
1.9.3
author maintainer
#17 lore raw

PATCH 10/33 perf tools: Factor ordered_events__flush to be more generic

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

Centralizing the next_flush calculation under the ordered_events__flush function.

---
 tools/perf/util/session.c | 47 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 36 insertions(+), 11 deletions(-)
tools/perf/util/session.c +36 -11
index ff0188c65783..72c7b0d3d0dc 100644
@@ -451,6 +451,11 @@ struct ordered_event {
 	struct list_head	list;
 };
 
+enum oe_flush {
+	OE_FLUSH__FINAL,
+	OE_FLUSH__ROUND,
+};
+
 static void perf_session_free_sample_buffers(struct perf_session *session)
 {
 	struct ordered_events *oe = &session->ordered_events;
@@ -564,8 +569,8 @@ static int perf_session_deliver_event(struct perf_session *session,
 				      struct perf_tool *tool,
 				      u64 file_offset);
 
-static int ordered_events__flush(struct perf_session *s,
-				 struct perf_tool *tool)
+static int __ordered_events__flush(struct perf_session *s,
+				   struct perf_tool *tool)
 {
 	struct ordered_events *oe = &s->ordered_events;
 	struct list_head *head = &oe->events;
@@ -615,6 +620,32 @@ static int ordered_events__flush(struct perf_session *s,
 	return 0;
 }
 
+static int ordered_events__flush(struct perf_session *s, struct perf_tool *tool,
+				 enum oe_flush how)
+{
+	struct ordered_events *oe = &s->ordered_events;
+	int err;
+
+	switch (how) {
+	case OE_FLUSH__FINAL:
+		oe->next_flush = ULLONG_MAX;
+		break;
+
+	case OE_FLUSH__ROUND:
+	default:
+		break;
+	};
+
+	err = __ordered_events__flush(s, tool);
+
+	if (!err) {
+		if (how == OE_FLUSH__ROUND)
+			oe->next_flush = oe->max_timestamp;
+	}
+
+	return err;
+}
+
 /*
  * When perf record finishes a pass on every buffers, it records this pseudo
  * event.
@@ -658,11 +689,7 @@ static int process_finished_round(struct perf_tool *tool,
 				  union perf_event *event __maybe_unused,
 				  struct perf_session *session)
 {
-	int ret = ordered_events__flush(session, tool);
-	if (!ret)
-		session->ordered_events.next_flush = session->ordered_events.max_timestamp;
-
-	return ret;
+	return ordered_events__flush(session, tool, OE_FLUSH__ROUND);
 }
 
 int perf_session_queue_event(struct perf_session *s, union perf_event *event,
@@ -1247,8 +1274,7 @@ more:
 		goto more;
 done:
 	/* do the final flush for ordered samples */
-	session->ordered_events.next_flush = ULLONG_MAX;
-	err = ordered_events__flush(session, tool);
+	err = ordered_events__flush(session, tool, OE_FLUSH__FINAL);
 out_err:
 	free(buf);
 	perf_session__warn_about_errors(session, tool);
@@ -1393,8 +1419,7 @@ more:
 
 out:
 	/* do the final flush for ordered samples */
-	session->ordered_events.next_flush = ULLONG_MAX;
-	err = ordered_events__flush(session, tool);
+	err = ordered_events__flush(session, tool, OE_FLUSH__FINAL);
 out_err:
 	ui_progress__finish();
 	perf_session__warn_about_errors(session, tool);
Signature · 1 line
1.9.3
author maintainer
#18 lore raw

PATCH 11/33 perf tools: Limit ordered events queue size

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

Add limit to the ordered events queue allocation. This way we will be able to control the size of the queue buffers.

There's no limit at the moment (it's set to (u64) -1). The config code will come in following patches.

---
 tools/perf/util/session.c | 12 +++++++++---
 tools/perf/util/session.h |  2 ++
 2 files changed, 11 insertions(+), 3 deletions(-)
tools/perf/util/session.c +9 -3
index 72c7b0d3d0dc..8d4538c91076 100644
@@ -78,6 +78,8 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
 	INIT_LIST_HEAD(&session->ordered_events.events);
 	INIT_LIST_HEAD(&session->ordered_events.cache);
 	INIT_LIST_HEAD(&session->ordered_events.to_free);
+	session->ordered_events.max_alloc_size = (u64) -1;
+	session->ordered_events.cur_alloc_size = 0;
 	machines__init(&session->machines);
 
 	if (file) {
@@ -518,7 +520,7 @@ static void queue_event(struct ordered_events *oe, struct ordered_event *new)
 static struct ordered_event *alloc_event(struct ordered_events *oe)
 {
 	struct list_head *cache = &oe->cache;
-	struct ordered_event *new;
+	struct ordered_event *new = NULL;
 
 	if (!list_empty(cache)) {
 		new = list_entry(cache->next, struct ordered_event, list);
@@ -527,10 +529,14 @@ static struct ordered_event *alloc_event(struct ordered_events *oe)
 		new = oe->buffer + oe->buffer_idx;
 		if (++oe->buffer_idx == MAX_SAMPLE_BUFFER)
 			oe->buffer = NULL;
-	} else {
-		oe->buffer = malloc(MAX_SAMPLE_BUFFER * sizeof(*new));
+	} else if (oe->cur_alloc_size < oe->max_alloc_size) {
+		size_t size = MAX_SAMPLE_BUFFER * sizeof(*new);
+
+		oe->buffer = malloc(size);
 		if (!oe->buffer)
 			return NULL;
+
+		oe->cur_alloc_size += size;
 		list_add(&oe->buffer->list, &oe->to_free);
 
 		/* First entry is abused to maintain the to_free list. */
tools/perf/util/session.h +2 -0
index 419eb50e1cd3..e2fbaf2567e1 100644
@@ -20,6 +20,8 @@ struct ordered_events {
 	u64			last_flush;
 	u64			next_flush;
 	u64			max_timestamp;
+	u64			max_alloc_size;
+	u64			cur_alloc_size;
 	struct list_head	events;
 	struct list_head	cache;
 	struct list_head	to_free;
Signature · 1 line
1.9.3
author maintainer
#19 lore raw

PATCH 12/33 perf tools: Flush ordered events in case of allocation failure

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

In previous patches we added a limit for ordered events queue allocation size. If we reach this size we need to flush (part of) the queue to get some free buffers.

The current functionality is not affected, because the limit is hard coded to (u64) -1. The configuration code for size will come in following patches.

---
 tools/perf/builtin-kvm.c  |  2 +-
 tools/perf/util/session.c | 29 +++++++++++++++++++++++++++--
 tools/perf/util/session.h |  3 ++-
 3 files changed, 30 insertions(+), 4 deletions(-)
tools/perf/builtin-kvm.c +1 -1
index 258a5274099d..7ccceadcd9f8 100644
@@ -732,7 +732,7 @@ static s64 perf_kvm__mmap_read_idx(struct perf_kvm_stat *kvm, int idx,
 			return -1;
 		}
 
-		err = perf_session_queue_event(kvm->session, event, &sample, 0);
+		err = perf_session_queue_event(kvm->session, event, &kvm->tool, &sample, 0);
 		/*
 		 * FIXME: Here we can't consume the event, as perf_session_queue_event will
 		 *        point to it, and it'll get possibly overwritten by the kernel.
tools/perf/util/session.c +27 -2
index 8d4538c91076..bd2483b6b446 100644
@@ -14,6 +14,7 @@
 #include "util.h"
 #include "cpumap.h"
 #include "perf_regs.h"
+#include "asm/bug.h"
 
 static int perf_session__open(struct perf_session *session)
 {
@@ -456,6 +457,7 @@ struct ordered_event {
 enum oe_flush {
 	OE_FLUSH__FINAL,
 	OE_FLUSH__ROUND,
+	OE_FLUSH__HALF,
 };
 
 static void perf_session_free_sample_buffers(struct perf_session *session)
@@ -637,6 +639,23 @@ static int ordered_events__flush(struct perf_session *s, struct perf_tool *tool,
 		oe->next_flush = ULLONG_MAX;
 		break;
 
+	case OE_FLUSH__HALF:
+	{
+		struct ordered_event *first, *last;
+		struct list_head *head = &oe->events;
+
+		first = list_entry(head->next, struct ordered_event, list);
+		last = oe->last;
+
+		/* Warn if we are called before any event got allocated. */
+		if (WARN_ONCE(!last || list_empty(head), "empty queue"))
+			return 0;
+
+		oe->next_flush  = first->timestamp;
+		oe->next_flush += (last->timestamp - first->timestamp) / 2;
+		break;
+	}
+
 	case OE_FLUSH__ROUND:
 	default:
 		break;
@@ -699,7 +718,8 @@ static int process_finished_round(struct perf_tool *tool,
 }
 
 int perf_session_queue_event(struct perf_session *s, union perf_event *event,
-				    struct perf_sample *sample, u64 file_offset)
+			     struct perf_tool *tool, struct perf_sample *sample,
+			     u64 file_offset)
 {
 	struct ordered_events *oe = &s->ordered_events;
 	u64 timestamp = sample->time;
@@ -714,6 +734,11 @@ int perf_session_queue_event(struct perf_session *s, union perf_event *event,
 	}
 
 	new = ordered_events__new(oe, timestamp);
+	if (!new) {
+		ordered_events__flush(s, tool, OE_FLUSH__HALF);
+		new = ordered_events__new(oe, timestamp);
+	}
+
 	if (!new)
 		return -ENOMEM;
 
@@ -1121,7 +1146,7 @@ static s64 perf_session__process_event(struct perf_session *session,
 		return ret;
 
 	if (tool->ordered_events) {
-		ret = perf_session_queue_event(session, event, &sample,
+		ret = perf_session_queue_event(session, event, tool, &sample,
 					       file_offset);
 		if (ret != -ETIME)
 			return ret;
tools/perf/util/session.h +2 -1
index e2fbaf2567e1..a09e3c8d825a 100644
@@ -67,7 +67,8 @@ int perf_session__process_events(struct perf_session *session,
 				 struct perf_tool *tool);
 
 int perf_session_queue_event(struct perf_session *s, union perf_event *event,
-			     struct perf_sample *sample, u64 file_offset);
+			     struct perf_tool *tool, struct perf_sample *sample,
+			     u64 file_offset);
 
 void perf_tool__fill_defaults(struct perf_tool *tool);
 
Signature · 1 line
1.9.3
author maintainer
#20 lore raw

PATCH 13/33 perf tools: Make perf_session__deliver_event global

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

Making perf_session__deliver_event global function, as it will be called from another object in following patch.

Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
[ Fixup naming to match class__method schema, as now is more widely exposed ]
---
 tools/perf/util/session.c | 23 ++++++++---------------
 tools/perf/util/session.h |  5 +++++
 2 files changed, 13 insertions(+), 15 deletions(-)
tools/perf/util/session.c +8 -15
index bd2483b6b446..ed6b7f14631f 100644
@@ -571,12 +571,6 @@ ordered_events__delete(struct ordered_events *oe, struct ordered_event *event)
 	oe->nr_events--;
 }
 
-static int perf_session_deliver_event(struct perf_session *session,
-				      union perf_event *event,
-				      struct perf_sample *sample,
-				      struct perf_tool *tool,
-				      u64 file_offset);
-
 static int __ordered_events__flush(struct perf_session *s,
 				   struct perf_tool *tool)
 {
@@ -607,8 +601,8 @@ static int __ordered_events__flush(struct perf_session *s,
 		if (ret)
 			pr_err("Can't parse sample, err = %d\n", ret);
 		else {
-			ret = perf_session_deliver_event(s, iter->event, &sample, tool,
-							 iter->file_offset);
+			ret = perf_session__deliver_event(s, iter->event, &sample, tool,
+							  iter->file_offset);
 			if (ret)
 				return ret;
 		}
@@ -1003,11 +997,10 @@ perf_session__deliver_sample(struct perf_session *session,
 					    &sample->read.one, machine);
 }
 
-static int perf_session_deliver_event(struct perf_session *session,
-				      union perf_event *event,
-				      struct perf_sample *sample,
-				      struct perf_tool *tool,
-				      u64 file_offset)
+int perf_session__deliver_event(struct perf_session *session,
+				union perf_event *event,
+				struct perf_sample *sample,
+				struct perf_tool *tool, u64 file_offset)
 {
 	struct perf_evsel *evsel;
 	struct machine *machine;
@@ -1152,8 +1145,8 @@ static s64 perf_session__process_event(struct perf_session *session,
 			return ret;
 	}
 
-	return perf_session_deliver_event(session, event, &sample, tool,
-					  file_offset);
+	return perf_session__deliver_event(session, event, &sample, tool,
+					   file_offset);
 }
 
 void perf_event_header__bswap(struct perf_event_header *hdr)
tools/perf/util/session.h +5 -0
index a09e3c8d825a..03da1cb14dc1 100644
@@ -72,6 +72,11 @@ int perf_session_queue_event(struct perf_session *s, union perf_event *event,
 
 void perf_tool__fill_defaults(struct perf_tool *tool);
 
+int perf_session__deliver_event(struct perf_session *session,
+				union perf_event *event,
+				struct perf_sample *sample,
+				struct perf_tool *tool, u64 file_offset);
+
 int perf_session__resolve_callchain(struct perf_session *session,
 				    struct perf_evsel *evsel,
 				    struct thread *thread,
Signature · 1 line
1.9.3
author maintainer
#21 lore raw

PATCH 14/33 perf tools: Create ordered-events object

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

Move ordered events code into separated object ordered-events.[ch].

No functional change was intended.

---
 tools/perf/Makefile.perf         |   2 +
 tools/perf/util/ordered-events.c | 196 +++++++++++++++++++++++++++++++++++++
 tools/perf/util/ordered-events.h |  41 ++++++++
 tools/perf/util/session.c        | 206 ---------------------------------------
 tools/perf/util/session.h        |  17 +---
 5 files changed, 240 insertions(+), 222 deletions(-)
 create mode 100644 tools/perf/util/ordered-events.c
 create mode 100644 tools/perf/util/ordered-events.h
tools/perf/Makefile.perf +2 -0
index 2240974b7745..1ea31e275b4d 100644
@@ -263,6 +263,7 @@ LIB_H += util/xyarray.h
 LIB_H += util/header.h
 LIB_H += util/help.h
 LIB_H += util/session.h
+LIB_H += util/ordered-events.h
 LIB_H += util/strbuf.h
 LIB_H += util/strlist.h
 LIB_H += util/strfilter.h
@@ -347,6 +348,7 @@ LIB_OBJS += $(OUTPUT)util/machine.o
 LIB_OBJS += $(OUTPUT)util/map.o
 LIB_OBJS += $(OUTPUT)util/pstack.o
 LIB_OBJS += $(OUTPUT)util/session.o
+LIB_OBJS += $(OUTPUT)util/ordered-events.o
 LIB_OBJS += $(OUTPUT)util/comm.o
 LIB_OBJS += $(OUTPUT)util/thread.o
 LIB_OBJS += $(OUTPUT)util/thread_map.o
tools/perf/util/ordered-events.c new file +196 -0

196 lines not on this page · raw message

tools/perf/util/ordered-events.h new file +41 -0

41 lines not on this page · raw message

tools/perf/util/session.c +0 -206

224 lines not on this page · raw message

tools/perf/util/session.h +1 -16
index 03da1cb14dc1..0630e658f8be 100644
@@ -9,28 +9,13 @@
 #include "symbol.h"
 #include "thread.h"
 #include "data.h"
+#include "ordered-events.h"
 #include <linux/rbtree.h>
 #include <linux/perf_event.h>
 
-struct ordered_event;
 struct ip_callchain;
 struct thread;
 
-struct ordered_events {
-	u64			last_flush;
-	u64			next_flush;
-	u64			max_timestamp;
-	u64			max_alloc_size;
-	u64			cur_alloc_size;
-	struct list_head	events;
-	struct list_head	cache;
-	struct list_head	to_free;
-	struct ordered_event	*buffer;
-	struct ordered_event	*last;
-	int			buffer_idx;
-	unsigned int		nr_events;
-};
-
 struct perf_session {
 	struct perf_header	header;
 	struct machines		machines;
Signature · 1 line
1.9.3
author maintainer
#22 lore raw

PATCH 15/33 perf tools: Use list_move in ordered_events_delete function

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

As Namhyung pointed out we can use list_move in ordered_events_delete.

---
 tools/perf/util/ordered-events.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
tools/perf/util/ordered-events.c +1 -2
index 95f8211ccdde..be6a48ce3eea 100644
@@ -96,8 +96,7 @@ ordered_events__new(struct ordered_events *oe, u64 timestamp)
 
 void ordered_events__delete(struct ordered_events *oe, struct ordered_event *event)
 {
-	list_del(&event->list);
-	list_add(&event->list, &oe->cache);
+	list_move(&event->list, &oe->cache);
 	oe->nr_events--;
 }
 
Signature · 1 line
1.9.3
author maintainer
#23 lore raw

PATCH 16/33 perf tools: Add ordered_events__init function

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

Adding ordered_events__init function for struct ordered_events struct initialization.

---
 tools/perf/util/ordered-events.c | 9 +++++++++
 tools/perf/util/ordered-events.h | 1 +
 tools/perf/util/session.c        | 6 +-----
 3 files changed, 11 insertions(+), 5 deletions(-)
tools/perf/util/ordered-events.c +9 -0
index be6a48ce3eea..7b8923e12dc7 100644
@@ -193,3 +193,12 @@ int ordered_events__flush(struct perf_session *s, struct perf_tool *tool,
 
 	return err;
 }
+
+void ordered_events__init(struct ordered_events *oe)
+{
+	INIT_LIST_HEAD(&oe->events);
+	INIT_LIST_HEAD(&oe->cache);
+	INIT_LIST_HEAD(&oe->to_free);
+	oe->max_alloc_size = (u64) -1;
+	oe->cur_alloc_size = 0;
+}
tools/perf/util/ordered-events.h +1 -0
index 8309983bdd70..62f9945319b6 100644
@@ -38,4 +38,5 @@ struct ordered_event *ordered_events__new(struct ordered_events *oe, u64 timesta
 void ordered_events__delete(struct ordered_events *oe, struct ordered_event *event);
 int ordered_events__flush(struct perf_session *s, struct perf_tool *tool,
 			  enum oe_flush how);
+void ordered_events__init(struct ordered_events *oe);
 #endif /* __ORDERED_EVENTS_H */
tools/perf/util/session.c +1 -5
index 0ccf051247f6..7f5851e433bb 100644
@@ -75,11 +75,7 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
 		goto out;
 
 	session->repipe = repipe;
-	INIT_LIST_HEAD(&session->ordered_events.events);
-	INIT_LIST_HEAD(&session->ordered_events.cache);
-	INIT_LIST_HEAD(&session->ordered_events.to_free);
-	session->ordered_events.max_alloc_size = (u64) -1;
-	session->ordered_events.cur_alloc_size = 0;
+	ordered_events__init(&session->ordered_events);
 	machines__init(&session->machines);
 
 	if (file) {
Signature · 1 line
1.9.3
author maintainer
#24 lore raw

PATCH 17/33 perf tools: Add ordered_events__free function

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

Adding ordered_events__free function to release all the struct ordered_events data. It's replacement for former perf_session_free_sample_buffers function.

---
 tools/perf/util/ordered-events.c | 11 +++++++++++
 tools/perf/util/ordered-events.h |  1 +
 tools/perf/util/session.c        | 17 ++---------------
 3 files changed, 14 insertions(+), 15 deletions(-)
tools/perf/util/ordered-events.c +11 -0
index 7b8923e12dc7..381d5fea1131 100644
@@ -202,3 +202,14 @@ void ordered_events__init(struct ordered_events *oe)
 	oe->max_alloc_size = (u64) -1;
 	oe->cur_alloc_size = 0;
 }
+
+void ordered_events__free(struct ordered_events *oe)
+{
+	while (!list_empty(&oe->to_free)) {
+		struct ordered_event *event;
+
+		event = list_entry(oe->to_free.next, struct ordered_event, list);
+		list_del(&event->list);
+		free(event);
+	}
+}
tools/perf/util/ordered-events.h +1 -0
index 62f9945319b6..6036bd657073 100644
@@ -39,4 +39,5 @@ void ordered_events__delete(struct ordered_events *oe, struct ordered_event *eve
 int ordered_events__flush(struct perf_session *s, struct perf_tool *tool,
 			  enum oe_flush how);
 void ordered_events__init(struct ordered_events *oe);
+void ordered_events__free(struct ordered_events *oe);
 #endif /* __ORDERED_EVENTS_H */
tools/perf/util/session.c +2 -15
index 7f5851e433bb..7d8dbf213d64 100644
@@ -442,19 +442,6 @@ static perf_event__swap_op perf_event__swap_ops[] = {
 	[PERF_RECORD_HEADER_MAX]	  = NULL,
 };
 
-static void perf_session_free_sample_buffers(struct perf_session *session)
-{
-	struct ordered_events *oe = &session->ordered_events;
-
-	while (!list_empty(&oe->to_free)) {
-		struct ordered_event *event;
-
-		event = list_entry(oe->to_free.next, struct ordered_event, list);
-		list_del(&event->list);
-		free(event);
-	}
-}
-
 /*
  * When perf record finishes a pass on every buffers, it records this pseudo
  * event.
@@ -1092,7 +1079,7 @@ done:
 out_err:
 	free(buf);
 	perf_session__warn_about_errors(session, tool);
-	perf_session_free_sample_buffers(session);
+	ordered_events__free(&session->ordered_events);
 	return err;
 }
 
@@ -1237,7 +1224,7 @@ out:
 out_err:
 	ui_progress__finish();
 	perf_session__warn_about_errors(session, tool);
-	perf_session_free_sample_buffers(session);
+	ordered_events__free(&session->ordered_events);
 	session->one_mmap = false;
 	return err;
 }
Signature · 1 line
1.9.3
author maintainer
#25 lore raw

PATCH 18/33 perf tools: Add perf_config_u64 function

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

Adding perf_config_u64 function to be able to parse 'llong' values out of config file.

---
 tools/perf/util/cache.h  |  1 +
 tools/perf/util/config.c | 24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+)
tools/perf/util/cache.h +1 -0
index 7b176dd02e1a..5cf9e1b5989d 100644
@@ -22,6 +22,7 @@ typedef int (*config_fn_t)(const char *, const char *, void *);
 extern int perf_default_config(const char *, const char *, void *);
 extern int perf_config(config_fn_t fn, void *);
 extern int perf_config_int(const char *, const char *);
+extern u64 perf_config_u64(const char *, const char *);
 extern int perf_config_bool(const char *, const char *);
 extern int config_error_nonbool(const char *);
 extern const char *perf_config_dirname(const char *, const char *);
tools/perf/util/config.c +24 -0
index 1e5e2e5af6b1..9970b8b0190b 100644
@@ -286,6 +286,21 @@ static int parse_unit_factor(const char *end, unsigned long *val)
 	return 0;
 }
 
+static int perf_parse_llong(const char *value, long long *ret)
+{
+	if (value && *value) {
+		char *end;
+		long long val = strtoll(value, &end, 0);
+		unsigned long factor = 1;
+
+		if (!parse_unit_factor(end, &factor))
+			return 0;
+		*ret = val * factor;
+		return 1;
+	}
+	return 0;
+}
+
 static int perf_parse_long(const char *value, long *ret)
 {
 	if (value && *value) {
@@ -307,6 +322,15 @@ static void die_bad_config(const char *name)
 	die("bad config value for '%s'", name);
 }
 
+u64 perf_config_u64(const char *name, const char *value)
+{
+	long long ret = 0;
+
+	if (!perf_parse_llong(value, &ret))
+		die_bad_config(name);
+	return (u64) ret;
+}
+
 int perf_config_int(const char *name, const char *value)
 {
 	long ret = 0;
Signature · 1 line
1.9.3
author maintainer
#26 lore raw

PATCH 19/33 perf tools: Add report.queue-size config file option

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

Adding report.queue-size config file option to setup the maximum allocation size for session's struct ordered_events object.

---
 tools/perf/builtin-report.c      | 13 ++++++++++++-
 tools/perf/util/ordered-events.h |  6 ++++++
 2 files changed, 18 insertions(+), 1 deletion(-)
tools/perf/builtin-report.c +12 -1
index c72cc5a12144..041e93da13e4 100644
@@ -58,17 +58,19 @@ struct report {
 	const char		*symbol_filter_str;
 	float			min_percent;
 	u64			nr_entries;
+	u64			queue_size;
 	DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
 };
 
 static int report__config(const char *var, const char *value, void *cb)
 {
+	struct report *rep = cb;
+
 	if (!strcmp(var, "report.group")) {
 		symbol_conf.event_group = perf_config_bool(var, value);
 		return 0;
 	}
 	if (!strcmp(var, "report.percent-limit")) {
-		struct report *rep = cb;
 		rep->min_percent = strtof(value, NULL);
 		return 0;
 	}
@@ -76,6 +78,10 @@ static int report__config(const char *var, const char *value, void *cb)
 		symbol_conf.cumulate_callchain = perf_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "report.queue-size")) {
+		rep->queue_size = perf_config_u64(var, value);
+		return 0;
+	}
 
 	return perf_default_config(var, value, cb);
 }
@@ -714,6 +720,11 @@ repeat:
 	if (session == NULL)
 		return -ENOMEM;
 
+	if (report.queue_size) {
+		ordered_events__set_alloc_size(&session->ordered_events,
+					       report.queue_size);
+	}
+
 	report.session = session;
 
 	has_br_stack = perf_header__has_feat(&session->header,
tools/perf/util/ordered-events.h +6 -0
index 6036bd657073..2ce847fb48a5 100644
@@ -40,4 +40,10 @@ int ordered_events__flush(struct perf_session *s, struct perf_tool *tool,
 			  enum oe_flush how);
 void ordered_events__init(struct ordered_events *oe);
 void ordered_events__free(struct ordered_events *oe);
+
+static inline
+void ordered_events__set_alloc_size(struct ordered_events *oe, u64 size)
+{
+	oe->max_alloc_size = size;
+}
 #endif /* __ORDERED_EVENTS_H */
Signature · 1 line
1.9.3
author maintainer
#27 lore raw

PATCH 20/33 perf tools: Add debug prints for ordered events queue

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

Adding some prints for ordered events queue, to help debug issues.

Adding debug_ordered_events debug variable to be able to enable ordered events debug messages using:

  $ perf --debug ordered-events=2 report ...

Also using oe pointer in perf_session__queue_event instead of chained session variable dereferencing.

---
 tools/perf/util/debug.c          | 36 +++++++++++++++++++++++++++++++++++-
 tools/perf/util/debug.h          |  8 ++++++++
 tools/perf/util/ordered-events.c | 26 ++++++++++++++++++++++++++
 tools/perf/util/session.c        |  4 +++-
 4 files changed, 72 insertions(+), 2 deletions(-)
tools/perf/util/debug.c +35 -1
index 71d419362634..ba357f3226c6 100644
@@ -13,8 +13,12 @@
 #include "util.h"
 #include "target.h"
 
+#define NSECS_PER_SEC  1000000000ULL
+#define NSECS_PER_USEC 1000ULL
+
 int verbose;
 bool dump_trace = false, quiet = false;
+int debug_ordered_events;
 
 static int _eprintf(int level, int var, const char *fmt, va_list args)
 {
@@ -42,6 +46,35 @@ int eprintf(int level, int var, const char *fmt, ...)
 	return ret;
 }
 
+static int __eprintf_time(u64 t, const char *fmt, va_list args)
+{
+	int ret = 0;
+	u64 secs, usecs, nsecs = t;
+
+	secs   = nsecs / NSECS_PER_SEC;
+	nsecs -= secs  * NSECS_PER_SEC;
+	usecs  = nsecs / NSECS_PER_USEC;
+
+	ret = fprintf(stderr, "[%13" PRIu64 ".%06" PRIu64 "] ",
+		      secs, usecs);
+	ret += vfprintf(stderr, fmt, args);
+	return ret;
+}
+
+int eprintf_time(int level, int var, u64 t, const char *fmt, ...)
+{
+	int ret = 0;
+	va_list args;
+
+	if (var >= level) {
+		va_start(args, fmt);
+		ret = __eprintf_time(t, fmt, args);
+		va_end(args);
+	}
+
+	return ret;
+}
+
 /*
  * Overloading libtraceevent standard info print
  * function, display with -v in perf.
@@ -110,7 +143,8 @@ static struct debug_variable {
 	const char *name;
 	int *ptr;
 } debug_variables[] = {
-	{ .name = "verbose", .ptr = &verbose },
+	{ .name = "verbose",		.ptr = &verbose },
+	{ .name = "ordered-events",	.ptr = &debug_ordered_events},
 	{ .name = NULL, }
 };
 
tools/perf/util/debug.h +8 -0
index 89fb6b0f7ab2..6944ea3a119b 100644
@@ -10,6 +10,7 @@
 
 extern int verbose;
 extern bool quiet, dump_trace;
+extern int debug_ordered_events;
 
 #ifndef pr_fmt
 #define pr_fmt(fmt) fmt
@@ -29,6 +30,12 @@ extern bool quiet, dump_trace;
 #define pr_debug3(fmt, ...) pr_debugN(3, pr_fmt(fmt), ##__VA_ARGS__)
 #define pr_debug4(fmt, ...) pr_debugN(4, pr_fmt(fmt), ##__VA_ARGS__)
 
+#define pr_time_N(n, var, t, fmt, ...) \
+	eprintf_time(n, var, t, fmt, ##__VA_ARGS__)
+
+#define pr_oe_time(t, fmt, ...)  pr_time_N(1, debug_ordered_events, t, pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_oe_time2(t, fmt, ...) pr_time_N(2, debug_ordered_events, t, pr_fmt(fmt), ##__VA_ARGS__)
+
 int dump_printf(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
 void trace_event(union perf_event *event);
 
@@ -38,6 +45,7 @@ int ui__warning(const char *format, ...) __attribute__((format(printf, 1, 2)));
 void pr_stat(const char *fmt, ...);
 
 int eprintf(int level, int var, const char *fmt, ...) __attribute__((format(printf, 3, 4)));
+int eprintf_time(int level, int var, u64 t, const char *fmt, ...) __attribute__((format(printf, 4, 5)));
 
 int perf_debug_option(const char *str);
 
tools/perf/util/ordered-events.c +26 -0
index 381d5fea1131..8a8aeab8e7ec 100644
@@ -1,10 +1,16 @@
 #include <linux/list.h>
+#include <linux/compiler.h>
 #include "ordered-events.h"
 #include "evlist.h"
 #include "session.h"
 #include "asm/bug.h"
 #include "debug.h"
 
+#define pr_N(n, fmt, ...) \
+	eprintf(n, debug_ordered_events, fmt, ##__VA_ARGS__)
+
+#define pr(fmt, ...) pr_N(1, pr_fmt(fmt), ##__VA_ARGS__)
+
 static void queue_event(struct ordered_events *oe, struct ordered_event *new)
 {
 	struct ordered_event *last = oe->last;
@@ -14,6 +20,8 @@ static void queue_event(struct ordered_events *oe, struct ordered_event *new)
 	++oe->nr_events;
 	oe->last = new;
 
+	pr_oe_time2(timestamp, "queue_event nr_events %u\n", oe->nr_events);
+
 	if (!last) {
 		list_add(&new->list, &oe->events);
 		oe->max_timestamp = timestamp;
@@ -69,12 +77,17 @@ static struct ordered_event *alloc_event(struct ordered_events *oe)
 		if (!oe->buffer)
 			return NULL;
 
+		pr("alloc size %" PRIu64 "B (+%zu), max %" PRIu64 "B\n",
+		   oe->cur_alloc_size, size, oe->max_alloc_size);
+
 		oe->cur_alloc_size += size;
 		list_add(&oe->buffer->list, &oe->to_free);
 
 		/* First entry is abused to maintain the to_free list. */
 		oe->buffer_idx = 2;
 		new = oe->buffer + 1;
+	} else {
+		pr("allocation limit reached %" PRIu64 "B\n", oe->max_alloc_size);
 	}
 
 	return new;
@@ -155,6 +168,11 @@ int ordered_events__flush(struct perf_session *s, struct perf_tool *tool,
 			  enum oe_flush how)
 {
 	struct ordered_events *oe = &s->ordered_events;
+	static const char * const str[] = {
+		"FINAL",
+		"ROUND",
+		"HALF ",
+	};
 	int err;
 
 	switch (how) {
@@ -184,6 +202,10 @@ int ordered_events__flush(struct perf_session *s, struct perf_tool *tool,
 		break;
 	};
 
+	pr_oe_time(oe->next_flush, "next_flush - ordered_events__flush PRE  %s, nr_events %u\n",
+		   str[how], oe->nr_events);
+	pr_oe_time(oe->max_timestamp, "max_timestamp\n");
+
 	err = __ordered_events__flush(s, tool);
 
 	if (!err) {
@@ -191,6 +213,10 @@ int ordered_events__flush(struct perf_session *s, struct perf_tool *tool,
 			oe->next_flush = oe->max_timestamp;
 	}
 
+	pr_oe_time(oe->next_flush, "next_flush - ordered_events__flush POST %s, nr_events %u\n",
+		   str[how], oe->nr_events);
+	pr_oe_time(oe->last_flush, "last_flush\n");
+
 	return err;
 }
 
tools/perf/util/session.c +3 -1
index 7d8dbf213d64..462be2749462 100644
@@ -499,8 +499,10 @@ int perf_session_queue_event(struct perf_session *s, union perf_event *event,
 	if (!timestamp || timestamp == ~0ULL)
 		return -ETIME;
 
-	if (timestamp < s->ordered_events.last_flush) {
+	if (timestamp < oe->last_flush) {
 		printf("Warning: Timestamp below last timeslice flush\n");
+		pr_oe_time(timestamp,      "out of order event");
+		pr_oe_time(oe->last_flush, "last flush");
 		return -EINVAL;
 	}
 
Signature · 1 line
1.9.3
author maintainer
#28 lore raw

PATCH 21/33 perf tools: Allow out of order messages in forced flush

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

In forced flush (OE_FLUSH__HALF) we break the rules of the flush timestamp via PERF_RECORD_FINISHED_ROUND event, so we could get out of order event.

Do not force error in this case plus changing the output warning to use WARN_ONCE.

---
 tools/perf/util/ordered-events.c |  4 ++++
 tools/perf/util/ordered-events.h |  2 ++
 tools/perf/util/session.c        | 12 +++++++++---
 3 files changed, 15 insertions(+), 3 deletions(-)
tools/perf/util/ordered-events.c +4 -0
index 8a8aeab8e7ec..706ce1a66169 100644
@@ -169,6 +169,7 @@ int ordered_events__flush(struct perf_session *s, struct perf_tool *tool,
 {
 	struct ordered_events *oe = &s->ordered_events;
 	static const char * const str[] = {
+		"NONE",
 		"FINAL",
 		"ROUND",
 		"HALF ",
@@ -198,6 +199,7 @@ int ordered_events__flush(struct perf_session *s, struct perf_tool *tool,
 	}
 
 	case OE_FLUSH__ROUND:
+	case OE_FLUSH__NONE:
 	default:
 		break;
 	};
@@ -211,6 +213,8 @@ int ordered_events__flush(struct perf_session *s, struct perf_tool *tool,
 	if (!err) {
 		if (how == OE_FLUSH__ROUND)
 			oe->next_flush = oe->max_timestamp;
+
+		oe->last_flush_type = how;
 	}
 
 	pr_oe_time(oe->next_flush, "next_flush - ordered_events__flush POST %s, nr_events %u\n",
tools/perf/util/ordered-events.h +2 -0
index 2ce847fb48a5..3b2f20542a01 100644
@@ -14,6 +14,7 @@ struct ordered_event {
 };
 
 enum oe_flush {
+	OE_FLUSH__NONE,
 	OE_FLUSH__FINAL,
 	OE_FLUSH__ROUND,
 	OE_FLUSH__HALF,
@@ -32,6 +33,7 @@ struct ordered_events {
 	struct ordered_event	*last;
 	int			buffer_idx;
 	unsigned int		nr_events;
+	enum oe_flush		last_flush_type;
 };
 
 struct ordered_event *ordered_events__new(struct ordered_events *oe, u64 timestamp);
tools/perf/util/session.c +9 -3
index 462be2749462..7e27f1eb260c 100644
@@ -14,6 +14,7 @@
 #include "util.h"
 #include "cpumap.h"
 #include "perf_regs.h"
+#include "asm/bug.h"
 
 static int perf_session__open(struct perf_session *session)
 {
@@ -500,10 +501,15 @@ int perf_session_queue_event(struct perf_session *s, union perf_event *event,
 		return -ETIME;
 
 	if (timestamp < oe->last_flush) {
-		printf("Warning: Timestamp below last timeslice flush\n");
+		WARN_ONCE(1, "Timestamp below last timeslice flush\n");
+
 		pr_oe_time(timestamp,      "out of order event");
-		pr_oe_time(oe->last_flush, "last flush");
-		return -EINVAL;
+		pr_oe_time(oe->last_flush, "last flush, last_flush_type %d\n",
+			   oe->last_flush_type);
+
+		/* We could get out of order messages after forced flush. */
+		if (oe->last_flush_type != OE_FLUSH__HALF)
+			return -EINVAL;
 	}
 
 	new = ordered_events__new(oe, timestamp);
Signature · 1 line
1.9.3
author maintainer
#29 lore raw

PATCH 22/33 perf tools: Show better error message in case we fail to open counters due to EBUSY error

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

Showing better error message in case we fail to open counters due to the EBUSY error. If we detect oprofile daemon process running, we now display following message for EBUSY error:

  $ perf record ls
  Error:
  The PMU counters are busy/taken by another profiler.
  We found oprofile daemon running, please stop it and try again.

In case oprofiled was not detected the current error message stays:

  $ perf record ls
  Error:
  The sys_perf_event_open() syscall returned with 16 (Device or resource busy) for event (cycles).
  /bin/dmesg may provide additional information.
  No CONFIG_PERF_EVENTS=y kernel support configured?

Also changing PERF_FLAG_FD_CLOEXEC detection code not to display error in case of EBUSY error, as it currently does:

  $ perf record ls
  Error:
  perf_event_open(..., PERF_FLAG_FD_CLOEXEC) failed with unexpected error 16 (Device or resource busy)
  perf_event_open(..., 0) failed unexpectedly with error 16 (Device or resource busy)
  The PMU counters are busy/taken by another profiler.
  We found oprofile daemon running, please stop it and try again.
Cc Adrian Hunter <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
Cc William Cohen <[email protected]>
Cc Yann Droneaud <[email protected]>
---
 tools/perf/util/cloexec.c |  4 ++--
 tools/perf/util/evsel.c   |  6 ++++++
 tools/perf/util/util.c    | 36 ++++++++++++++++++++++++++++++++++++
 tools/perf/util/util.h    |  1 +
 4 files changed, 45 insertions(+), 2 deletions(-)
tools/perf/util/cloexec.c +2 -2
index c5d05ec17220..dede0c388ed4 100644
@@ -25,7 +25,7 @@ static int perf_flag_probe(void)
 		return 1;
 	}
 
-	WARN_ONCE(err != EINVAL,
+	WARN_ONCE(err != EINVAL && err != EBUSY,
 		  "perf_event_open(..., PERF_FLAG_FD_CLOEXEC) failed with unexpected error %d (%s)\n",
 		  err, strerror(err));
 
@@ -33,7 +33,7 @@ static int perf_flag_probe(void)
 	fd = sys_perf_event_open(&attr, 0, -1, -1, 0);
 	err = errno;
 
-	if (WARN_ONCE(fd < 0,
+	if (WARN_ONCE(fd < 0 && err != EBUSY,
 		      "perf_event_open(..., 0) failed unexpectedly with error %d (%s)\n",
 		      err, strerror(err)))
 		return -1;
tools/perf/util/evsel.c +6 -0
index 92e5235f5377..0c8919decac8 100644
@@ -2039,6 +2039,12 @@ int perf_evsel__open_strerror(struct perf_evsel *evsel, struct target *target,
 	"No APIC? If so then you can boot the kernel with the \"lapic\" boot parameter to force-enable it.");
 #endif
 		break;
+	case EBUSY:
+		if (find_process("oprofiled"))
+			return scnprintf(msg, size,
+	"The PMU counters are busy/taken by another profiler.\n"
+	"We found oprofile daemon running, please stop it and try again.");
+		break;
 	default:
 		break;
 	}
tools/perf/util/util.c +36 -0
index e52e7461911b..b82a93cb1694 100644
@@ -536,3 +536,39 @@ void mem_bswap_64(void *src, int byte_size)
 		++m;
 	}
 }
+
+bool find_process(const char *name)
+{
+	size_t len = strlen(name);
+	DIR *dir;
+	struct dirent *d;
+	int ret = -1;
+
+	dir = opendir(procfs__mountpoint());
+	if (!dir)
+		return -1;
+
+	/* Walk through the directory. */
+	while (ret && (d = readdir(dir)) != NULL) {
+		char path[PATH_MAX];
+		char *data;
+		size_t size;
+
+		if ((d->d_type != DT_DIR) ||
+		     !strcmp(".", d->d_name) ||
+		     !strcmp("..", d->d_name))
+			continue;
+
+		scnprintf(path, sizeof(path), "%s/%s/comm",
+			  procfs__mountpoint(), d->d_name);
+
+		if (filename__read_str(path, &data, &size))
+			continue;
+
+		ret = strncmp(name, data, len);
+		free(data);
+	}
+
+	closedir(dir);
+	return ret ? false : true;
+}
tools/perf/util/util.h +1 -0
index 38af77550c75..03a1ea2266b8 100644
@@ -346,4 +346,5 @@ void mem_bswap_64(void *src, int byte_size);
 void mem_bswap_32(void *src, int byte_size);
 
 const char *get_filename_for_perf_kvm(void);
+bool find_process(const char *name);
 #endif /* GIT_COMPAT_UTIL_H */
Signature · 1 line
1.9.3
author maintainer
#30 lore raw

PATCH 23/33 perf kmem: Do not ignore mmap events

In reply to Arnaldo Carvalho de Melo

From: Namhyung Kim <[email protected]>

The perf kmem command didn't process mmap events for some unknown reason and it instead gets symbol info from a running kernel. This is problematic if perf kmem record was run on a different kernel.

This patch adds the mmap event handlers and reverts the commit e727ca73f85d ("perf kmem: Resolve kernel symbols again").

Cc Ingo Molnar <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
[ Fixed up merge conflict with Jiri's ordered_events rename patch set ]
---
 tools/perf/builtin-kmem.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
tools/perf/builtin-kmem.c +2 -3
index b5721116713b..84b82397a28e 100644
@@ -256,6 +256,8 @@ static int process_sample_event(struct perf_tool *tool __maybe_unused,
 static struct perf_tool perf_kmem = {
 	.sample		 = process_sample_event,
 	.comm		 = perf_event__process_comm,
+	.mmap		 = perf_event__process_mmap,
+	.mmap2		 = perf_event__process_mmap2,
 	.ordered_events	 = true,
 };
 
@@ -424,9 +426,6 @@ static int __cmd_kmem(void)
 	if (session == NULL)
 		return -ENOMEM;
 
-	if (perf_session__create_kernel_maps(session) < 0)
-		goto out_delete;
-
 	if (!perf_session__has_traces(session, "kmem record"))
 		goto out_delete;
 
Signature · 1 line
1.9.3
author maintainer
#31 lore raw

PATCH 24/33 perf tools: Fix make PYTHON override

In reply to Arnaldo Carvalho de Melo

From: Namhyung Kim <[email protected]>

Thomas reported that make PYTHON=python2 is not work on some systems. I can reproduce it on my ArchLinux box too.

This is because it's overridden by config/feature-checks/Makefile regardless of PYTHON setting. I guess it's a bug slipped into during the feature checking change.

Actually, we don't need to check python-config in the feature-checks. We can just pass appropriate FEATURE_CHECK_*FLAGS.

---
 tools/perf/config/Makefile                | 34 ++++++++++++++++++++-----------
 tools/perf/config/feature-checks/Makefile | 18 ++--------------
 2 files changed, 24 insertions(+), 28 deletions(-)
tools/perf/config/Makefile +22 -12
index 1f67aa02d240..e05d8f99424d 100644
@@ -120,6 +120,23 @@ ifdef PARSER_DEBUG
   CFLAGS             += -DPARSER_DEBUG
 endif
 
+ifndef NO_LIBPYTHON
+  override PYTHON := \
+    $(call get-executable-or-default,PYTHON,python)
+  override PYTHON_CONFIG := \
+    $(call get-executable-or-default,PYTHON_CONFIG,$(PYTHON)-config)
+
+  PYTHON_CONFIG_SQ := $(call shell-sq,$(PYTHON_CONFIG))
+
+  PYTHON_EMBED_LDOPTS := $(shell $(PYTHON_CONFIG_SQ) --ldflags 2>/dev/null)
+  PYTHON_EMBED_CCOPTS := $(shell $(PYTHON_CONFIG_SQ) --cflags 2>/dev/null)
+
+  FEATURE_CHECK_CFLAGS-libpython := $(PYTHON_EMBED_CCOPTS)
+  FEATURE_CHECK_LDFLAGS-libpython := $(PYTHON_EMBED_LDOPTS)
+  FEATURE_CHECK_CFLAGS-libpython-version := $(PYTHON_EMBED_CCOPTS)
+  FEATURE_CHECK_LDFLAGS-libpython-version := $(PYTHON_EMBED_LDOPTS)
+endif
+
 CFLAGS += -fno-omit-frame-pointer
 CFLAGS += -ggdb3
 CFLAGS += -funwind-tables
@@ -482,21 +499,14 @@ define disable-python_code
   NO_LIBPYTHON := 1
 endef
 
-override PYTHON := \
-  $(call get-executable-or-default,PYTHON,python)
-
-ifndef PYTHON
-  $(call disable-python,python interpreter)
+ifdef NO_LIBPYTHON
+  $(call disable-python)
 else
 
-  PYTHON_WORD := $(call shell-wordify,$(PYTHON))
-
-  ifdef NO_LIBPYTHON
-    $(call disable-python)
+  ifndef PYTHON
+    $(call disable-python,python interpreter)
   else
-
-    override PYTHON_CONFIG := \
-      $(call get-executable-or-default,PYTHON_CONFIG,$(PYTHON)-config)
+    PYTHON_WORD := $(call shell-wordify,$(PYTHON))
 
     ifndef PYTHON_CONFIG
       $(call disable-python,python-config tool)
tools/perf/config/feature-checks/Makefile +2 -16
index 6088f8d8a434..72ab2984718e 100644
@@ -101,25 +101,11 @@ FLAGS_PERL_EMBED=$(PERL_EMBED_CCOPTS) $(PERL_EMBED_LDOPTS)
 test-libperl.bin:
 	$(BUILD) $(FLAGS_PERL_EMBED)
 
-override PYTHON := python
-override PYTHON_CONFIG := python-config
-
-escape-for-shell-sq =  $(subst ','\'',$(1))
-shell-sq = '$(escape-for-shell-sq)'
-
-PYTHON_CONFIG_SQ = $(call shell-sq,$(PYTHON_CONFIG))
-
-PYTHON_EMBED_LDOPTS = $(shell $(PYTHON_CONFIG_SQ) --ldflags 2>/dev/null)
-PYTHON_EMBED_LDFLAGS = $(call strip-libs,$(PYTHON_EMBED_LDOPTS))
-PYTHON_EMBED_LIBADD = $(call grep-libs,$(PYTHON_EMBED_LDOPTS))
-PYTHON_EMBED_CCOPTS = $(shell $(PYTHON_CONFIG_SQ) --cflags 2>/dev/null)
-FLAGS_PYTHON_EMBED = $(PYTHON_EMBED_CCOPTS) $(PYTHON_EMBED_LDOPTS)
-
 test-libpython.bin:
-	$(BUILD) $(FLAGS_PYTHON_EMBED)
+	$(BUILD)
 
 test-libpython-version.bin:
-	$(BUILD) $(FLAGS_PYTHON_EMBED)
+	$(BUILD)
 
 test-libbfd.bin:
 	$(BUILD) -DPACKAGE='"perf"' -lbfd -lz -liberty -ldl
Signature · 1 line
1.9.3
author maintainer
#32 lore raw

PATCH 25/33 perf tools: Left-align output contents

In reply to Arnaldo Carvalho de Melo

From: Namhyung Kim <[email protected]>

Now perf left-aligns column headers but the contents does not. It should have same alignment. This requires a change in pid sort key - it consists of two part (pid and comm). As length of comm can be vary it'd be better to change the order of them.

Thanks to Jiri Olsa for pointing this out.

---
 tools/perf/util/sort.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
tools/perf/util/sort.c +5 -5
index 14e5a039bc45..eda9ee836cee 100644
@@ -70,12 +70,12 @@ static int hist_entry__thread_snprintf(struct hist_entry *he, char *bf,
 				       size_t size, unsigned int width)
 {
 	const char *comm = thread__comm_str(he->thread);
-	return repsep_snprintf(bf, size, "%*s:%5d", width - 6,
-			       comm ?: "", he->thread->tid);
+	return repsep_snprintf(bf, size, "%5d:%-*s", he->thread->tid,
+			       width - 6, comm ?: "");
 }
 
 struct sort_entry sort_thread = {
-	.se_header	= "Command:  Pid",
+	.se_header	= "  Pid:Command",
 	.se_cmp		= sort__thread_cmp,
 	.se_snprintf	= hist_entry__thread_snprintf,
 	.se_width_idx	= HISTC_THREAD,
@@ -106,7 +106,7 @@ sort__comm_sort(struct hist_entry *left, struct hist_entry *right)
 static int hist_entry__comm_snprintf(struct hist_entry *he, char *bf,
 				     size_t size, unsigned int width)
 {
-	return repsep_snprintf(bf, size, "%*s", width, comm__str(he->comm));
+	return repsep_snprintf(bf, size, "%-*s", width, comm__str(he->comm));
 }
 
 struct sort_entry sort_comm = {
@@ -305,7 +305,7 @@ static int hist_entry__srcline_snprintf(struct hist_entry *he, char *bf,
 					size_t size,
 					unsigned int width __maybe_unused)
 {
-	return repsep_snprintf(bf, size, "%s", he->srcline);
+	return repsep_snprintf(bf, size, "%-s", he->srcline);
 }
 
 struct sort_entry sort_srcline = {
Signature · 1 line
1.9.3
author maintainer
#33 lore raw

PATCH 26/33 perf tools: Make __hpp__fmt() receive an additional len argument

In reply to Arnaldo Carvalho de Melo

From: Namhyung Kim <[email protected]>

So that it can properly handle alignment requirements later. To do that, add percent_color_len_snprintf() fucntion to help coloring of overhead columns.

---
 tools/perf/ui/browsers/hists.c | 14 ++++++++------
 tools/perf/ui/gtk/hists.c      |  8 +++++---
 tools/perf/ui/hist.c           | 43 +++++++++++++++++++++---------------------
 tools/perf/util/color.c        | 16 ++++++++++++++++
 tools/perf/util/color.h        |  1 +
 tools/perf/util/hist.h         |  4 ++--
 6 files changed, 54 insertions(+), 32 deletions(-)
tools/perf/ui/browsers/hists.c +8 -6
index a94b11fc5e00..02507ba944e3 100644
@@ -653,17 +653,18 @@ struct hpp_arg {
 static int __hpp__slsmg_color_printf(struct perf_hpp *hpp, const char *fmt, ...)
 {
 	struct hpp_arg *arg = hpp->ptr;
-	int ret;
+	int ret, len;
 	va_list args;
 	double percent;
 
 	va_start(args, fmt);
+	len = va_arg(args, int);
 	percent = va_arg(args, double);
 	va_end(args);
 
 	ui_browser__set_percent_color(arg->b, percent, arg->current_entry);
 
-	ret = scnprintf(hpp->buf, hpp->size, fmt, percent);
+	ret = scnprintf(hpp->buf, hpp->size, fmt, len, percent);
 	slsmg_printf("%s", hpp->buf);
 
 	advance_hpp(hpp, ret);
@@ -681,7 +682,7 @@ hist_browser__hpp_color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,\
 				struct perf_hpp *hpp,			\
 				struct hist_entry *he)			\
 {									\
-	return __hpp__fmt(hpp, he, __hpp_get_##_field, " %6.2f%%",	\
+	return __hpp__fmt(hpp, he, __hpp_get_##_field, " %*.2f%%", 6,	\
 			  __hpp__slsmg_color_printf, true);		\
 }
 
@@ -697,13 +698,14 @@ hist_browser__hpp_color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,\
 				struct hist_entry *he)			\
 {									\
 	if (!symbol_conf.cumulate_callchain) {				\
-		int ret = scnprintf(hpp->buf, hpp->size, "%8s", "N/A");	\
+		int ret = scnprintf(hpp->buf, hpp->size,		\
+				    "%*s", 8, "N/A");			\
 		slsmg_printf("%s", hpp->buf);				\
 									\
 		return ret;						\
 	}								\
-	return __hpp__fmt(hpp, he, __hpp_get_acc_##_field, " %6.2f%%",	\
-			  __hpp__slsmg_color_printf, true);		\
+	return __hpp__fmt(hpp, he, __hpp_get_acc_##_field, " %*.2f%%",	\
+			  6, __hpp__slsmg_color_printf, true);	\
 }
 
 __HPP_COLOR_PERCENT_FN(overhead, period)
tools/perf/ui/gtk/hists.c +5 -3
index 6ca60e482cdc..91f6cd7d2312 100644
@@ -11,6 +11,7 @@
 static int __percent_color_snprintf(struct perf_hpp *hpp, const char *fmt, ...)
 {
 	int ret = 0;
+	int len;
 	va_list args;
 	double percent;
 	const char *markup;
@@ -18,6 +19,7 @@ static int __percent_color_snprintf(struct perf_hpp *hpp, const char *fmt, ...)
 	size_t size = hpp->size;
 
 	va_start(args, fmt);
+	len = va_arg(args, int);
 	percent = va_arg(args, double);
 	va_end(args);
 
@@ -25,7 +27,7 @@ static int __percent_color_snprintf(struct perf_hpp *hpp, const char *fmt, ...)
 	if (markup)
 		ret += scnprintf(buf, size, markup);
 
-	ret += scnprintf(buf + ret, size - ret, fmt, percent);
+	ret += scnprintf(buf + ret, size - ret, fmt, len, percent);
 
 	if (markup)
 		ret += scnprintf(buf + ret, size - ret, "</span>");
@@ -43,7 +45,7 @@ static int perf_gtk__hpp_color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,
 				       struct perf_hpp *hpp,			\
 				       struct hist_entry *he)			\
 {										\
-	return __hpp__fmt(hpp, he, he_get_##_field, " %6.2f%%",			\
+	return __hpp__fmt(hpp, he, he_get_##_field, " %*.2f%%", 6,		\
 			  __percent_color_snprintf, true);			\
 }
 
@@ -57,7 +59,7 @@ static int perf_gtk__hpp_color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,
 				       struct perf_hpp *hpp,			\
 				       struct hist_entry *he)			\
 {										\
-	return __hpp__fmt_acc(hpp, he, he_get_acc_##_field, " %6.2f%%",		\
+	return __hpp__fmt_acc(hpp, he, he_get_acc_##_field, " %*.2f%%", 6, 	\
 			      __percent_color_snprintf, true);			\
 }
 
tools/perf/ui/hist.c +22 -21

130 lines not on this page · raw message

tools/perf/util/color.c +16 -0
index 87b8672eb413..f4654183d391 100644
@@ -335,3 +335,19 @@ int percent_color_snprintf(char *bf, size_t size, const char *fmt, ...)
 	va_end(args);
 	return value_color_snprintf(bf, size, fmt, percent);
 }
+
+int percent_color_len_snprintf(char *bf, size_t size, const char *fmt, ...)
+{
+	va_list args;
+	int len;
+	double percent;
+	const char *color;
+
+	va_start(args, fmt);
+	len = va_arg(args, int);
+	percent = va_arg(args, double);
+	va_end(args);
+
+	color = get_percent_color(percent);
+	return color_snprintf(bf, size, color, fmt, len, percent);
+}
tools/perf/util/color.h +1 -0
index 7ff30a62a132..0a594b8a0c26 100644
@@ -41,6 +41,7 @@ int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...);
 int color_fwrite_lines(FILE *fp, const char *color, size_t count, const char *buf);
 int value_color_snprintf(char *bf, size_t size, const char *fmt, double value);
 int percent_color_snprintf(char *bf, size_t size, const char *fmt, ...);
+int percent_color_len_snprintf(char *bf, size_t size, const char *fmt, ...);
 int percent_color_fprintf(FILE *fp, const char *fmt, double percent);
 const char *get_percent_color(double percent);
 
tools/perf/util/hist.h +2 -2
index 742f49a85725..13d074db9ef1 100644
@@ -267,10 +267,10 @@ typedef int (*hpp_callback_fn)(struct perf_hpp *hpp, bool front);
 typedef int (*hpp_snprint_fn)(struct perf_hpp *hpp, const char *fmt, ...);
 
 int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
-	       hpp_field_fn get_field, const char *fmt,
+	       hpp_field_fn get_field, const char *fmt, int len,
 	       hpp_snprint_fn print_fn, bool fmt_percent);
 int __hpp__fmt_acc(struct perf_hpp *hpp, struct hist_entry *he,
-		   hpp_field_fn get_field, const char *fmt,
+		   hpp_field_fn get_field, const char *fmt, int len,
 		   hpp_snprint_fn print_fn, bool fmt_percent);
 
 static inline void advance_hpp(struct perf_hpp *hpp, int inc)
Signature · 1 line
1.9.3
author maintainer
#34 lore raw

PATCH 27/33 perf tools: Save column length in perf_hpp_fmt

In reply to Arnaldo Carvalho de Melo

From: Namhyung Kim <[email protected]>

Save column length in the hpp format and pass it to print functions. This is a preparation for users to control column width in the output.

---
 tools/perf/ui/browsers/hists.c |   2 +-
 tools/perf/ui/hist.c           | 135 +++++++++++++++++++++++++++--------------
 tools/perf/util/hist.h         |   2 +
 tools/perf/util/sort.c         |   3 +-
 4 files changed, 94 insertions(+), 48 deletions(-)
tools/perf/ui/browsers/hists.c +1 -1
index 02507ba944e3..c1d8d3925fe1 100644
@@ -849,7 +849,7 @@ static int hists__scnprintf_headers(char *buf, size_t size, struct hists *hists)
 		if (perf_hpp__should_skip(fmt))
 			continue;
 
-		/* We need to add the length of the columns header. */
+		/* We need to ensure length of the columns header. */
 		perf_hpp__reset_width(fmt, hists);
 
 		ret = fmt->header(fmt, &dummy_hpp, hists_to_evsel(hists));
tools/perf/ui/hist.c +89 -46

226 lines not on this page · raw message

tools/perf/util/hist.h +2 -0
index 13d074db9ef1..a7ae890f4c71 100644
@@ -207,6 +207,7 @@ struct perf_hpp_fmt {
 	struct list_head list;
 	struct list_head sort_list;
 	bool elide;
+	int len;
 };
 
 extern struct list_head perf_hpp__list;
@@ -261,6 +262,7 @@ static inline bool perf_hpp__should_skip(struct perf_hpp_fmt *format)
 }
 
 void perf_hpp__reset_width(struct perf_hpp_fmt *fmt, struct hists *hists);
+void perf_hpp__reset_sort_width(struct perf_hpp_fmt *fmt, struct hists *hists);
 
 typedef u64 (*hpp_field_fn)(struct hist_entry *he);
 typedef int (*hpp_callback_fn)(struct perf_hpp *hpp, bool front);
tools/perf/util/sort.c +2 -1
index eda9ee836cee..a7f8a7b0eced 100644
@@ -1194,7 +1194,7 @@ bool perf_hpp__same_sort_entry(struct perf_hpp_fmt *a, struct perf_hpp_fmt *b)
 	return hse_a->se == hse_b->se;
 }
 
-void perf_hpp__reset_width(struct perf_hpp_fmt *fmt, struct hists *hists)
+void perf_hpp__reset_sort_width(struct perf_hpp_fmt *fmt, struct hists *hists)
 {
 	struct hpp_sort_entry *hse;
 
@@ -1265,6 +1265,7 @@ __sort_dimension__alloc_hpp(struct sort_dimension *sd)
 	INIT_LIST_HEAD(&hse->hpp.list);
 	INIT_LIST_HEAD(&hse->hpp.sort_list);
 	hse->hpp.elide = false;
+	hse->hpp.len = 0;
 
 	return hse;
 }
Signature · 1 line
1.9.3
author maintainer
#35 lore raw

PATCH 28/33 perf report: Honor column width setting

In reply to Arnaldo Carvalho de Melo

From: Namhyung Kim <[email protected]>

Set column width and do not change it if user gives -w/--column-widths option. It'll truncate longer symbols than the width if exists.

---
 tools/perf/ui/browsers/hists.c | 18 +++++----
 tools/perf/ui/gtk/hists.c      | 10 ++---
 tools/perf/ui/hist.c           | 84 +++++++++++++++++++++++++++++-------------
 tools/perf/ui/stdio/hist.c     |  4 +-
 tools/perf/util/hist.h         | 14 ++++---
 tools/perf/util/sort.c         | 49 +++++++++++++++---------
 6 files changed, 116 insertions(+), 63 deletions(-)
tools/perf/ui/browsers/hists.c +11 -7
index c1d8d3925fe1..e07d4e848d5c 100644
@@ -678,12 +678,12 @@ static u64 __hpp_get_##_field(struct hist_entry *he)			\
 }									\
 									\
 static int								\
-hist_browser__hpp_color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,\
+hist_browser__hpp_color_##_type(struct perf_hpp_fmt *fmt,		\
 				struct perf_hpp *hpp,			\
 				struct hist_entry *he)			\
 {									\
-	return __hpp__fmt(hpp, he, __hpp_get_##_field, " %*.2f%%", 6,	\
-			  __hpp__slsmg_color_printf, true);		\
+	return hpp__fmt(fmt, hpp, he, __hpp_get_##_field, " %*.2f%%",	\
+			__hpp__slsmg_color_printf, true);		\
 }
 
 #define __HPP_COLOR_ACC_PERCENT_FN(_type, _field)			\
@@ -693,19 +693,20 @@ static u64 __hpp_get_acc_##_field(struct hist_entry *he)		\
 }									\
 									\
 static int								\
-hist_browser__hpp_color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,\
+hist_browser__hpp_color_##_type(struct perf_hpp_fmt *fmt,		\
 				struct perf_hpp *hpp,			\
 				struct hist_entry *he)			\
 {									\
 	if (!symbol_conf.cumulate_callchain) {				\
+		int len = fmt->user_len ?: fmt->len;			\
 		int ret = scnprintf(hpp->buf, hpp->size,		\
-				    "%*s", 8, "N/A");			\
+				    "%*s", len, "N/A");			\
 		slsmg_printf("%s", hpp->buf);				\
 									\
 		return ret;						\
 	}								\
-	return __hpp__fmt(hpp, he, __hpp_get_acc_##_field, " %*.2f%%",	\
-			  6, __hpp__slsmg_color_printf, true);	\
+	return hpp__fmt(fmt, hpp, he, __hpp_get_acc_##_field,		\
+			" %*.2f%%", __hpp__slsmg_color_printf, true);	\
 }
 
 __HPP_COLOR_PERCENT_FN(overhead, period)
@@ -1549,6 +1550,9 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 
 	memset(options, 0, sizeof(options));
 
+	if (symbol_conf.col_width_list_str)
+		perf_hpp__set_user_width(symbol_conf.col_width_list_str);
+
 	while (1) {
 		const struct thread *thread = NULL;
 		const struct dso *dso = NULL;
tools/perf/ui/gtk/hists.c +5 -5
index 91f6cd7d2312..897b2e140428 100644
@@ -41,12 +41,12 @@ static u64 he_get_##_field(struct hist_entry *he)				\
 	return he->stat._field;							\
 }										\
 										\
-static int perf_gtk__hpp_color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,	\
+static int perf_gtk__hpp_color_##_type(struct perf_hpp_fmt *fmt,		\
 				       struct perf_hpp *hpp,			\
 				       struct hist_entry *he)			\
 {										\
-	return __hpp__fmt(hpp, he, he_get_##_field, " %*.2f%%", 6,		\
-			  __percent_color_snprintf, true);			\
+	return hpp__fmt(fmt, hpp, he, he_get_##_field, " %*.2f%%",		\
+			__percent_color_snprintf, true);			\
 }
 
 #define __HPP_COLOR_ACC_PERCENT_FN(_type, _field)				\
@@ -59,8 +59,8 @@ static int perf_gtk__hpp_color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,
 				       struct perf_hpp *hpp,			\
 				       struct hist_entry *he)			\
 {										\
-	return __hpp__fmt_acc(hpp, he, he_get_acc_##_field, " %*.2f%%", 6, 	\
-			      __percent_color_snprintf, true);			\
+	return hpp__fmt_acc(fmt, hpp, he, he_get_acc_##_field, " %*.2f%%", 	\
+			    __percent_color_snprintf, true);			\
 }
 
 __HPP_COLOR_PERCENT_FN(overhead, period)
tools/perf/ui/hist.c +58 -26

141 lines not on this page · raw message

tools/perf/ui/stdio/hist.c +3 -1
index 40af0acb4fe9..15b451acbde6 100644
@@ -395,10 +395,12 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
 
 	init_rem_hits();
 
-
 	perf_hpp__for_each_format(fmt)
 		perf_hpp__reset_width(fmt, hists);
 
+	if (symbol_conf.col_width_list_str)
+		perf_hpp__set_user_width(symbol_conf.col_width_list_str);
+
 	if (!show_header)
 		goto print_entries;
 
tools/perf/util/hist.h +8 -6
index a7ae890f4c71..2e70003fcd00 100644
@@ -208,6 +208,7 @@ struct perf_hpp_fmt {
 	struct list_head sort_list;
 	bool elide;
 	int len;
+	int user_len;
 };
 
 extern struct list_head perf_hpp__list;
@@ -263,17 +264,18 @@ static inline bool perf_hpp__should_skip(struct perf_hpp_fmt *format)
 
 void perf_hpp__reset_width(struct perf_hpp_fmt *fmt, struct hists *hists);
 void perf_hpp__reset_sort_width(struct perf_hpp_fmt *fmt, struct hists *hists);
+void perf_hpp__set_user_width(const char *width_list_str);
 
 typedef u64 (*hpp_field_fn)(struct hist_entry *he);
 typedef int (*hpp_callback_fn)(struct perf_hpp *hpp, bool front);
 typedef int (*hpp_snprint_fn)(struct perf_hpp *hpp, const char *fmt, ...);
 
-int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
-	       hpp_field_fn get_field, const char *fmt, int len,
-	       hpp_snprint_fn print_fn, bool fmt_percent);
-int __hpp__fmt_acc(struct perf_hpp *hpp, struct hist_entry *he,
-		   hpp_field_fn get_field, const char *fmt, int len,
-		   hpp_snprint_fn print_fn, bool fmt_percent);
+int hpp__fmt(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
+	     struct hist_entry *he, hpp_field_fn get_field,
+	     const char *fmtstr, hpp_snprint_fn print_fn, bool fmt_percent);
+int hpp__fmt_acc(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
+		 struct hist_entry *he, hpp_field_fn get_field,
+		 const char *fmtstr, hpp_snprint_fn print_fn, bool fmt_percent);
 
 static inline void advance_hpp(struct perf_hpp *hpp, int inc)
 {
tools/perf/util/sort.c +31 -18

132 lines not on this page · raw message

Signature · 1 line
1.9.3
author maintainer
#36 lore raw

PATCH 29/33 perf top: Add -w option for setting column width

In reply to Arnaldo Carvalho de Melo

From: Namhyung Kim <[email protected]>

Add -w/--column-widths option like perf report does so that users are able to see symbols even with some very long C++ library/functions.

It can be a list separated by comma for each column.

  $ perf top -w 0,20,30

The value of 0 means there's no limit.

---
 tools/perf/Documentation/perf-report.txt | 2 +-
 tools/perf/Documentation/perf-top.txt    | 6 ++++++
 tools/perf/builtin-top.c                 | 3 +++
 3 files changed, 10 insertions(+), 1 deletion(-)
tools/perf/Documentation/perf-report.txt +1 -1
index d2b59af62bc0..d561e0214f52 100644
@@ -147,7 +147,7 @@ OPTIONS
 -w::
 --column-widths=<width[,width...]>::
 	Force each column width to the provided list, for large terminal
-	readability.
+	readability.  0 means no limit (default behavior).
 
 -t::
 --field-separator=::
tools/perf/Documentation/perf-top.txt +6 -0
index 180ae02137a5..28fdee394880 100644
@@ -193,6 +193,12 @@ Default is to monitor all CPUS.
 	sum of shown entries will be always 100%. "absolute" means it retains
 	the original value before and after the filter is applied.
 
+-w::
+--column-widths=<width[,width...]>::
+	Force each column width to the provided list, for large terminal
+	readability.  0 means no limit (default behavior).
+
+
 INTERACTIVE PROMPTING KEYS
 --------------------------
 
tools/perf/builtin-top.c +3 -0
index 377971dc89a3..bde216b2071c 100644
@@ -1131,6 +1131,9 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
 		     "Don't show entries under that percent", parse_percent_limit),
 	OPT_CALLBACK(0, "percentage", NULL, "relative|absolute",
 		     "How to display percentage of filtered entries", parse_filter_percentage),
+	OPT_STRING('w', "column-widths", &symbol_conf.col_width_list_str,
+		   "width[,width...]",
+		   "don't try to adjust column width, use these fixed values"),
 	OPT_END()
 	};
 	const char * const top_usage[] = {
Signature · 1 line
1.9.3
author maintainer
#37 lore raw

PATCH 30/33 perf tools: Add name field into perf_hpp_fmt

In reply to Arnaldo Carvalho de Melo

From: Namhyung Kim <[email protected]>

It makes the code a bit simpler and easier to debug IMHO.

I guess it can also remove similar code in perf diff, but let's keep it for a future work. :)

---
 tools/perf/ui/gtk/hists.c |   4 +-
 tools/perf/ui/hist.c      | 136 +++++++++++++++++++++-------------------------
 tools/perf/util/hist.h    |   1 +
 tools/perf/util/sort.c    |   6 +-
 4 files changed, 66 insertions(+), 81 deletions(-)
tools/perf/ui/gtk/hists.c +1 -3
index 897b2e140428..f3fa4258b256 100644
@@ -207,10 +207,8 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists,
 		if (perf_hpp__is_sort_entry(fmt))
 			sym_col = col_idx;
 
-		fmt->header(fmt, &hpp, hists_to_evsel(hists));
-
 		gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
-							    -1, ltrim(s),
+							    -1, fmt->name,
 							    renderer, "markup",
 							    col_idx++, NULL);
 	}
tools/perf/ui/hist.c +61 -75

194 lines not on this page · raw message

tools/perf/util/hist.h +1 -0
index 2e70003fcd00..95405a8fbd95 100644
@@ -192,6 +192,7 @@ struct perf_hpp {
 };
 
 struct perf_hpp_fmt {
+	const char *name;
 	int (*header)(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
 		      struct perf_evsel *evsel);
 	int (*width)(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
tools/perf/util/sort.c +3 -3
index 153b3803f0b7..b4a805e5e440 100644
@@ -1206,8 +1206,7 @@ void perf_hpp__reset_sort_width(struct perf_hpp_fmt *fmt, struct hists *hists)
 		return;
 
 	hse = container_of(fmt, struct hpp_sort_entry, hpp);
-	hists__new_col_len(hists, hse->se->se_width_idx,
-			   strlen(hse->se->se_header));
+	hists__new_col_len(hists, hse->se->se_width_idx, strlen(fmt->name));
 }
 
 static int __sort__hpp_header(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
@@ -1221,7 +1220,7 @@ static int __sort__hpp_header(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
 	if (!len)
 		len = hists__col_len(&evsel->hists, hse->se->se_width_idx);
 
-	return scnprintf(hpp->buf, hpp->size, "%-*.*s", len, len, hse->se->se_header);
+	return scnprintf(hpp->buf, hpp->size, "%-*.*s", len, len, fmt->name);
 }
 
 static int __sort__hpp_width(struct perf_hpp_fmt *fmt,
@@ -1265,6 +1264,7 @@ __sort_dimension__alloc_hpp(struct sort_dimension *sd)
 	}
 
 	hse->se = sd->entry;
+	hse->hpp.name = sd->entry->se_header;
 	hse->hpp.header = __sort__hpp_header;
 	hse->hpp.width = __sort__hpp_width;
 	hse->hpp.entry = __sort__hpp_entry;
Signature · 1 line
1.9.3
author maintainer
#38 lore raw

PATCH 31/33 perf tools: Fix column alignment when headers aren't shown on TUI

In reply to Arnaldo Carvalho de Melo

From: Namhyung Kim <[email protected]>

If user sets ui.show-headers config option to false, it didn't calculate default column width so it broke the alignment. This is because it does the calculation just before showing headers.

Move it to the beginning of the hist browser so that it can be called regardless of the config option.

---
 tools/perf/ui/browsers/hists.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
tools/perf/ui/browsers/hists.c +4 -3
index e07d4e848d5c..045c1e16ac59 100644
@@ -850,9 +850,6 @@ static int hists__scnprintf_headers(char *buf, size_t size, struct hists *hists)
 		if (perf_hpp__should_skip(fmt))
 			continue;
 
-		/* We need to ensure length of the columns header. */
-		perf_hpp__reset_width(fmt, hists);
-
 		ret = fmt->header(fmt, &dummy_hpp, hists_to_evsel(hists));
 		if (advance_hpp_check(&dummy_hpp, ret))
 			break;
@@ -1501,6 +1498,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 	char buf[64];
 	char script_opt[64];
 	int delay_secs = hbt ? hbt->refresh : 0;
+	struct perf_hpp_fmt *fmt;
 
 #define HIST_BROWSER_HELP_COMMON					\
 	"h/?/F1        Show this window\n"				\
@@ -1550,6 +1548,9 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 
 	memset(options, 0, sizeof(options));
 
+	perf_hpp__for_each_format(fmt)
+		perf_hpp__reset_width(fmt, hists);
+
 	if (symbol_conf.col_width_list_str)
 		perf_hpp__set_user_width(symbol_conf.col_width_list_str);
 
Signature · 1 line
1.9.3
author maintainer
#39 lore raw

PATCH 32/33 perf tools: Fix PERF_FLAG_FD_CLOEXEC flag probing event type open counters due to EBUSY error

In reply to Arnaldo Carvalho de Melo

From: Jiri Olsa <[email protected]>

We were using PERF_COUNT_SW_CPU_CLOCK as an probing event type. Using expected PERF_TYPE_SOFTWARE type instead.

Cc Adrian Hunter <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
Cc William Cohen <[email protected]>
Cc Yann Droneaud <[email protected]>
---
 tools/perf/util/cloexec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/util/cloexec.c +1 -1
index dede0c388ed4..5073c01af618 100644
@@ -9,7 +9,7 @@ static int perf_flag_probe(void)
 {
 	/* use 'safest' configuration as used in perf_evsel__fallback() */
 	struct perf_event_attr attr = {
-		.type = PERF_COUNT_SW_CPU_CLOCK,
+		.type = PERF_TYPE_SOFTWARE,
 		.config = PERF_COUNT_SW_CPU_CLOCK,
 	};
 	int fd;
Signature · 1 line
1.9.3
author maintainer
#40 lore raw

PATCH 33/33 perf tools: Default to python version 2

In reply to Arnaldo Carvalho de Melo

From: Thomas Ilsche <[email protected]>

According to PEP 394 recommendation [1], it's more portable to use python2 rather than plain python to refer python binary version 2.

Since there're distros using python3 by default like Arch, and we don't support python3 (yet), it'd be better using python2 explicitly.

But older versions (prior to 2.7) seem not to provide python2 but just python. Given that it's only old version, try python2 first and then fallback to python. It'll ensure that it always points to python 2.x.

I tested (compiles and perf script runs) with the combinations:

1) python -> python2.x, python-config -> python2.x-config
python2 N/A, python2-config N/A

2) python -> python3.x, python-config -> python3.x-config python2 -> python2.x, python2-config -> python2.x-config

3) python -> python2.x, python-config -> python2.x-config python2 -> python2.x, python2-config -> python2.x-config

4) python -> python2.x, python-config -> python2.x-config python2 -> python2.x, python2-config N/A

Based on / replaces the patch 2/2 by Namhyung Kim.

[1] https://www.python.org/dev/peps/pep-0394

---
 tools/perf/config/Makefile | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)
tools/perf/config/Makefile +9 -3
index e05d8f99424d..75d4c237b03d 100644
@@ -121,10 +121,16 @@ ifdef PARSER_DEBUG
 endif
 
 ifndef NO_LIBPYTHON
-  override PYTHON := \
-    $(call get-executable-or-default,PYTHON,python)
+  # Try different combinations to accommodate systems that only have
+  # python[2][-config] in weird combinations but always preferring
+  # python2 and python2-config as per pep-0394. If we catch a
+  # python[-config] in version 3, the version check will kill it.
+  PYTHON2 := $(if $(call get-executable,python2),python2,python)
+  override PYTHON := $(call get-executable-or-default,PYTHON,$(PYTHON2))
+  PYTHON2_CONFIG := \
+    $(if $(call get-executable,$(PYTHON)-config),$(PYTHON)-config,python-config)
   override PYTHON_CONFIG := \
-    $(call get-executable-or-default,PYTHON_CONFIG,$(PYTHON)-config)
+    $(call get-executable-or-default,PYTHON_CONFIG,$(PYTHON2_CONFIG))
 
   PYTHON_CONFIG_SQ := $(call shell-sq,$(PYTHON_CONFIG))
 
Signature · 1 line
1.9.3