| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20150924155631.GK1897@kernel.org>
Date: Thu, 24 Sep 2015 12:56:31 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Adrian Hunter <adrian.hunter@...el.com>,
Borislav Petkov <bp@...e.de>, David Ahern <dsahern@...il.com>,
Frederic Weisbecker <fweisbec@...il.com>,
Jiri Olsa <jolsa@...hat.com>,
Stephane Eranian <eranian@...gle.com>,
Wang Nan <wangnan0@...wei.com>, Ingo Molnar <mingo@...nel.org>,
平松雅巳 / HIRAMATU，MASAMI
<masami.hiramatsu.pt@...achi.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH/RFC] perf buildid: Cache kernel DSO created when reading
buildid header table
Em Fri, Sep 25, 2015 at 12:01:12AM +0900, Namhyung Kim escreveu:
> 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..
Right, my idea is to delete dso->kernel, to remove this confusion.
Please see my tmp.perf/core branch, there are a few patches there using
alternative methods in places previously using the botched dso->kernel.
> In this case, it seems that mmap events set kernel cpumode but
> build-id events don't.
in the previous sentence you think build-id events don't set
perf_event_attr.misc bits related to cpumode...
> So kernel cpumode in a bulid-id event
> indicates that it is a kernel (vmlinux) dso, right?
While here you ask if it does?
All these are synthesized, it is a matter of looking at the routines
synthesizing them :-)
> I'll test this tomorrow..
Thanks, over time there were areas where multiple people touched and
added different semantics that are biting now, I'm trying to, while
fixing a bug, the one Wang Nan reported, clarify those things.
It is taking more time than I antecipated, as I'm stumbling in what look
like other bugs in the perf-probe codebase :-/
- Arnaldo
> Thanks,
> Namhyung
>
>
> >
> > From: Arnaldo Carvalho de Melo <acme@...hat.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-48fh3cf6pvs4zs2fj4nhc4b5@git.kernel.org
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.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 majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists