From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754918AbcEQJHH (ORCPT ); Tue, 17 May 2016 05:07:07 -0400 Received: from szxga03-in.huawei.com ([119.145.14.66]:36251 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751576AbcEQJHE (ORCPT ); Tue, 17 May 2016 05:07:04 -0400 Subject: Re: [PATCH v3 1/7 UPDATE] perf tools: Find vdso with the consider of cross-platform To: Adrian Hunter References: <573455BA.1070707@intel.com> <1463129509-160934-1-git-send-email-hekuang@huawei.com> <573AC939.8070107@intel.com> CC: , , , , , , , , , , , , , , , , From: Hekuang Message-ID: <573ADECF.1070408@huawei.com> Date: Tue, 17 May 2016 17:05:19 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <573AC939.8070107@intel.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.110.55.166] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A090203.573ADEDB.006F,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2013-05-26 15:14:31, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 58234dc6566b6764e999c413c791d959 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 在 2016/5/17 15:33, Adrian Hunter 写道: > 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. Thanks, update >> + 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; >> >