[PATCH 1/2] perf tools: avoid traverse dsos list while find vdso

[PATCH 1/2] perf tools: avoid traverse dsos list while find vdso

[Chenggang Qin]

From: Chenggang Qin <chenggang.qcg@taobao.com>

Vdso is only one in a system. It is not necessory to traverse the
macine->user_dsos list when looking for the dso of vdso.
The flag vdso_found should be replaced by a pointor that point to the dso of
vdso. If the pointer is NULL, dso of vdso have not been created. Else, the
pointor can be returned directly in function vdso__dso_findnew().
The list traversing can be avoided by this method.
Thanks.

Cc: David Ahern <dsahern@gmail.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Yanmin Zhang <yanmin.zhang@intel.com>
Cc: Wu Fengguang <fengguang.wu@intel.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Chenggang Qin <chenggang.qcg@taobao.com>

---
 tools/perf/util/vdso.c |   22 ++++++++--------------
 1 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c
index 3915982..8022ef0 100644
--- a/tools/perf/util/vdso.c
+++ b/tools/perf/util/vdso.c
@@ -13,7 +13,7 @@
 #include "symbol.h"
 #include "linux/string.h"
 
-static bool vdso_found;
+static struct dso *vdso_dso = NULL;
 static char vdso_file[] = "/tmp/perf-vdso.so-XXXXXX";
 
 static int find_vdso_map(void **start, void **end)
@@ -55,9 +55,6 @@ static char *get_file(void)
 	size_t size;
 	int fd;
 
-	if (vdso_found)
-		return vdso_file;
-
 	if (find_vdso_map(&start, &end))
 		return NULL;
 
@@ -79,33 +76,30 @@ static char *get_file(void)
  out:
 	free(buf);
 
-	vdso_found = (vdso != NULL);
 	return vdso;
 }
 
 void vdso__exit(void)
 {
-	if (vdso_found)
+	if (vdso_dso)
 		unlink(vdso_file);
 }
 
 struct dso *vdso__dso_findnew(struct list_head *head)
 {
-	struct dso *dso = dsos__find(head, VDSO__MAP_NAME, true);
-
-	if (!dso) {
+	if (!vdso_dso) {
 		char *file;
 
 		file = get_file();
 		if (!file)
 			return NULL;
 
-		dso = dso__new(VDSO__MAP_NAME);
-		if (dso != NULL) {
-			dsos__add(head, dso);
-			dso__set_long_name(dso, file);
+		vdso_dso = dso__new(VDSO__MAP_NAME);
+		if (vdso_dso != NULL) {
+			dsos__add(head, vdso_dso);
+			dso__set_long_name(vdso_dso, file);
 		}
 	}
 
-	return dso;
+	return vdso_dso;
 }
-- 
1.7.8.rc2.5.g815b

[PATCH 2/2] perf tools: remove short name compare in dsos__find()

[Chenggang Qin]

From: Chenggang Qin <chenggang.qcg@taobao.com>

If the list traversal is avoided by the last patch, the short name compare in
dsos__find() is unnecessary. The purpose of short name compare is only to find
the dso of vdso. If the vdso can be found by a pointor, the short name compare
can be removed.
Thanks

Cc: David Ahern <dsahern@gmail.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Yanmin Zhang <yanmin.zhang@intel.com>
Cc: Wu Fengguang <fengguang.wu@intel.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Chenggang Qin <chenggang.qcg@taobao.com>

---
 tools/perf/util/dso.c |   10 ++--------
 tools/perf/util/dso.h |    3 +--
 2 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index c4374f0..6f7d5a9 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -513,16 +513,10 @@ void dsos__add(struct list_head *head, struct dso *dso)
 	list_add_tail(&dso->node, head);
 }
 
-struct dso *dsos__find(struct list_head *head, const char *name, bool cmp_short)
+struct dso *dsos__find(struct list_head *head, const char *name)
 {
 	struct dso *pos;
 
-	if (cmp_short) {
-		list_for_each_entry(pos, head, node)
-			if (strcmp(pos->short_name, name) == 0)
-				return pos;
-		return NULL;
-	}
 	list_for_each_entry(pos, head, node)
 		if (strcmp(pos->long_name, name) == 0)
 			return pos;
@@ -531,7 +525,7 @@ struct dso *dsos__find(struct list_head *head, const char *name, bool cmp_short)
 
 struct dso *__dsos__findnew(struct list_head *head, const char *name)
 {
-	struct dso *dso = dsos__find(head, name, false);
+	struct dso *dso = dsos__find(head, name);
 
 	if (!dso) {
 		dso = dso__new(name);
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index d51aaf2..450199a 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -133,8 +133,7 @@ struct dso *dso__kernel_findnew(struct machine *machine, const char *name,
 				const char *short_name, int dso_type);
 
 void dsos__add(struct list_head *head, struct dso *dso);
-struct dso *dsos__find(struct list_head *head, const char *name,
-		       bool cmp_short);
+struct dso *dsos__find(struct list_head *head, const char *name);
 struct dso *__dsos__findnew(struct list_head *head, const char *name);
 bool __dsos__read_build_ids(struct list_head *head, bool with_hits);
 
-- 
1.7.8.rc2.5.g815b

Re: [PATCH 1/2] perf tools: avoid traverse dsos list while find vdso

[Arnaldo Carvalho de Melo]

Em Mon, Sep 09, 2013 at 09:21:55PM +0800, Chenggang Qin escreveu:
> From: Chenggang Qin <chenggang.qcg@taobao.com>
> 
> Vdso is only one in a system. It is not necessory to traverse the
> macine->user_dsos list when looking for the dso of vdso.
> The flag vdso_found should be replaced by a pointor that point to the dso of
> vdso. If the pointer is NULL, dso of vdso have not been created. Else, the
> pointor can be returned directly in function vdso__dso_findnew().
> The list traversing can be avoided by this method.
> Thanks.
> 
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
> Cc: Arjan van de Ven <arjan@linux.intel.com>
> Cc: Namhyung Kim <namhyung@gmail.com>
> Cc: Yanmin Zhang <yanmin.zhang@intel.com>
> Cc: Wu Fengguang <fengguang.wu@intel.com>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Chenggang Qin <chenggang.qcg@taobao.com>
> 
> ---
>  tools/perf/util/vdso.c |   22 ++++++++--------------
>  1 files changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c
> index 3915982..8022ef0 100644
> --- a/tools/perf/util/vdso.c
> +++ b/tools/perf/util/vdso.c
> @@ -13,7 +13,7 @@
>  #include "symbol.h"
>  #include "linux/string.h"
>  
> -static bool vdso_found;
> +static struct dso *vdso_dso = NULL;
>  static char vdso_file[] = "/tmp/perf-vdso.so-XXXXXX";
>  
>  static int find_vdso_map(void **start, void **end)
> @@ -55,9 +55,6 @@ static char *get_file(void)
>  	size_t size;
>  	int fd;
>  
> -	if (vdso_found)
> -		return vdso_file;
> -
>  	if (find_vdso_map(&start, &end))
>  		return NULL;
>  
> @@ -79,33 +76,30 @@ static char *get_file(void)
>   out:
>  	free(buf);
>  
> -	vdso_found = (vdso != NULL);
>  	return vdso;
>  }
>  
>  void vdso__exit(void)
>  {
> -	if (vdso_found)
> +	if (vdso_dso)
>  		unlink(vdso_file);

Don't we have to delete the vfso_dso and set it to NULL?

>  }
>  
>  struct dso *vdso__dso_findnew(struct list_head *head)
>  {
> -	struct dso *dso = dsos__find(head, VDSO__MAP_NAME, true);
> -
> -	if (!dso) {
> +	if (!vdso_dso) {
>  		char *file;
>  
>  		file = get_file();
>  		if (!file)
>  			return NULL;
>  
> -		dso = dso__new(VDSO__MAP_NAME);
> -		if (dso != NULL) {
> -			dsos__add(head, dso);
> -			dso__set_long_name(dso, file);
> +		vdso_dso = dso__new(VDSO__MAP_NAME);
> +		if (vdso_dso != NULL) {
> +			dsos__add(head, vdso_dso);
> +			dso__set_long_name(vdso_dso, file);
>  		}
>  	}
>  
> -	return dso;
> +	return vdso_dso;
>  }
> -- 
> 1.7.8.rc2.5.g815b