From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: "Wangnan (F)" <wangnan0@huawei.com>
Cc: linux-kernel@vger.kernel.org, lizefan@huawei.com,
pi3orama@163.com, Namhyung Kim <namhyung@kernel.org>,
Jiri Olsa <jolsa@redhat.com>,
Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Subject: Re: [PATCH] perf probe: Fix module probing with shortname
Date: Wed, 23 Sep 2015 22:50:08 -0300 [thread overview]
Message-ID: <20150924015008.GE1897@kernel.org> (raw)
In-Reply-To: <20150923160319.GG5041@redhat.com>
Em Wed, Sep 23, 2015 at 01:03:19PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Sep 22, 2015 at 11:49:02PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Wed, Sep 23, 2015 at 09:14:44AM +0800, Wangnan (F) escreveu:
> > > On 2015/9/22 21:35, Arnaldo Carvalho de Melo wrote:
> > > >Em Tue, Sep 22, 2015 at 03:34:32AM +0000, Wang Nan escreveu:
> > > >>This is caused by a misunderstood of dso->kernel in kernel_get_module_dso()
> > > >>that, for kernel module, dso->kernel is DSO_TYPE_USER. dso->kernel is DSO_TYPE_KERNEL
> > > >>iff dso is vmlinux.
> > > >Kernel modules having DSO_TYPE_USER seems to be the bug, no? I'll try to
> > > >check that...
> > > I also noticed this problem when I working on commit
> > > 1f121b03d058dd07199d8924373d3c52a207f63b ("perf tools: Deal with
> > > kernel module names in '[]' correctly") ;)
> > Thanks for working on this, it is an area that needs cleaning up, too
> > many ways to say what a dso is, will study your findings and try to come
> > up with a patch proposal tomorrow.
> > > So care must be taken.
>
> So, yes, there are multiple uses for this dso->kernel thing, we need to
> look at each one and go on clarifying it so that this gets corrected and
> sane, but I think we need some helpers to clarify all this, namely:
>
> Adding DSO_TYPE_KMODULE and DSO_TYPE_GUEST_KMODULE, setting
> dso->kernel with it when loading host and guest kernel modules and
> adding:
>
> bool dso__is_host_kernel(struct dso *dso)
> bool dso__is_host_kmodule(struct dso *dso)
>
> bool dso__is_guest_kernel(struct dso *dso)
So I tried this, ended up basically using __map__is_kernel(), in several
places, which I think is right, and I've stashed in my tmp.perf/core
branch so that you can take a look.
But then I realized that we already have a better way to achieve what is
needed in that function, see the patch below, fixed things for me:
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 2b78e8f19b45..7fb0533ab18c 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -269,12 +269,13 @@ static int kernel_get_module_dso(const char *module, struct dso **pdso)
int ret = 0;
if (module) {
- list_for_each_entry(dso, &host_machine->dsos.head, node) {
- if (!dso->kernel)
- continue;
- if (strncmp(dso->short_name + 1, module,
- dso->short_name_len - 2) == 0)
- goto found;
+ char module_name[128];
+
+ snprintf(module_name, sizeof(module_name), "[%s]", module);
+ map = map_groups__find_by_name(&host_machine->kmaps, MAP__FUNCTION, module_name);
+ if (map) {
+ dso = map->dso;
+ goto found;
}
pr_debug("Failed to find module %s.\n", module);
return -ENOENT;
next prev parent reply other threads:[~2015-09-24 1:50 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-22 3:34 [PATCH] perf probe: Fix module probing with shortname Wang Nan
2015-09-22 13:35 ` Arnaldo Carvalho de Melo
2015-09-23 1:14 ` Wangnan (F)
2015-09-23 2:49 ` Arnaldo Carvalho de Melo
2015-09-23 16:03 ` Arnaldo Carvalho de Melo
2015-09-24 1:50 ` Arnaldo Carvalho de Melo [this message]
2015-09-24 2:05 ` Arnaldo Carvalho de Melo
2015-09-24 8:28 ` pi3orama
2015-09-24 10:10 ` 平松雅巳 / HIRAMATU,MASAMI
2015-09-24 13:23 ` 'Arnaldo Carvalho de Melo'
2015-09-25 1:57 ` 平松雅巳 / HIRAMATU,MASAMI
2015-09-24 10:18 ` 平松雅巳 / HIRAMATU,MASAMI
2015-09-26 6:21 ` [tip:perf/urgent] perf probe: Use existing routine to look for a kernel module by dso->short_name tip-bot for Arnaldo Carvalho de Melo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150924015008.GE1897@kernel.org \
--to=acme@kernel.org \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lizefan@huawei.com \
--cc=masami.hiramatsu.pt@hitachi.com \
--cc=namhyung@kernel.org \
--cc=pi3orama@163.com \
--cc=wangnan0@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox