Messages in this thread | | | From | Namhyung Kim <> | Date | Tue, 3 Feb 2015 00:03:36 +0900 | Subject | Re: [PATCH 01/42] perf tools: Support to read compressed module from build-id cache |
| |
On Fri, Jan 30, 2015 at 11:32 PM, Jiri Olsa <jolsa@redhat.com> wrote: > On Thu, Jan 29, 2015 at 05:06:42PM +0900, Namhyung Kim wrote: >> The commit c00c48fc6e6e ("perf symbols: Preparation for compressed >> kernel module support") added support for compressed kernel modules >> but it only supports system path DSOs. When a dso is read from >> build-id cache, its filename doesn't end with ".gz" but has build-id. >> In this case, we should fallback to the original dso->name. >> >> Signed-off-by: Namhyung Kim <namhyung@kernel.org> >> --- >> tools/perf/util/symbol-elf.c | 13 ++++++++----- >> 1 file changed, 8 insertions(+), 5 deletions(-) >> >> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c >> index 06fcd1bf98b6..b24f9d8727a8 100644 >> --- a/tools/perf/util/symbol-elf.c >> +++ b/tools/perf/util/symbol-elf.c >> @@ -574,13 +574,16 @@ static int decompress_kmodule(struct dso *dso, const char *name, >> const char *ext = strrchr(name, '.'); >> char tmpbuf[] = "/tmp/perf-kmod-XXXXXX"; >> >> - if ((type != DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP && >> - type != DSO_BINARY_TYPE__GUEST_KMODULE_COMP) || >> - type != dso->symtab_type) >> + if (type != DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP && >> + type != DSO_BINARY_TYPE__GUEST_KMODULE_COMP && >> + type != DSO_BINARY_TYPE__BUILD_ID_CACHE) >> return -1; > > hum, is it possible the type == DSO_BINARY_TYPE__BUILD_ID_CACHE could get in here? > > > --- > for (i = 0; i < DSO_BINARY_TYPE__SYMTAB_CNT; i++) { > struct symsrc *ss = &ss_[ss_pos]; > bool next_slot = false; > > enum dso_binary_type symtab_type = binary_type_symtab[i]; > > if (!dso__is_compatible_symtab_type(dso, kmod, symtab_type)) > continue; > > --- ^^^ this check should rule out buildid symtab_type for kmod dso?
AFAICS symtab_type of BUILD_ID_CACHE always returns true in this function.
> > symsrc__init( > > > I wonder wether we should set special type from compressed binaries (as of now), > or instead try to decompress anything that looks like it's compressed ;-) > it seems more to be more generic and could simplify the code..
I don't know. But it seems only kernel modules are compressed now. If user-level dso also supports compression, we need to think about it again..
Thanks, Namhyung
| |