Hi Arnaldo,
On Thu, Sep 24, 2015 at 10:42:28AM -0300, Arnaldo Carvalho de Melo wrote:
> Namhyung,
>
> Can you take a look and perhaps give me your Acked-by?
The dso->kernel and event->cpumode is always confusing for modules..
In this case, it seems that mmap events set kernel cpumode but
build-id events don't. So kernel cpumode in a bulid-id event
indicates that it is a kernel (vmlinux) dso, right?
I'll test this tomorrow..
Thanks,
Namhyung
>
> From: Arnaldo Carvalho de Melo <a...@redhat.com>
>
> No need to traverse it all again using the filename extension to
> disambiguate kernel from kernel modules, just cache it when
> reading the buildid table.
>
> This also fixes a refcount bug in the error path, i.e. in the error path
> for the __machine__create_kernel_maps() call we do a dso__put(), which
> is ok and expected if that dso came from machine__findnew_dso(), but
> wasn't when we found it by traversing the machine dso list without
> grabbing a dso refcount (dso__get()) before dropping the list lock.
>
> Fixes: b837a8bdc489 ("perf tools: Fix build-id matching on vmlinux")
> Link: http://lkml.kernel.org/n/tip-48fh3cf6pvs4zs2fj4nhc...@git.kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo <a...@redhat.com>
> ---
> tools/perf/util/header.c | 7 +++++++
> tools/perf/util/machine.c | 34 +++-------------------------------
> tools/perf/util/machine.h | 1 +
> 3 files changed, 11 insertions(+), 31 deletions(-)
>
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 43838003c1a1..6bc92ae1e99a 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -1260,6 +1260,13 @@ static int __event_process_build_id(struct
> build_id_event *bev,
>
> if (!is_kernel_module(filename, cpumode))
> dso->kernel = dso_type;
> + /*
> + * We don't need to grab a reference as long as
> + * the kernel dso is in the machine dso list.
> + */
> + if (cpumode == PERF_RECORD_MISC_KERNEL ||
> + cpumode == PERF_RECORD_MISC_GUEST_KERNEL)
> + machine->kernel_dso = dso;
>
> build_id__sprintf(dso->build_id, sizeof(dso->build_id),
> sbuild_id);
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index d07a38678e14..f6e689b2c83c 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -62,6 +62,7 @@ int machine__init(struct machine *machine, const char
> *root_dir, pid_t pid)
> }
>
> machine->current_tid = NULL;
> + machine->kernel_dso = NULL;
>
> return 0;
> }
> @@ -1180,39 +1181,10 @@ static int machine__process_kernel_mmap_event(struct
> machine *machine,
> * Should be there already, from the build-id table in
> * the header.
> */
> - struct dso *kernel = NULL;
> - struct dso *dso;
> + struct dso *kernel;
>
> pthread_rwlock_rdlock(&machine->dsos.lock);
> -
> - list_for_each_entry(dso, &machine->dsos.head, node) {
> -
> - /*
> - * The cpumode passed to is_kernel_module is not the
> - * cpumode of *this* event. If we insist on passing
> - * correct cpumode to is_kernel_module, we should
> - * record the cpumode when we adding this dso to the
> - * linked list.
> - *
> - * However we don't really need passing correct
> - * cpumode. We know the correct cpumode must be kernel
> - * mode (if not, we should not link it onto kernel_dsos
> - * list).
> - *
> - * Therefore, we pass PERF_RECORD_MISC_CPUMODE_UNKNOWN.
> - * is_kernel_module() treats it as a kernel cpumode.
> - */
> -
> - if (!dso->kernel ||
> - is_kernel_module(dso->long_name,
> - PERF_RECORD_MISC_CPUMODE_UNKNOWN))
> - continue;
> -
> -
> - kernel = dso;
> - break;
> - }
> -
> + kernel = dso__get(machine->kernel_dso);
> pthread_rwlock_unlock(&machine->dsos.lock);
>
> if (kernel == NULL)
> diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
> index 9dfc4281f940..d7b044a1046f 100644
> --- a/tools/perf/util/machine.h
> +++ b/tools/perf/util/machine.h
> @@ -38,6 +38,7 @@ struct machine {
> struct dsos dsos;
> struct map_groups kmaps;
> struct map *vmlinux_maps[MAP__NR_TYPES];
> + struct dso *kernel_dso;
> u64 kernel_start;
> symbol_filter_t symbol_filter;
> pid_t *current_tid;
> --
> 2.1.0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
