Re: [PATCH 08/17] perf tools: Move kernel mmap name into struct machine

From: Arnaldo Carvalho de Melo
Date: Tue Feb 06 2018 - 14:10:46 EST

Next message: Bjorn Helgaas: "Re: [PATCH 20/20] x86/PCI: VMD: Fix malformed default"
Previous message: Jin Qian: "[PATCHv2 1/1] ext4: don't put symlink in pagecache into highmem"
In reply to: Jiri Olsa: "[PATCH 08/17] perf tools: Move kernel mmap name into struct machine"
Next in thread: Jiri Olsa: "[PATCH 03/17] perf script: Add --show-round-event to display PERF_RECORD_FINISHED_ROUND"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Em Tue, Feb 06, 2018 at 07:18:04PM +0100, Jiri Olsa escreveu:
> It simplifies and centralizes the code. The kernel mmap
> name is set for machine type, which we know from the
> beginning, so there's no reason to generate it every time
> we need it.
>
> Link: http://lkml.kernel.org/n/tip-2fx7kxxdc5zcm6990cq2mday@xxxxxxxxxxxxxx
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
> tools/perf/util/build-id.c | 10 +++----
> tools/perf/util/event.c | 5 +---
> tools/perf/util/machine.c | 67 +++++++++++++++++++++++-----------------------
> tools/perf/util/machine.h | 3 +--
> tools/perf/util/symbol.c | 3 +--
> 5 files changed, 39 insertions(+), 49 deletions(-)
>
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index 7f8553630c4d..537eadd81914 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -316,7 +316,6 @@ static int machine__write_buildid_table(struct machine *machine,
> struct feat_fd *fd)
> {
> int err = 0;
> - char nm[PATH_MAX];
> struct dso *pos;
> u16 kmisc = PERF_RECORD_MISC_KERNEL,
> umisc = PERF_RECORD_MISC_USER;
> @@ -338,9 +337,8 @@ static int machine__write_buildid_table(struct machine *machine,
> name = pos->short_name;
> name_len = pos->short_name_len;
> } else if (dso__is_kcore(pos)) {
> - machine__mmap_name(machine, nm, sizeof(nm));
> - name = nm;
> - name_len = strlen(nm);
> + name = machine->mmap_name;
> + name_len = strlen(name);
> } else {
> name = pos->long_name;
> name_len = pos->long_name_len;
> @@ -813,12 +811,10 @@ static int dso__cache_build_id(struct dso *dso, struct machine *machine)
> bool is_kallsyms = dso__is_kallsyms(dso);
> bool is_vdso = dso__is_vdso(dso);
> const char *name = dso->long_name;
> - char nm[PATH_MAX];
>
> if (dso__is_kcore(dso)) {
> is_kallsyms = true;
> - machine__mmap_name(machine, nm, sizeof(nm));
> - name = nm;
> + name = machine->mmap_name;
> }
> return build_id_cache__add_b(dso->build_id, sizeof(dso->build_id), name,
> dso->nsinfo, is_kallsyms, is_vdso);
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 44e603c27944..4644e751a3e3 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -894,8 +894,6 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
> struct machine *machine)
> {
> size_t size;
> - const char *mmap_name;
> - char name_buff[PATH_MAX];
> struct map *map = machine__kernel_map(machine);
> struct kmap *kmap;
> int err;
> @@ -918,7 +916,6 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
> return -1;
> }
>
> - mmap_name = machine__mmap_name(machine, name_buff, sizeof(name_buff));
> if (machine__is_host(machine)) {
> /*
> * kernel uses PERF_RECORD_MISC_USER for user space maps,
> @@ -931,7 +928,7 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
>
> kmap = map__kmap(map);
> size = snprintf(event->mmap.filename, sizeof(event->mmap.filename),
> - "%s%s", mmap_name, kmap->ref_reloc_sym->name) + 1;
> + "%s%s", machine->mmap_name, kmap->ref_reloc_sym->name) + 1;
> size = PERF_ALIGN(size, sizeof(u64));
> event->mmap.header.type = PERF_RECORD_MMAP;
> event->mmap.header.size = (sizeof(event->mmap) -
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index e300a643f65b..447f10e1323e 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -48,6 +48,27 @@ static void machine__threads_init(struct machine *machine)
> }
> }
>
> +static int machine__mmap_name(struct machine *machine)

Rename this to machine__set_mmap_name()?

> +{
> + if (machine__is_host(machine)) {
> + if (symbol_conf.vmlinux_name)
> + machine->mmap_name = strdup(symbol_conf.vmlinux_name);
> + else
> + machine->mmap_name = strdup("[kernel.kallsyms]");
> + } else if (machine__is_default_guest(machine)) {
> + if (symbol_conf.default_guest_vmlinux_name)
> + machine->mmap_name = strdup(symbol_conf.default_guest_vmlinux_name);
> + else
> + machine->mmap_name = strdup("[guest.kernel.kallsyms]");
> + } else {
> + if (asprintf(&machine->mmap_name, "[guest.kernel.kallsyms.%d]",
> + machine->pid) < 0)
> + machine->mmap_name = NULL;
> + }
> +
> + return machine->mmap_name ? 0 : -ENOMEM;
> +}
> +
> int machine__init(struct machine *machine, const char *root_dir, pid_t pid)
> {
> int err = -ENOMEM;
> @@ -75,6 +96,9 @@ int machine__init(struct machine *machine, const char *root_dir, pid_t pid)
> if (machine->root_dir == NULL)
> return -ENOMEM;
>
> + if (machine__mmap_name(machine))
> + goto out;
> +
> if (pid != HOST_KERNEL_ID) {
> struct thread *thread = machine__findnew_thread(machine, -1,
> pid);
> @@ -92,8 +116,10 @@ int machine__init(struct machine *machine, const char *root_dir, pid_t pid)
> err = 0;
>
> out:
> - if (err)
> + if (err) {
> free(machine->root_dir);
> + free(machine->mmap_name);

zfree()

> + }
> return 0;
> }
>
> @@ -186,6 +212,7 @@ void machine__exit(struct machine *machine)
> dsos__exit(&machine->dsos);
> machine__exit_vdso(machine);
> zfree(&machine->root_dir);
> + zfree(&machine->mmap_name);
> zfree(&machine->current_tid);
>
> for (i = 0; i < THREADS__TABLE_SIZE; i++) {
> @@ -328,20 +355,6 @@ void machines__process_guests(struct machines *machines,
> }
> }
>
> -char *machine__mmap_name(struct machine *machine, char *bf, size_t size)
> -{
> - if (machine__is_host(machine))
> - snprintf(bf, size, "[%s]", "kernel.kallsyms");
> - else if (machine__is_default_guest(machine))
> - snprintf(bf, size, "[%s]", "guest.kernel.kallsyms");
> - else {
> - snprintf(bf, size, "[%s.%d]", "guest.kernel.kallsyms",
> - machine->pid);
> - }
> -
> - return bf;
> -}
> -
> void machines__set_id_hdr_size(struct machines *machines, u16 id_hdr_size)
> {
> struct rb_node *node;
> @@ -777,25 +790,13 @@ size_t machine__fprintf(struct machine *machine, FILE *fp)
>
> static struct dso *machine__get_kernel(struct machine *machine)
> {
> - const char *vmlinux_name = NULL;
> + const char *vmlinux_name = machine->mmap_name;
> struct dso *kernel;
>
> if (machine__is_host(machine)) {
> - vmlinux_name = symbol_conf.vmlinux_name;
> - if (!vmlinux_name)
> - vmlinux_name = DSO__NAME_KALLSYMS;
> -
> kernel = machine__findnew_kernel(machine, vmlinux_name,
> "[kernel]", DSO_TYPE_KERNEL);
> } else {
> - char bf[PATH_MAX];
> -
> - if (machine__is_default_guest(machine))
> - vmlinux_name = symbol_conf.default_guest_vmlinux_name;
> - if (!vmlinux_name)
> - vmlinux_name = machine__mmap_name(machine, bf,
> - sizeof(bf));
> -
> kernel = machine__findnew_kernel(machine, vmlinux_name,
> "[guest.kernel]",
> DSO_TYPE_GUEST_KERNEL);
> @@ -1295,7 +1296,6 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
> union perf_event *event)
> {
> struct map *map;
> - char kmmap_prefix[PATH_MAX];
> enum dso_kernel_type kernel_type;
> bool is_kernel_mmap;
>
> @@ -1303,15 +1303,14 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
> if (machine__uses_kcore(machine))
> return 0;
>
> - machine__mmap_name(machine, kmmap_prefix, sizeof(kmmap_prefix));
> if (machine__is_host(machine))
> kernel_type = DSO_TYPE_KERNEL;
> else
> kernel_type = DSO_TYPE_GUEST_KERNEL;
>
> is_kernel_mmap = memcmp(event->mmap.filename,
> - kmmap_prefix,
> - strlen(kmmap_prefix) - 1) == 0;
> + machine->mmap_name,
> + strlen(machine->mmap_name) - 1) == 0;
> if (event->mmap.filename[0] == '/' ||
> (!is_kernel_mmap && event->mmap.filename[0] == '[')) {
> map = machine__findnew_module_map(machine, event->mmap.start,
> @@ -1322,7 +1321,7 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
> map->end = map->start + event->mmap.len;
> } else if (is_kernel_mmap) {
> const char *symbol_name = (event->mmap.filename +
> - strlen(kmmap_prefix));
> + strlen(machine->mmap_name));
> /*
> * Should be there already, from the build-id table in
> * the header.
> @@ -1363,7 +1362,7 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
> up_read(&machine->dsos.lock);
>
> if (kernel == NULL)
> - kernel = machine__findnew_dso(machine, kmmap_prefix);
> + kernel = machine__findnew_dso(machine, machine->mmap_name);
> if (kernel == NULL)
> goto out_problem;
>
> diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
> index 5ce860b64c74..cb0a20f3a96b 100644
> --- a/tools/perf/util/machine.h
> +++ b/tools/perf/util/machine.h
> @@ -43,6 +43,7 @@ struct machine {
> bool comm_exec;
> bool kptr_restrict_warned;
> char *root_dir;
> + char *mmap_name;
> struct threads threads[THREADS__TABLE_SIZE];
> struct vdso_info *vdso_info;
> struct perf_env *env;
> @@ -142,8 +143,6 @@ struct machine *machines__find(struct machines *machines, pid_t pid);
> struct machine *machines__findnew(struct machines *machines, pid_t pid);
>
> void machines__set_id_hdr_size(struct machines *machines, u16 id_hdr_size);
> -char *machine__mmap_name(struct machine *machine, char *bf, size_t size);
> -
> void machines__set_comm_exec(struct machines *machines, bool comm_exec);
>
> struct machine *machine__new_host(void);
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index cc065d4bfafc..dcb81176669a 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1960,8 +1960,7 @@ static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map)
> pr_debug("Using %s for symbols\n", kallsyms_filename);
> if (err > 0 && !dso__is_kcore(dso)) {
> dso->binary_type = DSO_BINARY_TYPE__GUEST_KALLSYMS;
> - machine__mmap_name(machine, path, sizeof(path));
> - dso__set_long_name(dso, strdup(path), true);
> + dso__set_long_name(dso, machine->mmap_name, false);
> map__fixup_start(map);
> map__fixup_end(map);
> }
> --
> 2.13.6

Next message: Bjorn Helgaas: "Re: [PATCH 20/20] x86/PCI: VMD: Fix malformed default"
Previous message: Jin Qian: "[PATCHv2 1/1] ext4: don't put symlink in pagecache into highmem"
In reply to: Jiri Olsa: "[PATCH 08/17] perf tools: Move kernel mmap name into struct machine"
Next in thread: Jiri Olsa: "[PATCH 03/17] perf script: Add --show-round-event to display PERF_RECORD_FINISHED_ROUND"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]