Re: [PATCH v3 1/7 UPDATE] perf tools: Find vdso with the consider of cross-platform

[Hekuang]

在 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 <heku...@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..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;