Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1423023 > unrolled thread
| Started by | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| First post | 2016-06-15 15:40 +0200 |
| Last post | 2016-06-16 18:40 +0200 |
| Articles | 4 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH v3 1/7 UPDATE2] perf tools: Find vdso with the consider of cross-platform Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-06-15 15:40 +0200
Re: [PATCH v3 1/7 UPDATE2] perf tools: Find vdso with the consider of cross-platform Hekuang <hekuang@huawei.com> - 2016-06-16 02:30 +0200
Re: [PATCH v3 1/7 UPDATE2] perf tools: Find vdso with the consider of cross-platform Adrian Hunter <adrian.hunter@intel.com> - 2016-06-16 15:10 +0200
Re: [PATCH v3 1/7 UPDATE2] perf tools: Find vdso with the consider of cross-platform Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-06-16 18:40 +0200
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2016-06-15 15:40 +0200 |
| Subject | Re: [PATCH v3 1/7 UPDATE2] perf tools: Find vdso with the consider of cross-platform |
| Message-ID | <rKjcZ-37G-9@gated-at.bofh.it> |
Em Tue, May 17, 2016 at 09:04:54AM +0000, He Kuang escreveu:
> 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.
Without looking at the code, just trying to understand your patch by
reading your description: Why would we look for names if we have
build-ids? All this type and name comparasions seems wrong if we have a
build-id, no?
Adrian?
- Arnaldo
> 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 <hekuang@huawei.com>
> ---
> 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..8f81c41 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);
> + if (dso_type != dso__type(dso, machine))
> + dso = NULL;
> + }
> + break;
> + case DSO__TYPE_X32BIT:
> + dso = __dsos__find(&machine->dsos, DSO__NAME_VDSOX32, true);
> + 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;
> --
> 1.8.5.2
[toc] | [next] | [standalone]
| From | Hekuang <hekuang@huawei.com> |
|---|---|
| Date | 2016-06-16 02:30 +0200 |
| Subject | Re: [PATCH v3 1/7 UPDATE2] perf tools: Find vdso with the consider of cross-platform |
| Message-ID | <rKtm2-1aF-31@gated-at.bofh.it> |
| In reply to | #1423023 |
hi
在 2016/6/15 21:34, Arnaldo Carvalho de Melo 写道:
> Em Tue, May 17, 2016 at 09:04:54AM +0000, He Kuang escreveu:
>> 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.
> Without looking at the code, just trying to understand your patch by
> reading your description: Why would we look for names if we have
> build-ids? All this type and name comparasions seems wrong if we have a
> build-id, no?
>
> Adrian?
>
> - Arnaldo
>
We should find a binary with the {name, buildid} pair, the key is
name not buildid.
There're more than one vdso binaries in system, we store
parts/all of them when recording and should find out which one to
use for unwinding in the 'perf script' stage, by comparing the
name and the thread's dso_type.
Thank you.
>> 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 <hekuang@huawei.com>
>> ---
>> 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..8f81c41 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);
>> + if (dso_type != dso__type(dso, machine))
>> + dso = NULL;
>> + }
>> + break;
>> + case DSO__TYPE_X32BIT:
>> + dso = __dsos__find(&machine->dsos, DSO__NAME_VDSOX32, true);
>> + 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;
>> --
>> 1.8.5.2
[toc] | [prev] | [next] | [standalone]
| From | Adrian Hunter <adrian.hunter@intel.com> |
|---|---|
| Date | 2016-06-16 15:10 +0200 |
| Subject | Re: [PATCH v3 1/7 UPDATE2] perf tools: Find vdso with the consider of cross-platform |
| Message-ID | <rKFdw-lG-9@gated-at.bofh.it> |
| In reply to | #1423023 |
On 15/06/16 16:34, Arnaldo Carvalho de Melo wrote: > Em Tue, May 17, 2016 at 09:04:54AM +0000, He Kuang escreveu: >> 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. > > Without looking at the code, just trying to understand your patch by > reading your description: Why would we look for names if we have > build-ids? All this type and name comparasions seems wrong if we have a > build-id, no? > > Adrian? We match maps to builds ids using the file name - consider machine__findnew_[v]dso() called in map__new(). So in the context of a perf data file, we consider the file name to be unique. A vdso map does not have a file name - all we know is that it is vdso. We look at the thread to tell if it is 32-bit, 64-bit or x32. Then we need to get the build id which has been recorded using short name "[vdso]" or "[vdso32]" or "[vdsox32]". The problem is that on a 32-bit machine, we use the name "[vdso]". If you take a 32-bit perf data file to a 64-bit machine, it gets hard to figure out if "[vdso]" is 32-bit or 64-bit. This patch solves that problem. I acked it in another email.
[toc] | [prev] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2016-06-16 18:40 +0200 |
| Message-ID | <rKIuJ-2fo-1@gated-at.bofh.it> |
| In reply to | #1424034 |
Em Thu, Jun 16, 2016 at 04:00:12PM +0300, Adrian Hunter escreveu: > On 15/06/16 16:34, Arnaldo Carvalho de Melo wrote: > > Em Tue, May 17, 2016 at 09:04:54AM +0000, He Kuang escreveu: > >> 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. > > > > Without looking at the code, just trying to understand your patch by > > reading your description: Why would we look for names if we have > > build-ids? All this type and name comparasions seems wrong if we have a > > build-id, no? > > > > Adrian? > > We match maps to builds ids using the file name - consider > machine__findnew_[v]dso() called in map__new(). So in the context of a perf > data file, we consider the file name to be unique. > > A vdso map does not have a file name - all we know is that it is vdso. We > look at the thread to tell if it is 32-bit, 64-bit or x32. Then we need to > get the build id which has been recorded using short name "[vdso]" or > "[vdso32]" or "[vdsox32]". > > The problem is that on a 32-bit machine, we use the name "[vdso]". If you > take a 32-bit perf data file to a 64-bit machine, it gets hard to figure out > if "[vdso]" is 32-bit or 64-bit. > > This patch solves that problem. > > I acked it in another email. Ok, somehow this got lost, I thought to have applied it already, but re-reading the changeset comment, I got confused, thanks for providing this explanation, that I will stick to the changeset log for this message, to help understanding this when scratching our heads in the future. - Arnaldo
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web