Skip to contentSkip to conversation

perf tools: Various fixes

PATCH Superseded Jiri Olsa 4 participants 13 messages Dec 17, 2015 – Jan 5, 2016 perf events

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Jiri Olsa

author reviewer
#0 lore raw

cover perf tools: Various fixes

hi,
sending several changes together:
- leftover for the stat enable/disable changes with Adrian's patch
- fixes for issues Noel found with DWARF unwind

Noel, could you please rerun your test on this?

Also available in:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/fixes

thanks,
jirka

---
Adrian Hunter (1):
perf tools: Make perf_evlist__open() open evsels with their cpus and threads (like perf record does)

Jiri Olsa (5):
perf tools: Remove perf_evlist__(enable|disable)_event functions perf tools: Use find_map function in access_dso_mem perf tools libunwind: Check for mmaps also in MAP__VARIABLE tree perf tools libdw: Check for mmaps also in MAP__VARIABLE tree perf record: Always store data mmaps

 tools/perf/arch/x86/util/intel-bts.c  |  4 ++--
 tools/perf/arch/x86/util/intel-pt.c   |  4 ++--
 tools/perf/builtin-kvm.c              |  2 +-
 tools/perf/builtin-record.c           |  2 +-
 tools/perf/builtin-top.c              |  2 +-
 tools/perf/builtin-trace.c            |  2 +-
 tools/perf/tests/code-reading.c       |  2 +-
 tools/perf/tests/dwarf-unwind.c       |  2 +-
 tools/perf/tests/keep-tracking.c      |  2 +-
 tools/perf/tests/mmap-thread-lookup.c |  4 ++--
 tools/perf/tests/switch-tracking.c    |  6 +++---
 tools/perf/util/event.c               | 16 ++++++----------
 tools/perf/util/event.h               |  5 ++---
 tools/perf/util/evlist.c              | 44 +-------------------------------------------
 tools/perf/util/evlist.h              |  4 ----
 tools/perf/util/machine.c             |  6 +++---
 tools/perf/util/machine.h             |  6 +++---
 tools/perf/util/unwind-libdw.c        | 10 ++++++++++
 tools/perf/util/unwind-libunwind.c    | 20 ++++++++++++++------
 19 files changed, 55 insertions(+), 88 deletions(-)
author reviewer
#1 lore raw

PATCH 1/6 perf tools: Remove perf_evlist__(enable|disable)_event functions

Replacing them with perf_evsel__(enable|disable).

Link http://lkml.kernel.org/n/[email protected] open here
Signed-off-by Jiri Olsa <[email protected]>
---
 tools/perf/arch/x86/util/intel-bts.c |  4 ++--
 tools/perf/arch/x86/util/intel-pt.c  |  4 ++--
 tools/perf/tests/keep-tracking.c     |  2 +-
 tools/perf/tests/switch-tracking.c   |  6 +++---
 tools/perf/util/evlist.c             | 42 ------------------------------------
 tools/perf/util/evlist.h             |  4 ----
 6 files changed, 8 insertions(+), 54 deletions(-)
tools/perf/arch/x86/util/intel-bts.c +2 -2
index 9b94ce520917..8d8150f1cf9b 100644
@@ -327,7 +327,7 @@ static int intel_bts_snapshot_start(struct auxtrace_record *itr)
 
 	evlist__for_each(btsr->evlist, evsel) {
 		if (evsel->attr.type == btsr->intel_bts_pmu->type)
-			return perf_evlist__disable_event(btsr->evlist, evsel);
+			return perf_evsel__disable(evsel);
 	}
 	return -EINVAL;
 }

@@ -340,7 +340,7 @@ static int intel_bts_snapshot_finish(struct auxtrace_record *itr)
 
 	evlist__for_each(btsr->evlist, evsel) {
 		if (evsel->attr.type == btsr->intel_bts_pmu->type)
-			return perf_evlist__enable_event(btsr->evlist, evsel);
+			return perf_evsel__enable(evsel);
 	}
 	return -EINVAL;
 }
tools/perf/arch/x86/util/intel-pt.c +2 -2
index b64d46285ebb..f05daacc9e78 100644
@@ -725,7 +725,7 @@ static int intel_pt_snapshot_start(struct auxtrace_record *itr)
 
 	evlist__for_each(ptr->evlist, evsel) {
 		if (evsel->attr.type == ptr->intel_pt_pmu->type)
-			return perf_evlist__disable_event(ptr->evlist, evsel);
+			return perf_evsel__disable(evsel);
 	}
 	return -EINVAL;
 }

@@ -738,7 +738,7 @@ static int intel_pt_snapshot_finish(struct auxtrace_record *itr)
 
 	evlist__for_each(ptr->evlist, evsel) {
 		if (evsel->attr.type == ptr->intel_pt_pmu->type)
-			return perf_evlist__enable_event(ptr->evlist, evsel);
+			return perf_evsel__enable(evsel);
 	}
 	return -EINVAL;
 }
tools/perf/tests/keep-tracking.c +1 -1
index a337a6da1f39..7b27943ecfef 100644
@@ -124,7 +124,7 @@ int test__keep_tracking(int subtest __maybe_unused)
 
 	evsel = perf_evlist__last(evlist);
 
-	CHECK__(perf_evlist__disable_event(evlist, evsel));
+	CHECK__(perf_evsel__disable(evsel));
 
 	comm = "Test COMM 2";
 	CHECK__(prctl(PR_SET_NAME, (unsigned long)comm, 0, 0, 0));
tools/perf/tests/switch-tracking.c +3 -3
index dfbd8d69ce89..ebd80168d51e 100644
@@ -455,7 +455,7 @@ int test__switch_tracking(int subtest __maybe_unused)
 
 	perf_evlist__enable(evlist);
 
-	err = perf_evlist__disable_event(evlist, cpu_clocks_evsel);
+	err = perf_evsel__disable(cpu_clocks_evsel);
 	if (err) {
 		pr_debug("perf_evlist__disable_event failed!\n");
 		goto out_err;

@@ -474,7 +474,7 @@ int test__switch_tracking(int subtest __maybe_unused)
 		goto out_err;
 	}
 
-	err = perf_evlist__disable_event(evlist, cycles_evsel);
+	err = perf_evsel__disable(cycles_evsel);
 	if (err) {
 		pr_debug("perf_evlist__disable_event failed!\n");
 		goto out_err;

@@ -500,7 +500,7 @@ int test__switch_tracking(int subtest __maybe_unused)
 		goto out_err;
 	}
 
-	err = perf_evlist__enable_event(evlist, cycles_evsel);
+	err = perf_evsel__enable(cycles_evsel);
 	if (err) {
 		pr_debug("perf_evlist__disable_event failed!\n");
 		goto out_err;
tools/perf/util/evlist.c +0 -42

48 lines not on this page · raw message

tools/perf/util/evlist.h +0 -4
index 139a50038097..25c6c824d870 100644
@@ -152,10 +152,6 @@ void perf_evlist__disable(struct perf_evlist *evlist);
 void perf_evlist__enable(struct perf_evlist *evlist);
 void perf_evlist__toggle_enable(struct perf_evlist *evlist);
 
-int perf_evlist__disable_event(struct perf_evlist *evlist,
-			       struct perf_evsel *evsel);
-int perf_evlist__enable_event(struct perf_evlist *evlist,
-			      struct perf_evsel *evsel);
 int perf_evlist__enable_event_idx(struct perf_evlist *evlist,
 				  struct perf_evsel *evsel, int idx);
Signature · 1 line
2.4.3
author reviewer
#2 lore raw

PATCH 2/6 perf tools: Make perf_evlist__open() open evsels with their cpus and threads (like perf record does)

In reply to Jiri Olsa

From: Adrian Hunter <[email protected]>

'perf record' uses perf_evsel__open() to open events and passes the evsel->cpus and evsel->threads. Many tests and some tools instead use perf_evlist__open() which passes instead evlist->cpus and evlist->threads.

Make perf_evlist__open() follow the 'perf record' behaviour so that a consistent approach is taken.

Signed-off-by Adrian Hunter <[email protected]>
---
 tools/perf/util/evlist.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/util/evlist.c +1 -1
index 05efa910a090..43f4c400a171 100644
@@ -1428,7 +1428,7 @@ int perf_evlist__open(struct perf_evlist *evlist)
 	perf_evlist__update_id_pos(evlist);
 
 	evlist__for_each(evlist, evsel) {
-		err = perf_evsel__open(evsel, evlist->cpus, evlist->threads);
+		err = perf_evsel__open(evsel, evsel->cpus, evsel->threads);
 		if (err < 0)
 			goto out_err;
 	}
Signature · 1 line
2.4.3
author reviewer
#3 lore raw

PATCH 3/6 perf tools: Use find_map function in access_dso_mem

In reply to Jiri Olsa

The find_map helper is already there, so let's use it.

Also we're going to introduce wider search in following patch, so it'll be easier to make this change on single place.

Link http://lkml.kernel.org/n/[email protected] open here
Signed-off-by Jiri Olsa <[email protected]>
---
 tools/perf/util/unwind-libunwind.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
tools/perf/util/unwind-libunwind.c +5 -6
index 3c258a0e4092..f37859c04317 100644
@@ -416,20 +416,19 @@ get_proc_name(unw_addr_space_t __maybe_unused as,
 static int access_dso_mem(struct unwind_info *ui, unw_word_t addr,
 			  unw_word_t *data)
 {
-	struct addr_location al;
+	struct map *map;
 	ssize_t size;
 
-	thread__find_addr_map(ui->thread, PERF_RECORD_MISC_USER,
-			      MAP__FUNCTION, addr, &al);
-	if (!al.map) {
+	map = find_map(addr, ui);
+	if (!map) {
 		pr_debug("unwind: no map for %lx\n", (unsigned long)addr);
 		return -1;
 	}
 
-	if (!al.map->dso)
+	if (!map->dso)
 		return -1;
 
-	size = dso__data_read_addr(al.map->dso, al.map, ui->machine,
+	size = dso__data_read_addr(map->dso, map, ui->machine,
 				   addr, (u8 *) data, sizeof(*data));
 
 	return !(size == sizeof(*data));
Signature · 1 line
2.4.3
author reviewer
#4 lore raw

PATCH 4/6 perf tools libunwind: Check for mmaps also in MAP__VARIABLE tree

In reply to Jiri Olsa

We've seen cases (softice) where DWARF unwinder went through non executable mmaps, which we need to lookup in MAP__VARIABLE tree.

Reported-by Noel Grandin <[email protected]>
Link http://lkml.kernel.org/n/[email protected] open here
Signed-off-by Jiri Olsa <[email protected]>
---
 tools/perf/util/unwind-libunwind.c | 9 +++++++++
 1 file changed, 9 insertions(+)
tools/perf/util/unwind-libunwind.c +9 -0
index f37859c04317..ee7e372297e5 100644
@@ -319,6 +319,15 @@ static struct map *find_map(unw_word_t ip, struct unwind_info *ui)
 
 	thread__find_addr_map(ui->thread, PERF_RECORD_MISC_USER,
 			      MAP__FUNCTION, ip, &al);
+	if (!al.map) {
+		/*
+		 * We've seen cases (softice) where DWARF unwinder went
+		 * through non executable mmaps, which we need to lookup
+		 * in MAP__VARIABLE tree.
+		 */
+		thread__find_addr_map(ui->thread, PERF_RECORD_MISC_USER,
+				      MAP__VARIABLE, ip, &al);
+	}
 	return al.map;
 }
Signature · 1 line
2.4.3
author reviewer
#5 lore raw

PATCH 5/6 perf tools libdw: Check for mmaps also in MAP__VARIABLE tree

In reply to Jiri Olsa

We've seen cases (softice) where DWARF unwinder went through non executable mmaps, which we need to lookup in MAP__VARIABLE tree.

Reported-by Noel Grandin <[email protected]>
Link http://lkml.kernel.org/n/[email protected] open here
Signed-off-by Jiri Olsa <[email protected]>
---
 tools/perf/util/unwind-libdw.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
tools/perf/util/unwind-libdw.c +10 -0
index db8142ba7cb9..cf5e250bc78e 100644
@@ -96,6 +96,16 @@ static int access_dso_mem(struct unwind_info *ui, Dwarf_Addr addr,
 	thread__find_addr_map(ui->thread, PERF_RECORD_MISC_USER,
 			      MAP__FUNCTION, addr, &al);
 	if (!al.map) {
+		/*
+		 * We've seen cases (softice) where DWARF unwinder went
+		 * through non executable mmaps, which we need to lookup
+		 * in MAP__VARIABLE tree.
+		 */
+		thread__find_addr_map(ui->thread, PERF_RECORD_MISC_USER,
+				      MAP__VARIABLE, addr, &al);
+	}
+
+	if (!al.map) {
 		pr_debug("unwind: no map for %lx\n", (unsigned long)addr);
 		return -1;
 	}
Signature · 1 line
2.4.3
author reviewer
#6 lore raw

PATCH 6/6 perf record: Always store data mmaps

In reply to Jiri Olsa

Currently we don't synthesize data mmap by default. It depends on -d option, that enables data address sampling.

But we've seen cases (softice) where DWARF unwinder went through non executable mmaps, which we need to lookup in MAP__VARIABLE tree.

Given the discussion we had with Arnaldo, where he mentioned he wanted to unify MAP__VARIABLE/MAP__FUNCTION anyway, I'm making data mmaps to be synthesized unconditionally.

Reported-by Noel Grandin <[email protected]>
Link http://lkml.kernel.org/n/[email protected] open here
Signed-off-by Jiri Olsa <[email protected]>
---
 tools/perf/builtin-kvm.c              |  2 +-
 tools/perf/builtin-record.c           |  2 +-
 tools/perf/builtin-top.c              |  2 +-
 tools/perf/builtin-trace.c            |  2 +-
 tools/perf/tests/code-reading.c       |  2 +-
 tools/perf/tests/dwarf-unwind.c       |  2 +-
 tools/perf/tests/mmap-thread-lookup.c |  4 ++--
 tools/perf/util/event.c               | 16 ++++++----------
 tools/perf/util/event.h               |  5 ++---
 tools/perf/util/machine.c             |  6 +++---
 tools/perf/util/machine.h             |  6 +++---
 11 files changed, 22 insertions(+), 27 deletions(-)
tools/perf/builtin-kvm.c +1 -1
index 4418d9214872..20cc1b424212 100644
@@ -1400,7 +1400,7 @@ static int kvm_events_live(struct perf_kvm_stat *kvm,
 	perf_session__set_id_hdr_size(kvm->session);
 	ordered_events__set_copy_on_queue(&kvm->session->ordered_events, true);
 	machine__synthesize_threads(&kvm->session->machines.host, &kvm->opts.target,
-				    kvm->evlist->threads, false, kvm->opts.proc_map_timeout);
+				    kvm->evlist->threads, kvm->opts.proc_map_timeout);
 	err = kvm_live_open_events(kvm);
 	if (err)
 		goto out;
tools/perf/builtin-record.c +1 -1
index 9c5cdc2c4471..e37d9c40f05d 100644
@@ -612,7 +612,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	}
 
 	err = __machine__synthesize_threads(machine, tool, &opts->target, rec->evlist->threads,
-					    process_synthesized_event, opts->sample_address,
+					    process_synthesized_event,
 					    opts->proc_map_timeout);
 	if (err != 0)
 		goto out_child;
tools/perf/builtin-top.c +1 -1
index 9ebd67a42ede..f252747ae45c 100644
@@ -960,7 +960,7 @@ static int __cmd_top(struct perf_top *top)
 		goto out_delete;
 
 	machine__synthesize_threads(&top->session->machines.host, &opts->target,
-				    top->evlist->threads, false, opts->proc_map_timeout);
+				    top->evlist->threads, opts->proc_map_timeout);
 
 	if (sort__has_socket) {
 		ret = perf_env__read_cpu_topology_map(&perf_env);
tools/perf/builtin-trace.c +1 -1
index 20916dd77aac..59a96637bbe7 100644
@@ -1649,7 +1649,7 @@ static int trace__symbols_init(struct trace *trace, struct perf_evlist *evlist)
 		return -errno;
 
 	err = __machine__synthesize_threads(trace->host, &trace->tool, &trace->opts.target,
-					    evlist->threads, trace__tool_process, false,
+					    evlist->threads, trace__tool_process,
 					    trace->opts.proc_map_timeout);
 	if (err)
 		symbol__exit();
tools/perf/tests/code-reading.c +1 -1
index 313a48c6b2bc..388c73d85180 100644
@@ -495,7 +495,7 @@ static int do_test_code_reading(bool try_kcore)
 	}
 
 	ret = perf_event__synthesize_thread_map(NULL, threads,
-						perf_event__process, machine, false, 500);
+						perf_event__process, machine, 500);
 	if (ret < 0) {
 		pr_debug("perf_event__synthesize_thread_map failed\n");
 		goto out_err;
tools/perf/tests/dwarf-unwind.c +1 -1
index 1c5c0221cea2..8e413b88b2d9 100644
@@ -32,7 +32,7 @@ static int init_live_machine(struct machine *machine)
 	pid_t pid = getpid();
 
 	return perf_event__synthesize_mmap_events(NULL, &event, pid, pid,
-						  mmap_handler, machine, true, 500);
+						  mmap_handler, machine, 500);
 }
 
 #define MAX_STACK 8
tools/perf/tests/mmap-thread-lookup.c +2 -2
index 0c5ce44f723f..5da977c5bda1 100644
@@ -129,7 +129,7 @@ static int synth_all(struct machine *machine)
 {
 	return perf_event__synthesize_threads(NULL,
 					      perf_event__process,
-					      machine, 0, 500);
+					      machine, 500);
 }
 
 static int synth_process(struct machine *machine)

@@ -141,7 +141,7 @@ static int synth_process(struct machine *machine)
 
 	err = perf_event__synthesize_thread_map(NULL, map,
 						perf_event__process,
-						machine, 0, 500);
+						machine, 500);
 
 	thread_map__put(map);
 	return err;
tools/perf/util/event.c +6 -10
index cd61bb1f3917..3d648b335d45 100644
@@ -229,7 +229,6 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
 				       pid_t pid, pid_t tgid,
 				       perf_event__handler_t process,
 				       struct machine *machine,
-				       bool mmap_data,
 				       unsigned int proc_map_timeout)
 {
 	char filename[PATH_MAX];

@@ -320,7 +319,7 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
 			event->mmap2.flags |= MAP_PRIVATE;
 
 		if (prot[2] != 'x') {
-			if (!mmap_data || prot[0] != 'r')
+			if (prot[0] != 'r')
 				continue;
 
 			event->header.misc |= PERF_RECORD_MISC_MMAP_DATA;

@@ -419,7 +418,6 @@ static int __event__synthesize_thread(union perf_event *comm_event,
 					  perf_event__handler_t process,
 				      struct perf_tool *tool,
 				      struct machine *machine,
-				      bool mmap_data,
 				      unsigned int proc_map_timeout)
 {
 	char filename[PATH_MAX];

@@ -437,7 +435,7 @@ static int __event__synthesize_thread(union perf_event *comm_event,
 			return -1;
 
 		return perf_event__synthesize_mmap_events(tool, mmap_event, pid, tgid,
-							  process, machine, mmap_data,
+							  process, machine,
 							  proc_map_timeout);
 	}
 

@@ -479,7 +477,7 @@ static int __event__synthesize_thread(union perf_event *comm_event,
 		if (_pid == pid) {
 			/* process the parent's maps too */
 			rc = perf_event__synthesize_mmap_events(tool, mmap_event, pid, tgid,
-						process, machine, mmap_data, proc_map_timeout);
+						process, machine, proc_map_timeout);
 			if (rc)
 				break;
 		}

@@ -493,7 +491,6 @@ int perf_event__synthesize_thread_map(struct perf_tool *tool,
 				      struct thread_map *threads,
 				      perf_event__handler_t process,
 				      struct machine *machine,
-				      bool mmap_data,
 				      unsigned int proc_map_timeout)
 {
 	union perf_event *comm_event, *mmap_event, *fork_event;

@@ -517,7 +514,7 @@ int perf_event__synthesize_thread_map(struct perf_tool *tool,
 					       fork_event,
 					       thread_map__pid(threads, thread), 0,
 					       process, tool, machine,
-					       mmap_data, proc_map_timeout)) {
+					       proc_map_timeout)) {
 			err = -1;
 			break;
 		}

@@ -543,7 +540,7 @@ int perf_event__synthesize_thread_map(struct perf_tool *tool,
 						       fork_event,
 						       comm_event->comm.pid, 0,
 						       process, tool, machine,
-						       mmap_data, proc_map_timeout)) {
+						       proc_map_timeout)) {
 				err = -1;
 				break;
 			}

@@ -561,7 +558,6 @@ out:
 int perf_event__synthesize_threads(struct perf_tool *tool,
 				   perf_event__handler_t process,
 				   struct machine *machine,
-				   bool mmap_data,
 				   unsigned int proc_map_timeout)
 {
 	DIR *proc;

@@ -602,7 +598,7 @@ int perf_event__synthesize_threads(struct perf_tool *tool,
  		 * one thread couldn't be synthesized.
  		 */
 		__event__synthesize_thread(comm_event, mmap_event, fork_event, pid,
-					   1, process, tool, machine, mmap_data,
+					   1, process, tool, machine,
 					   proc_map_timeout);
 	}
tools/perf/util/event.h +2 -3
index b7ffb7ee9971..0561c7806690 100644
@@ -514,7 +514,7 @@ typedef int (*perf_event__handler_t)(struct perf_tool *tool,
 int perf_event__synthesize_thread_map(struct perf_tool *tool,
 				      struct thread_map *threads,
 				      perf_event__handler_t process,
-				      struct machine *machine, bool mmap_data,
+				      struct machine *machine,
 				      unsigned int proc_map_timeout);
 int perf_event__synthesize_thread_map2(struct perf_tool *tool,
 				      struct thread_map *threads,

@@ -526,7 +526,7 @@ int perf_event__synthesize_cpu_map(struct perf_tool *tool,
 				   struct machine *machine);
 int perf_event__synthesize_threads(struct perf_tool *tool,
 				   perf_event__handler_t process,
-				   struct machine *machine, bool mmap_data,
+				   struct machine *machine,
 				   unsigned int proc_map_timeout);
 int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 				       perf_event__handler_t process,

@@ -632,7 +632,6 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
 				       pid_t pid, pid_t tgid,
 				       perf_event__handler_t process,
 				       struct machine *machine,
-				       bool mmap_data,
 				       unsigned int proc_map_timeout);
 
 size_t perf_event__fprintf_comm(union perf_event *event, FILE *fp);
tools/perf/util/machine.c +3 -3
index ad79297c76c8..585c668519dc 100644
@@ -1991,13 +1991,13 @@ int machines__for_each_thread(struct machines *machines,
 
 int __machine__synthesize_threads(struct machine *machine, struct perf_tool *tool,
 				  struct target *target, struct thread_map *threads,
-				  perf_event__handler_t process, bool data_mmap,
+				  perf_event__handler_t process,
 				  unsigned int proc_map_timeout)
 {
 	if (target__has_task(target))
-		return perf_event__synthesize_thread_map(tool, threads, process, machine, data_mmap, proc_map_timeout);
+		return perf_event__synthesize_thread_map(tool, threads, process, machine, proc_map_timeout);
 	else if (target__has_cpu(target))
-		return perf_event__synthesize_threads(tool, process, machine, data_mmap, proc_map_timeout);
+		return perf_event__synthesize_threads(tool, process, machine, proc_map_timeout);
 	/* command specified */
 	return 0;
 }
tools/perf/util/machine.h +3 -3
index 2c2b443df5ba..91a93b2c2a3a 100644
@@ -231,15 +231,15 @@ int machines__for_each_thread(struct machines *machines,
 
 int __machine__synthesize_threads(struct machine *machine, struct perf_tool *tool,
 				  struct target *target, struct thread_map *threads,
-				  perf_event__handler_t process, bool data_mmap,
+				  perf_event__handler_t process,
 				  unsigned int proc_map_timeout);
 static inline
 int machine__synthesize_threads(struct machine *machine, struct target *target,
-				struct thread_map *threads, bool data_mmap,
+				struct thread_map *threads,
 				unsigned int proc_map_timeout)
 {
 	return __machine__synthesize_threads(machine, NULL, target, threads,
-					     perf_event__process, data_mmap,
+					     perf_event__process,
 					     proc_map_timeout);
 }
Signature · 1 line
2.4.3
maintainer
#7 lore raw

Re: [PATCH 6/6] perf record: Always store data mmaps

Quoted 9 lines · Jiri Olsa

Currently we don't synthesize data mmap by default. It depends on -d option, that enables data address sampling.

But we've seen cases (softice) where DWARF unwinder went through non executable mmaps, which we need to lookup in MAP__VARIABLE tree.

Given the discussion we had with Arnaldo, where he mentioned he wanted to unify MAP__VARIABLE/MAP__FUNCTION anyway, I'm making data mmaps to be synthesized unconditionally.

But I'm afraid of increased data size. Could you check how much is it?

Thanks,
Namhyung

Quoted 285 lines
Reported-by Noel Grandin <[email protected]>
Link http://lkml.kernel.org/n/[email protected] open here
Signed-off-by Jiri Olsa <[email protected]>
---
 tools/perf/builtin-kvm.c              |  2 +-
 tools/perf/builtin-record.c           |  2 +-
 tools/perf/builtin-top.c              |  2 +-
 tools/perf/builtin-trace.c            |  2 +-
 tools/perf/tests/code-reading.c       |  2 +-
 tools/perf/tests/dwarf-unwind.c       |  2 +-
 tools/perf/tests/mmap-thread-lookup.c |  4 ++--
 tools/perf/util/event.c               | 16 ++++++----------
 tools/perf/util/event.h               |  5 ++---
 tools/perf/util/machine.c             |  6 +++---
 tools/perf/util/machine.h             |  6 +++---
 11 files changed, 22 insertions(+), 27 deletions(-)
tools/perf/builtin-kvm.c +1 -1
index 4418d9214872..20cc1b424212 100644
@@ -1400,7 +1400,7 @@ static int kvm_events_live(struct perf_kvm_stat *kvm,
 	perf_session__set_id_hdr_size(kvm->session);
 	ordered_events__set_copy_on_queue(&kvm->session->ordered_events, true);
 	machine__synthesize_threads(&kvm->session->machines.host, &kvm->opts.target,
-				    kvm->evlist->threads, false, kvm->opts.proc_map_timeout);
+				    kvm->evlist->threads, kvm->opts.proc_map_timeout);
 	err = kvm_live_open_events(kvm);
 	if (err)
 		goto out;
tools/perf/builtin-record.c +1 -1
index 9c5cdc2c4471..e37d9c40f05d 100644
@@ -612,7 +612,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	}
 
 	err = __machine__synthesize_threads(machine, tool, &opts->target, rec->evlist->threads,
-					    process_synthesized_event, opts->sample_address,
+					    process_synthesized_event,
 					    opts->proc_map_timeout);
 	if (err != 0)
 		goto out_child;
tools/perf/builtin-top.c +1 -1
index 9ebd67a42ede..f252747ae45c 100644
@@ -960,7 +960,7 @@ static int __cmd_top(struct perf_top *top)
 		goto out_delete;
 
 	machine__synthesize_threads(&top->session->machines.host, &opts->target,
-				    top->evlist->threads, false, opts->proc_map_timeout);
+				    top->evlist->threads, opts->proc_map_timeout);
 
 	if (sort__has_socket) {
 		ret = perf_env__read_cpu_topology_map(&perf_env);
tools/perf/builtin-trace.c +1 -1
index 20916dd77aac..59a96637bbe7 100644
@@ -1649,7 +1649,7 @@ static int trace__symbols_init(struct trace *trace, struct perf_evlist *evlist)
 		return -errno;
 
 	err = __machine__synthesize_threads(trace->host, &trace->tool, &trace->opts.target,
-					    evlist->threads, trace__tool_process, false,
+					    evlist->threads, trace__tool_process,
 					    trace->opts.proc_map_timeout);
 	if (err)
 		symbol__exit();
tools/perf/tests/code-reading.c +1 -1
index 313a48c6b2bc..388c73d85180 100644
@@ -495,7 +495,7 @@ static int do_test_code_reading(bool try_kcore)
 	}
 
 	ret = perf_event__synthesize_thread_map(NULL, threads,
-						perf_event__process, machine, false, 500);
+						perf_event__process, machine, 500);
 	if (ret < 0) {
 		pr_debug("perf_event__synthesize_thread_map failed\n");
 		goto out_err;
tools/perf/tests/dwarf-unwind.c +1 -1
index 1c5c0221cea2..8e413b88b2d9 100644
@@ -32,7 +32,7 @@ static int init_live_machine(struct machine *machine)
 	pid_t pid = getpid();
 
 	return perf_event__synthesize_mmap_events(NULL, &event, pid, pid,
-						  mmap_handler, machine, true, 500);
+						  mmap_handler, machine, 500);
 }
 
 #define MAX_STACK 8
tools/perf/tests/mmap-thread-lookup.c +2 -2
index 0c5ce44f723f..5da977c5bda1 100644
@@ -129,7 +129,7 @@ static int synth_all(struct machine *machine)
 {
 	return perf_event__synthesize_threads(NULL,
 					      perf_event__process,
-					      machine, 0, 500);
+					      machine, 500);
 }
 
 static int synth_process(struct machine *machine)

@@ -141,7 +141,7 @@ static int synth_process(struct machine *machine)
 
 	err = perf_event__synthesize_thread_map(NULL, map,
 						perf_event__process,
-						machine, 0, 500);
+						machine, 500);
 
 	thread_map__put(map);
 	return err;
tools/perf/util/event.c +6 -10
index cd61bb1f3917..3d648b335d45 100644
@@ -229,7 +229,6 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
 				       pid_t pid, pid_t tgid,
 				       perf_event__handler_t process,
 				       struct machine *machine,
-				       bool mmap_data,
 				       unsigned int proc_map_timeout)
 {
 	char filename[PATH_MAX];

@@ -320,7 +319,7 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
 			event->mmap2.flags |= MAP_PRIVATE;
 
 		if (prot[2] != 'x') {
-			if (!mmap_data || prot[0] != 'r')
+			if (prot[0] != 'r')
 				continue;
 
 			event->header.misc |= PERF_RECORD_MISC_MMAP_DATA;

@@ -419,7 +418,6 @@ static int __event__synthesize_thread(union perf_event *comm_event,
 					  perf_event__handler_t process,
 				      struct perf_tool *tool,
 				      struct machine *machine,
-				      bool mmap_data,
 				      unsigned int proc_map_timeout)
 {
 	char filename[PATH_MAX];

@@ -437,7 +435,7 @@ static int __event__synthesize_thread(union perf_event *comm_event,
 			return -1;
 
 		return perf_event__synthesize_mmap_events(tool, mmap_event, pid, tgid,
-							  process, machine, mmap_data,
+							  process, machine,
 							  proc_map_timeout);
 	}
 

@@ -479,7 +477,7 @@ static int __event__synthesize_thread(union perf_event *comm_event,
 		if (_pid == pid) {
 			/* process the parent's maps too */
 			rc = perf_event__synthesize_mmap_events(tool, mmap_event, pid, tgid,
-						process, machine, mmap_data, proc_map_timeout);
+						process, machine, proc_map_timeout);
 			if (rc)
 				break;
 		}

@@ -493,7 +491,6 @@ int perf_event__synthesize_thread_map(struct perf_tool *tool,
 				      struct thread_map *threads,
 				      perf_event__handler_t process,
 				      struct machine *machine,
-				      bool mmap_data,
 				      unsigned int proc_map_timeout)
 {
 	union perf_event *comm_event, *mmap_event, *fork_event;

@@ -517,7 +514,7 @@ int perf_event__synthesize_thread_map(struct perf_tool *tool,
 					       fork_event,
 					       thread_map__pid(threads, thread), 0,
 					       process, tool, machine,
-					       mmap_data, proc_map_timeout)) {
+					       proc_map_timeout)) {
 			err = -1;
 			break;
 		}

@@ -543,7 +540,7 @@ int perf_event__synthesize_thread_map(struct perf_tool *tool,
 						       fork_event,
 						       comm_event->comm.pid, 0,
 						       process, tool, machine,
-						       mmap_data, proc_map_timeout)) {
+						       proc_map_timeout)) {
 				err = -1;
 				break;
 			}

@@ -561,7 +558,6 @@ out:
 int perf_event__synthesize_threads(struct perf_tool *tool,
 				   perf_event__handler_t process,
 				   struct machine *machine,
-				   bool mmap_data,
 				   unsigned int proc_map_timeout)
 {
 	DIR *proc;

@@ -602,7 +598,7 @@ int perf_event__synthesize_threads(struct perf_tool *tool,
  		 * one thread couldn't be synthesized.
  		 */
 		__event__synthesize_thread(comm_event, mmap_event, fork_event, pid,
-					   1, process, tool, machine, mmap_data,
+					   1, process, tool, machine,
 					   proc_map_timeout);
 	}
tools/perf/util/event.h +2 -3
index b7ffb7ee9971..0561c7806690 100644
@@ -514,7 +514,7 @@ typedef int (*perf_event__handler_t)(struct perf_tool *tool,
 int perf_event__synthesize_thread_map(struct perf_tool *tool,
 				      struct thread_map *threads,
 				      perf_event__handler_t process,
-				      struct machine *machine, bool mmap_data,
+				      struct machine *machine,
 				      unsigned int proc_map_timeout);
 int perf_event__synthesize_thread_map2(struct perf_tool *tool,
 				      struct thread_map *threads,

@@ -526,7 +526,7 @@ int perf_event__synthesize_cpu_map(struct perf_tool *tool,
 				   struct machine *machine);
 int perf_event__synthesize_threads(struct perf_tool *tool,
 				   perf_event__handler_t process,
-				   struct machine *machine, bool mmap_data,
+				   struct machine *machine,
 				   unsigned int proc_map_timeout);
 int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 				       perf_event__handler_t process,

@@ -632,7 +632,6 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
 				       pid_t pid, pid_t tgid,
 				       perf_event__handler_t process,
 				       struct machine *machine,
-				       bool mmap_data,
 				       unsigned int proc_map_timeout);
 
 size_t perf_event__fprintf_comm(union perf_event *event, FILE *fp);
tools/perf/util/machine.c +3 -3
index ad79297c76c8..585c668519dc 100644
@@ -1991,13 +1991,13 @@ int machines__for_each_thread(struct machines *machines,
 
 int __machine__synthesize_threads(struct machine *machine, struct perf_tool *tool,
 				  struct target *target, struct thread_map *threads,
-				  perf_event__handler_t process, bool data_mmap,
+				  perf_event__handler_t process,
 				  unsigned int proc_map_timeout)
 {
 	if (target__has_task(target))
-		return perf_event__synthesize_thread_map(tool, threads, process, machine, data_mmap, proc_map_timeout);
+		return perf_event__synthesize_thread_map(tool, threads, process, machine, proc_map_timeout);
 	else if (target__has_cpu(target))
-		return perf_event__synthesize_threads(tool, process, machine, data_mmap, proc_map_timeout);
+		return perf_event__synthesize_threads(tool, process, machine, proc_map_timeout);
 	/* command specified */
 	return 0;
 }
tools/perf/util/machine.h +3 -3
index 2c2b443df5ba..91a93b2c2a3a 100644
@@ -231,15 +231,15 @@ int machines__for_each_thread(struct machines *machines,
 
 int __machine__synthesize_threads(struct machine *machine, struct perf_tool *tool,
 				  struct target *target, struct thread_map *threads,
-				  perf_event__handler_t process, bool data_mmap,
+				  perf_event__handler_t process,
 				  unsigned int proc_map_timeout);
 static inline
 int machine__synthesize_threads(struct machine *machine, struct target *target,
-				struct thread_map *threads, bool data_mmap,
+				struct thread_map *threads,
 				unsigned int proc_map_timeout)
 {
 	return __machine__synthesize_threads(machine, NULL, target, threads,
-					     perf_event__process, data_mmap,
+					     perf_event__process,
 					     proc_map_timeout);
 }
Signature · 1 line
2.4.3
#8 lore raw

Re: [PATCH 6/6] perf record: Always store data mmaps

Quoted 4 lines · Namhyung Kim
Quoted 3 lines · Jiri Olsa

Currently we don't synthesize data mmap by default. It depends on -d option, that enables data address sampling.

Quoted 8 lines
Quoted 6 lines

But we've seen cases (softice) where DWARF unwinder went through non executable mmaps, which we need to lookup in MAP__VARIABLE tree.

Given the discussion we had with Arnaldo, where he mentioned he wanted to unify MAP__VARIABLE/MAP__FUNCTION anyway, I'm making data mmaps to be synthesized unconditionally.

But I'm afraid of increased data size. Could you check how much is it?

I'll check

jirka

#9 lore raw
In reply to Jiri Olsa
Quoted 11 lines · Jiri Olsa

hi,
sending several changes together:
- leftover for the stat enable/disable changes with Adrian's patch
- fixes for issues Noel found with DWARF unwind

Noel, could you please rerun your test on this?

Also available in:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/fixes

Quoted 7 lines

thanks,
jirka

---
Adrian Hunter (1):
perf tools: Make perf_evlist__open() open evsels with their cpus and threads (like perf record does)

argh, forgot to CC Adrian, sry.. adding him to the loop now

jirka

Quoted 28 lines

Jiri Olsa (5):
perf tools: Remove perf_evlist__(enable|disable)_event functions perf tools: Use find_map function in access_dso_mem perf tools libunwind: Check for mmaps also in MAP__VARIABLE tree perf tools libdw: Check for mmaps also in MAP__VARIABLE tree perf record: Always store data mmaps

 tools/perf/arch/x86/util/intel-bts.c  |  4 ++--
 tools/perf/arch/x86/util/intel-pt.c   |  4 ++--
 tools/perf/builtin-kvm.c              |  2 +-
 tools/perf/builtin-record.c           |  2 +-
 tools/perf/builtin-top.c              |  2 +-
 tools/perf/builtin-trace.c            |  2 +-
 tools/perf/tests/code-reading.c       |  2 +-
 tools/perf/tests/dwarf-unwind.c       |  2 +-
 tools/perf/tests/keep-tracking.c      |  2 +-
 tools/perf/tests/mmap-thread-lookup.c |  4 ++--
 tools/perf/tests/switch-tracking.c    |  6 +++---
 tools/perf/util/event.c               | 16 ++++++----------
 tools/perf/util/event.h               |  5 ++---
 tools/perf/util/evlist.c              | 44 +-------------------------------------------
 tools/perf/util/evlist.h              |  4 ----
 tools/perf/util/machine.c             |  6 +++---
 tools/perf/util/machine.h             |  6 +++---
 tools/perf/util/unwind-libdw.c        | 10 ++++++++++
 tools/perf/util/unwind-libunwind.c    | 20 ++++++++++++++------
 19 files changed, 55 insertions(+), 88 deletions(-)
Tested-by on the v1 series
#10 lore raw
In reply to Jiri Olsa

This series is

Tested-by Noel Grandin <[email protected]>
Quoted 47 lines · Jiri Olsa

hi,
sending several changes together:
- leftover for the stat enable/disable changes with Adrian's patch
- fixes for issues Noel found with DWARF unwind

Noel, could you please rerun your test on this?

Also available in:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/fixes

thanks,
jirka

---
Adrian Hunter (1):
perf tools: Make perf_evlist__open() open evsels with their cpus and threads (like perf record does)

Jiri Olsa (5):
perf tools: Remove perf_evlist__(enable|disable)_event functions perf tools: Use find_map function in access_dso_mem perf tools libunwind: Check for mmaps also in MAP__VARIABLE tree perf tools libdw: Check for mmaps also in MAP__VARIABLE tree perf record: Always store data mmaps

  tools/perf/arch/x86/util/intel-bts.c  |  4 ++--
  tools/perf/arch/x86/util/intel-pt.c   |  4 ++--
  tools/perf/builtin-kvm.c              |  2 +-
  tools/perf/builtin-record.c           |  2 +-
  tools/perf/builtin-top.c              |  2 +-
  tools/perf/builtin-trace.c            |  2 +-
  tools/perf/tests/code-reading.c       |  2 +-
  tools/perf/tests/dwarf-unwind.c       |  2 +-
  tools/perf/tests/keep-tracking.c      |  2 +-
  tools/perf/tests/mmap-thread-lookup.c |  4 ++--
  tools/perf/tests/switch-tracking.c    |  6 +++---
  tools/perf/util/event.c               | 16 ++++++----------
  tools/perf/util/event.h               |  5 ++---
  tools/perf/util/evlist.c              | 44 +-------------------------------------------
  tools/perf/util/evlist.h              |  4 ----
  tools/perf/util/machine.c             |  6 +++---
  tools/perf/util/machine.h             |  6 +++---
  tools/perf/util/unwind-libdw.c        | 10 ++++++++++
  tools/perf/util/unwind-libunwind.c    | 20 ++++++++++++++------
  19 files changed, 55 insertions(+), 88 deletions(-)
#11 lore raw
Quoted 15 lines · Noel Grandin

This series is

Tested-by Noel Grandin <[email protected]>
Quoted 10 lines · Jiri Olsa

hi,
sending several changes together:
- leftover for the stat enable/disable changes with Adrian's patch
- fixes for issues Noel found with DWARF unwind

Noel, could you please rerun your test on this?

Also available in:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/fixes

I updated perf/fixes branch woth Noel's tag

thanks,
jirka

#12 lore raw
Quoted 11 lines · Jiri Olsa
Quoted 10 lines · Noel Grandin

This series is

Tested-by Noel Grandin <[email protected]>
Quoted 5 lines · Jiri Olsa

hi,
sending several changes together:
- leftover for the stat enable/disable changes with Adrian's patch
- fixes for issues Noel found with DWARF unwind

Quoted 7 lines
Quoted 5 lines
Quoted 5 lines

Noel, could you please rerun your test on this?

Also available in:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/fixes

I updated perf/fixes branch woth Noel's tag

perf/fixes rebased to latest acme/perf/core

FYI it contains couple more small fixes on top, which I'll send out later

thanks,
jirka