From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754363AbcEQHhW (ORCPT ); Tue, 17 May 2016 03:37:22 -0400 Received: from mga02.intel.com ([134.134.136.20]:27594 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751983AbcEQHhS (ORCPT ); Tue, 17 May 2016 03:37:18 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,324,1459839600"; d="scan'208";a="978512445" Subject: Re: [PATCH v3 1/7 UPDATE] perf tools: Find vdso with the consider of cross-platform To: He Kuang References: <573455BA.1070707@intel.com> <1463129509-160934-1-git-send-email-hekuang@huawei.com> Cc: peterz@infradead.org, mingo@redhat.com, acme@kernel.org, alexander.shishkin@linux.intel.com, jolsa@redhat.com, wangnan0@huawei.com, jpoimboe@redhat.com, ak@linux.intel.com, eranian@google.com, namhyung@kernel.org, sukadev@linux.vnet.ibm.com, masami.hiramatsu.pt@hitachi.com, tumanova@linux.vnet.ibm.com, kan.liang@intel.com, penberg@kernel.org, dsahern@gmail.com, linux-kernel@vger.kernel.org From: Adrian Hunter Organization: Intel Finland Oy, Registered Address: PL 281, 00181 Helsinki, Business Identity Code: 0357606 - 4, Domiciled in Helsinki Message-ID: <573AC939.8070107@intel.com> Date: Tue, 17 May 2016 10:33:13 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <1463129509-160934-1-git-send-email-hekuang@huawei.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 13/05/16 11:51, He Kuang wrote: > There's a problem in machine__findnew_vdso(), vdso buildid generated > by a 32-bit machine stores it with the name 'vdso', but when > processing buildid on a 64-bit machine with the same 'perf.data', perf > will search for vdso named as 'vdso32' and get failed. > > This patch tries to find the exsiting dsos in machine->dsos by thread > dso_type. 64-bit thread tries to find vdso with name 'vdso', because > all 64-bit vdso is named as that. 32-bit thread first tries to find > vdso with name 'vdso32' if this thread was run on 64-bit machine, if > failed, then it tries 'vdso' which indicates that the thread was run > on 32-bit machine when recording. > > Signed-off-by: He Kuang > --- > tools/perf/util/vdso.c | 40 +++++++++++++++++++++++++++++++++++++--- > 1 file changed, 37 insertions(+), 3 deletions(-) > > diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c > index 44d440d..99f4a3d 100644 > --- a/tools/perf/util/vdso.c > +++ b/tools/perf/util/vdso.c > @@ -134,8 +134,6 @@ static struct dso *__machine__addnew_vdso(struct machine *machine, const char *s > return dso; > } > > -#if BITS_PER_LONG == 64 > - > static enum dso_type machine__thread_dso_type(struct machine *machine, > struct thread *thread) > { > @@ -156,6 +154,8 @@ static enum dso_type machine__thread_dso_type(struct machine *machine, > return dso_type; > } > > +#if BITS_PER_LONG == 64 > + > static int vdso__do_copy_compat(FILE *f, int fd) > { > char buf[4096]; > @@ -283,8 +283,38 @@ static int __machine__findnew_vdso_compat(struct machine *machine, > > #endif > > +static struct dso *machine__find_vdso(struct machine *machine, > + struct thread *thread) > +{ > + struct dso *dso = NULL; > + enum dso_type dso_type; > + > + dso_type = machine__thread_dso_type(machine, thread); > + switch (dso_type) { > + case DSO__TYPE_32BIT: > + dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO32, true); > + if (!dso) > + dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO, > + true); So if we have not yet added the 32-bit vdso but have added the 64-bit vdso, we will return the wrong one. Can we check it? e.g. if (!dso) dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO, true); if (dso_type != dso__type(dso, machine)) dso = NULL; > + break; > + case DSO__TYPE_X32BIT: > + dso = __dsos__find(&machine->dsos, DSO__NAME_VDSOX32, true); > + if (!dso) > + dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO, > + true); The x32 vdso is never called DSO__NAME_VDSO so this is not correct, but for the same reason we don't need this __dsos__find() anyway. > + break; > + case DSO__TYPE_64BIT: > + case DSO__TYPE_UNKNOWN: > + default: > + dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO, true); > + break; > + } > + > + return dso; > +} > + > struct dso *machine__findnew_vdso(struct machine *machine, > - struct thread *thread __maybe_unused) > + struct thread *thread) > { > struct vdso_info *vdso_info; > struct dso *dso = NULL; > @@ -297,6 +327,10 @@ struct dso *machine__findnew_vdso(struct machine *machine, > if (!vdso_info) > goto out_unlock; > > + dso = machine__find_vdso(machine, thread); > + if (dso) > + goto out_unlock; > + > #if BITS_PER_LONG == 64 > if (__machine__findnew_vdso_compat(machine, thread, vdso_info, &dso)) > goto out_unlock; >