[PATCH 0/3] perf fixes

[PATCH 0/3] perf fixes

[Stanislav Fomichev]

This series contains couple of unrelated changes. I already sent two of them
(perf util, perf tools) to the lkml but didn't get any response.
Thus resending them along with new perf timechart fix.

- perf timechart: fix wrong SVG height
  removes unused empty space from generated SVG when called with -p 0 option

- perf util: free cpu_map in perf_session__cpu_bitmap
  frees temporary cpu_map

- perf tools: bring back old behavior when NO_DEMAGLE doesn't link with libbfd
  this is a patch I'd like to get feedback on. The motivation behind it is to
  have a possibility to link without libbfd (because on 3.10 lts NO_DEMANGLE=1
  does not link with libbfd and I'd like to preserve this behavior).

Stanislav Fomichev (3):
  perf timechart: fix wrong SVG height
  perf util: free cpu_map in perf_session__cpu_bitmap
  perf tools: bring back old behavior when NO_DEMAGLE doesn't link with
    libbfd

 tools/perf/builtin-timechart.c | 3 +++
 tools/perf/config/Makefile     | 8 +++-----
 tools/perf/util/session.c      | 2 ++
 3 files changed, 8 insertions(+), 5 deletions(-)

-- 
1.8.3.2

[PATCH 1/3] perf timechart: fix wrong SVG height

[Stanislav Fomichev]

If we call perf timechart with -p 0 arguments, it means we don't
want any tasks related data. It works, but space for tasks data is
reserved in the generated SVG. Remove this unused empty space via
passing 0 as count to the open_svg.

Signed-off-by: Stanislav Fomichev <stfomichev@yandex-team.ru>
---
 tools/perf/builtin-timechart.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index 652af0b66a62..25526d6eae59 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -1045,6 +1045,9 @@ static void write_svg_file(struct timechart *tchart, const char *filename)
 		thresh /= 10;
 	} while (!process_filter && thresh && count < tchart->proc_num);
 
+	if (!tchart->proc_num)
+		count = 0;
+
 	open_svg(filename, tchart->numcpus, count, tchart->first_time, tchart->last_time);
 
 	svg_time_grid();
-- 
1.8.3.2

[PATCH 2/3] perf util: free cpu_map in perf_session__cpu_bitmap

[Stanislav Fomichev]

Signed-off-by: Stanislav Fomichev <stfomichev@yandex-team.ru>
---
 tools/perf/util/session.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 7acc03e8f3b2..03815af30b16 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1600,6 +1600,7 @@ int perf_session__cpu_bitmap(struct perf_session *session,
 		int cpu = map->map[i];
 
 		if (cpu >= MAX_NR_CPUS) {
+			cpu_map__delete(map);
 			pr_err("Requested CPU %d too large. "
 			       "Consider raising MAX_NR_CPUS\n", cpu);
 			return -1;
@@ -1607,6 +1608,7 @@ int perf_session__cpu_bitmap(struct perf_session *session,
 
 		set_bit(cpu, cpu_bitmap);
 	}
+	cpu_map__delete(map);
 
 	return 0;
 }
-- 
1.8.3.2

[PATCH 3/3] perf tools: bring back old behavior when NO_DEMAGLE doesn't link with libbfd

[Stanislav Fomichev]

This commit reverts part of the 3e6a147deef9 "perf tools: Separate lbfd
check out of NO_DEMANGLE condition" which always links perf with libbfd.
I'd like to preserve old behavior when NO_DEMAGLE does not link with
it, because some machines may contain different versions of binutils
(hence miss required libbfd version) and I still want an option to build perf
which works on any machine regardless of binutils version.

Signed-off-by: Stanislav Fomichev <stfomichev@yandex-team.ru>
---
 tools/perf/config/Makefile | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index d604e50fc167..4b76865c9bef 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -477,10 +477,6 @@ else
   endif
 endif
 
-ifeq ($(feature-libbfd), 1)
-  EXTLIBS += -lbfd
-endif
-
 ifdef NO_DEMANGLE
   CFLAGS += -DNO_DEMANGLE
 else
@@ -488,7 +484,9 @@ else
     EXTLIBS += -liberty
     CFLAGS += -DHAVE_CPLUS_DEMANGLE_SUPPORT
   else
-    ifneq ($(feature-libbfd), 1)
+    ifeq ($(feature-libbfd), 1)
+      EXTLIBS += -lbfd
+    else
       $(call feature_check,liberty)
       ifeq ($(feature-liberty), 1)
         EXTLIBS += -lbfd -liberty
-- 
1.8.3.2

Re: [PATCH 3/3] perf tools: bring back old behavior when NO_DEMAGLE doesn't link with libbfd

[Arnaldo Carvalho de Melo]

Em Mon, Jan 20, 2014 at 03:39:40PM +0400, Stanislav Fomichev escreveu:
> This commit reverts part of the 3e6a147deef9 "perf tools: Separate lbfd
> check out of NO_DEMANGLE condition" which always links perf with libbfd.
> I'd like to preserve old behavior when NO_DEMAGLE does not link with
> it, because some machines may contain different versions of binutils
> (hence miss required libbfd version) and I still want an option to build perf
> which works on any machine regardless of binutils version.

This is a tricky part, with another recent patch touching it to make it
work on opensuse, where a linker script is not present to ask for extra
libs.

IIRC Andi's change made the if-else block to be irrelevant, if IIRC from
reading Namhyung and Jiri Olsa comments, so, what were the tests you
performed? I.e. which distros did you test your change on?

For reference, this is the change from Andi to make it work on some
opensuse release:

http://lkml.kernel.org/r/1389661461-18996-2-git-send-email-andi@firstfloor.org

I processed the two other patches, thanks,


- Arnaldo
 
> Signed-off-by: Stanislav Fomichev <stfomichev@yandex-team.ru>
> ---
>  tools/perf/config/Makefile | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
> index d604e50fc167..4b76865c9bef 100644
> --- a/tools/perf/config/Makefile
> +++ b/tools/perf/config/Makefile
> @@ -477,10 +477,6 @@ else
>    endif
>  endif
>  
> -ifeq ($(feature-libbfd), 1)
> -  EXTLIBS += -lbfd
> -endif
> -
>  ifdef NO_DEMANGLE
>    CFLAGS += -DNO_DEMANGLE
>  else
> @@ -488,7 +484,9 @@ else
>      EXTLIBS += -liberty
>      CFLAGS += -DHAVE_CPLUS_DEMANGLE_SUPPORT
>    else
> -    ifneq ($(feature-libbfd), 1)
> +    ifeq ($(feature-libbfd), 1)
> +      EXTLIBS += -lbfd
> +    else
>        $(call feature_check,liberty)
>        ifeq ($(feature-liberty), 1)
>          EXTLIBS += -lbfd -liberty
> -- 
> 1.8.3.2

Re: [PATCH 3/3] perf tools: bring back old behavior when NO_DEMAGLE doesn't link with libbfd

[Andi Kleen]

On Mon, Jan 20, 2014 at 10:22:25AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Jan 20, 2014 at 03:39:40PM +0400, Stanislav Fomichev escreveu:
> > This commit reverts part of the 3e6a147deef9 "perf tools: Separate lbfd
> > check out of NO_DEMANGLE condition" which always links perf with libbfd.
> > I'd like to preserve old behavior when NO_DEMAGLE does not link with
> > it, because some machines may contain different versions of binutils
> > (hence miss required libbfd version) and I still want an option to build perf
> > which works on any machine regardless of binutils version.

BFD is not only used for demangling, it's also used for decoding 
source lines now. So tying it only to NO_DEMANGLE would not be correct.

-Andi

[tip:perf/urgent] perf timechart: Fix wrong SVG height

[tip-bot for Stanislav Fomichev]

Commit-ID:  3415d8b851307c75a1e8aa16030db9172306df78
Gitweb:     http://git.kernel.org/tip/3415d8b851307c75a1e8aa16030db9172306df78
Author:     Stanislav Fomichev <stfomichev@yandex-team.ru>
AuthorDate: Mon, 20 Jan 2014 15:39:38 +0400
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 20 Jan 2014 16:19:08 -0300

perf timechart: Fix wrong SVG height

If we call perf timechart with -p 0 arguments, it means we don't want
any tasks related data. It works, but space for tasks data is reserved
in the generated SVG. Remove this unused empty space via passing 0 as
count to the open_svg.

Signed-off-by: Stanislav Fomichev <stfomichev@yandex-team.ru>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1390217980-22424-2-git-send-email-stfomichev@yandex-team.ru
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-timechart.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index 652af0b..25526d6 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -1045,6 +1045,9 @@ static void write_svg_file(struct timechart *tchart, const char *filename)
 		thresh /= 10;
 	} while (!process_filter && thresh && count < tchart->proc_num);
 
+	if (!tchart->proc_num)
+		count = 0;
+
 	open_svg(filename, tchart->numcpus, count, tchart->first_time, tchart->last_time);
 
 	svg_time_grid();

[tip:perf/urgent] perf session: Free cpu_map in perf_session__cpu_bitmap

[tip-bot for Stanislav Fomichev]

Commit-ID:  8bac41cbfe2efe55e2b93673b84761ed7dd75f69
Gitweb:     http://git.kernel.org/tip/8bac41cbfe2efe55e2b93673b84761ed7dd75f69
Author:     Stanislav Fomichev <stfomichev@yandex-team.ru>
AuthorDate: Mon, 20 Jan 2014 15:39:39 +0400
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 20 Jan 2014 16:19:08 -0300

perf session: Free cpu_map in perf_session__cpu_bitmap

This method uses a temporary struct cpu_map to figure out the cpus
present in the received cpu list in string form, but it failed to free
it after returning. Fix it.

Signed-off-by: Stanislav Fomichev <stfomichev@yandex-team.ru>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1390217980-22424-3-git-send-email-stfomichev@yandex-team.ru
[ Use goto + err = -1 to do the delete just once, in the normal exit path ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/session.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 7acc03e..0b39a48 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1573,7 +1573,7 @@ next:
 int perf_session__cpu_bitmap(struct perf_session *session,
 			     const char *cpu_list, unsigned long *cpu_bitmap)
 {
-	int i;
+	int i, err = -1;
 	struct cpu_map *map;
 
 	for (i = 0; i < PERF_TYPE_MAX; ++i) {
@@ -1602,13 +1602,17 @@ int perf_session__cpu_bitmap(struct perf_session *session,
 		if (cpu >= MAX_NR_CPUS) {
 			pr_err("Requested CPU %d too large. "
 			       "Consider raising MAX_NR_CPUS\n", cpu);
-			return -1;
+			goto out_delete_map;
 		}
 
 		set_bit(cpu, cpu_bitmap);
 	}
 
-	return 0;
+	err = 0;
+
+out_delete_map:
+	cpu_map__delete(map);
+	return err;
 }
 
 void perf_session__fprintf_info(struct perf_session *session, FILE *fp,