From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753283AbbBBPD6 (ORCPT ); Mon, 2 Feb 2015 10:03:58 -0500 Received: from mail-ob0-f170.google.com ([209.85.214.170]:60383 "EHLO mail-ob0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751113AbbBBPD4 (ORCPT ); Mon, 2 Feb 2015 10:03:56 -0500 MIME-Version: 1.0 In-Reply-To: <20150130143246.GE6188@krava.brq.redhat.com> References: <1422518843-25818-1-git-send-email-namhyung@kernel.org> <1422518843-25818-2-git-send-email-namhyung@kernel.org> <20150130143246.GE6188@krava.brq.redhat.com> From: Namhyung Kim Date: Tue, 3 Feb 2015 00:03:36 +0900 X-Google-Sender-Auth: 9Sb_GYlYx6entFXJUs1kn5ZyBUo Message-ID: Subject: Re: [PATCH 01/42] perf tools: Support to read compressed module from build-id cache To: Jiri Olsa Cc: Arnaldo Carvalho de Melo , Ingo Molnar , Peter Zijlstra , LKML , David Ahern , Adrian Hunter , Andi Kleen , Stephane Eranian , Frederic Weisbecker Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jan 30, 2015 at 11:32 PM, Jiri Olsa 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 >> --- >> 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