From: Arnaldo Carvalho de Melo [mailto:a...@kernel.org] >> >Sent: Thursday, September 24, 2015 10:50 AM >> >To: Wangnan (F) >> >Cc: linux-kernel@vger.kernel.org; lize...@huawei.com; pi3or...@163.com; >> >Namhyung Kim; Jiri Olsa; 平松雅巳 / HIRAMATU,MASAMI >> >Subject: Re: [PATCH] perf probe: Fix module probing with shortname >> > >> >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: >> >> Hmm, this is a bit hacky, but yes, it looks good to me. > >You mean the [] dso->short_name part? But that was already being taken >into account by the previous, dso list search.
Ah, OK. I didn't know that... >But yeah, probably we should remove those brackets, anything requiring >full disambiguation should use the dso->long_name anyway, but I'll >leave this for later :) OK, thanks, > >> However, I hope to have above solution, for future use. > >Right, my work now is to reduce the use of dso->kernel, using >__map__is_kernel(map), we'll see what is left. > >> Acked-by: Masami Hiramatsu <masami.hiramatsu...@hitachi.com> > >Thanks! Thanks! > >> Thanks! >> >> > >> >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;