Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1399758 > unrolled thread

[PATCH v3 1/7] perf tools: Set vdso name to vdso[64,32] depending on platform

Started byHe Kuang <hekuang@huawei.com>
First post2016-05-12 10:50 +0200
Last post2016-05-13 11:00 +0200
Articles 11 — 4 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.


Contents

  [PATCH v3 1/7] perf tools: Set vdso name to vdso[64,32] depending on platform He Kuang <hekuang@huawei.com> - 2016-05-12 10:50 +0200
    Re: [PATCH v3 1/7] perf tools: Set vdso name to vdso[64,32] depending  on platform Adrian Hunter <adrian.hunter@intel.com> - 2016-05-12 12:20 +0200
      [PATCH v3 1/7 UPDATE] perf tools: Find vdso with the consider of cross-platform He Kuang <hekuang@huawei.com> - 2016-05-13 11:00 +0200
        Re: [PATCH v3 1/7 UPDATE] perf tools: Find vdso with the consider of  cross-platform Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-05-16 15:40 +0200
          Re: [PATCH v3 1/7 UPDATE] perf tools: Find vdso with the consider of  cross-platform Adrian Hunter <adrian.hunter@intel.com> - 2016-05-17 09:40 +0200
        Re: [PATCH v3 1/7 UPDATE] perf tools: Find vdso with the consider of  cross-platform Adrian Hunter <adrian.hunter@intel.com> - 2016-05-17 09:40 +0200
          [PATCH v3 1/7 UPDATE2] perf tools: Find vdso with the consider of cross-platform He Kuang <hekuang@huawei.com> - 2016-05-17 11:10 +0200
            Re: [PATCH v3 1/7 UPDATE2] perf tools: Find vdso with the consider of  cross-platform Adrian Hunter <adrian.hunter@intel.com> - 2016-05-17 11:30 +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-05-17 14:50 +0200
          Re: [PATCH v3 1/7 UPDATE] perf tools: Find vdso with the consider of  cross-platform Hekuang <hekuang@huawei.com> - 2016-05-17 11:10 +0200
      Re: [PATCH v3 1/7] perf tools: Set vdso name to vdso[64,32] depending  on platform Hekuang <hekuang@huawei.com> - 2016-05-13 11:00 +0200

#1399758 — [PATCH v3 1/7] perf tools: Set vdso name to vdso[64,32] depending on platform

FromHe Kuang <hekuang@huawei.com>
Date2016-05-12 10:50 +0200
Subject[PATCH v3 1/7] perf tools: Set vdso name to vdso[64,32] depending on platform
Message-ID<rxUtI-1Wg-3@gated-at.bofh.it>
This is a preparation for cross-platform vdso lookup.

There is a naming confusion about vdso name, 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 uses different names when storing the buildid, i.e. vdso64
for 64-bit machine and vdso32 for 32-bit machine, and eliminates this
naming confusion.

Signed-off-by: He Kuang <hekuang@huawei.com>
---
 tools/perf/util/vdso.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/vdso.h b/tools/perf/util/vdso.h
index cdc4fab..45e9ef4 100644
--- a/tools/perf/util/vdso.h
+++ b/tools/perf/util/vdso.h
@@ -4,10 +4,15 @@
 #include <linux/types.h>
 #include <string.h>
 #include <stdbool.h>
+#include "util.h"
 
 #define VDSO__MAP_NAME "[vdso]"
 
-#define DSO__NAME_VDSO    "[vdso]"
+#if BITS_PER_LONG == 64
+#define DSO__NAME_VDSO    "[vdso64]"
+#else
+#define DSO__NAME_VDSO    "[vdso32]"
+#endif
 #define DSO__NAME_VDSO32  "[vdso32]"
 #define DSO__NAME_VDSOX32 "[vdsox32]"
 
-- 
1.8.5.2

[toc] | [next] | [standalone]


#1399863 — Re: [PATCH v3 1/7] perf tools: Set vdso name to vdso[64,32] depending on platform

FromAdrian Hunter <adrian.hunter@intel.com>
Date2016-05-12 12:20 +0200
SubjectRe: [PATCH v3 1/7] perf tools: Set vdso name to vdso[64,32] depending on platform
Message-ID<rxVSO-3yT-21@gated-at.bofh.it>
In reply to#1399758
On 12/05/16 11:43, He Kuang wrote:
> This is a preparation for cross-platform vdso lookup.
> 
> There is a naming confusion about vdso name, 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 uses different names when storing the buildid, i.e. vdso64
> for 64-bit machine and vdso32 for 32-bit machine, and eliminates this
> naming confusion.

That looks like it will break existing perf.data files because they will
have a different name recorded in the buildid section.

Also it doesn't look like it would work the other way around i.e. recording
on a 64-bit machine and processing on a 32-bit machine.

> 
> Signed-off-by: He Kuang <hekuang@huawei.com>
> ---
>  tools/perf/util/vdso.h | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/vdso.h b/tools/perf/util/vdso.h
> index cdc4fab..45e9ef4 100644
> --- a/tools/perf/util/vdso.h
> +++ b/tools/perf/util/vdso.h
> @@ -4,10 +4,15 @@
>  #include <linux/types.h>
>  #include <string.h>
>  #include <stdbool.h>
> +#include "util.h"
>  
>  #define VDSO__MAP_NAME "[vdso]"
>  
> -#define DSO__NAME_VDSO    "[vdso]"
> +#if BITS_PER_LONG == 64
> +#define DSO__NAME_VDSO    "[vdso64]"
> +#else
> +#define DSO__NAME_VDSO    "[vdso32]"
> +#endif
>  #define DSO__NAME_VDSO32  "[vdso32]"
>  #define DSO__NAME_VDSOX32 "[vdsox32]"
>  
> 

[toc] | [prev] | [next] | [standalone]


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

FromHe Kuang <hekuang@huawei.com>
Date2016-05-13 11:00 +0200
Subject[PATCH v3 1/7 UPDATE] perf tools: Find vdso with the consider of cross-platform
Message-ID<ryh6W-8oG-1@gated-at.bofh.it>
In reply to#1399863
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 <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..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);
+		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);
+		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]


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

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2016-05-16 15:40 +0200
SubjectRe: [PATCH v3 1/7 UPDATE] perf tools: Find vdso with the consider of cross-platform
Message-ID<rzqUx-7Dg-5@gated-at.bofh.it>
In reply to#1400621
Em Fri, May 13, 2016 at 08:51:49AM +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.
> 
> 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.

Adrian, are you ok now?


- Arnaldo
 
> 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..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);
> +		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);
> +		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]


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

FromAdrian Hunter <adrian.hunter@intel.com>
Date2016-05-17 09:40 +0200
SubjectRe: [PATCH v3 1/7 UPDATE] perf tools: Find vdso with the consider of cross-platform
Message-ID<rzHLH-1Co-3@gated-at.bofh.it>
In reply to#1401496
On 16/05/16 16:32, Arnaldo Carvalho de Melo wrote:
> Em Fri, May 13, 2016 at 08:51:49AM +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.
>>
>> 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.
> 
> Adrian, are you ok now?

I sent a couple more comments.

[toc] | [prev] | [next] | [standalone]


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

FromAdrian Hunter <adrian.hunter@intel.com>
Date2016-05-17 09:40 +0200
SubjectRe: [PATCH v3 1/7 UPDATE] perf tools: Find vdso with the consider of cross-platform
Message-ID<rzHLH-1Co-15@gated-at.bofh.it>
In reply to#1400621
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 <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..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;
> 

[toc] | [prev] | [next] | [standalone]


#1402208 — [PATCH v3 1/7 UPDATE2] perf tools: Find vdso with the consider of cross-platform

FromHe Kuang <hekuang@huawei.com>
Date2016-05-17 11:10 +0200
Subject[PATCH v3 1/7 UPDATE2] perf tools: Find vdso with the consider of cross-platform
Message-ID<rzJaS-2C9-9@gated-at.bofh.it>
In reply to#1402159
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 <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]


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

FromAdrian Hunter <adrian.hunter@intel.com>
Date2016-05-17 11:30 +0200
SubjectRe: [PATCH v3 1/7 UPDATE2] perf tools: Find vdso with the consider of cross-platform
Message-ID<rzJue-2JE-11@gated-at.bofh.it>
In reply to#1402208
On 17/05/16 12:04, 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 <hekuang@huawei.com>

Looks OK.

Acked-by: Adrian Hunter <adrian.hunter@intel.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;
> 

[toc] | [prev] | [next] | [standalone]


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

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2016-05-17 14:50 +0200
SubjectRe: [PATCH v3 1/7 UPDATE2] perf tools: Find vdso with the consider of cross-platform
Message-ID<rzMBH-4CW-1@gated-at.bofh.it>
In reply to#1402220
Em Tue, May 17, 2016 at 12:17:29PM +0300, Adrian Hunter escreveu:
> On 17/05/16 12:04, 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 <hekuang@huawei.com>
 
> Looks OK.
 
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>

Thanks, applied.

- Arnaldo

[toc] | [prev] | [next] | [standalone]


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

FromHekuang <hekuang@huawei.com>
Date2016-05-17 11:10 +0200
SubjectRe: [PATCH v3 1/7 UPDATE] perf tools: Find vdso with the consider of cross-platform
Message-ID<rzJaT-2C9-21@gated-at.bofh.it>
In reply to#1402159

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

[toc] | [prev] | [next] | [standalone]


#1400622 — Re: [PATCH v3 1/7] perf tools: Set vdso name to vdso[64,32] depending on platform

FromHekuang <hekuang@huawei.com>
Date2016-05-13 11:00 +0200
SubjectRe: [PATCH v3 1/7] perf tools: Set vdso name to vdso[64,32] depending on platform
Message-ID<ryh6W-8oG-7@gated-at.bofh.it>
In reply to#1399863

在 2016/5/12 18:06, Adrian Hunter 写道:
> On 12/05/16 11:43, He Kuang wrote:
>> This is a preparation for cross-platform vdso lookup.
>>
>> There is a naming confusion about vdso name, 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 uses different names when storing the buildid, i.e. vdso64
>> for 64-bit machine and vdso32 for 32-bit machine, and eliminates this
>> naming confusion.
> That looks like it will break existing perf.data files because they will
> have a different name recorded in the buildid section.
>
> Also it doesn't look like it would work the other way around i.e. recording
> on a 64-bit machine and processing on a 32-bit machine.

Yes, please have a look at the new patch.

Thanks
>> Signed-off-by: He Kuang <hekuang@huawei.com>
>> ---
>>   tools/perf/util/vdso.h | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/util/vdso.h b/tools/perf/util/vdso.h
>> index cdc4fab..45e9ef4 100644
>> --- a/tools/perf/util/vdso.h
>> +++ b/tools/perf/util/vdso.h
>> @@ -4,10 +4,15 @@
>>   #include <linux/types.h>
>>   #include <string.h>
>>   #include <stdbool.h>
>> +#include "util.h"
>>   
>>   #define VDSO__MAP_NAME "[vdso]"
>>   
>> -#define DSO__NAME_VDSO    "[vdso]"
>> +#if BITS_PER_LONG == 64
>> +#define DSO__NAME_VDSO    "[vdso64]"
>> +#else
>> +#define DSO__NAME_VDSO    "[vdso32]"
>> +#endif
>>   #define DSO__NAME_VDSO32  "[vdso32]"
>>   #define DSO__NAME_VDSOX32 "[vdsox32]"
>>   
>>
>

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web