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

From: Namhyung Kim
Date: Tue Jan 05 2016 - 06:17:32 EST


On Thu, Dec 17, 2015 at 09:26:55PM +0100, Jiri Olsa wrote:
> 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


>
> Reported-by: Noel Grandin <noelgrandin@xxxxxxxxx>
> Link: http://lkml.kernel.org/n/tip-lh02yir6qfycn8zr892rmlgg@xxxxxxxxxxxxxx
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
> 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(-)
>
> diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
> index 4418d9214872..20cc1b424212 100644
> --- a/tools/perf/builtin-kvm.c
> +++ b/tools/perf/builtin-kvm.c
> @@ -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;
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 9c5cdc2c4471..e37d9c40f05d 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -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;
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 9ebd67a42ede..f252747ae45c 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -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);
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index 20916dd77aac..59a96637bbe7 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -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();
> diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
> index 313a48c6b2bc..388c73d85180 100644
> --- a/tools/perf/tests/code-reading.c
> +++ b/tools/perf/tests/code-reading.c
> @@ -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;
> diff --git a/tools/perf/tests/dwarf-unwind.c b/tools/perf/tests/dwarf-unwind.c
> index 1c5c0221cea2..8e413b88b2d9 100644
> --- a/tools/perf/tests/dwarf-unwind.c
> +++ b/tools/perf/tests/dwarf-unwind.c
> @@ -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
> diff --git a/tools/perf/tests/mmap-thread-lookup.c b/tools/perf/tests/mmap-thread-lookup.c
> index 0c5ce44f723f..5da977c5bda1 100644
> --- a/tools/perf/tests/mmap-thread-lookup.c
> +++ b/tools/perf/tests/mmap-thread-lookup.c
> @@ -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;
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index cd61bb1f3917..3d648b335d45 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -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);
> }
>
> diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
> index b7ffb7ee9971..0561c7806690 100644
> --- a/tools/perf/util/event.h
> +++ b/tools/perf/util/event.h
> @@ -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);
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index ad79297c76c8..585c668519dc 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -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;
> }
> diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
> index 2c2b443df5ba..91a93b2c2a3a 100644
> --- a/tools/perf/util/machine.h
> +++ b/tools/perf/util/machine.h
> @@ -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);
> }
>
> --
> 2.4.3
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/