2014-01-20 11:46:46

by Stanislav Fomichev

[permalink] [raw]
Subject: [PATCH 0/3] perf fixes

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


2014-01-20 11:40:18

by Stanislav Fomichev

[permalink] [raw]
Subject: [PATCH 1/3] 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 <[email protected]>
---
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

2014-01-20 11:40:25

by Stanislav Fomichev

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

Signed-off-by: Stanislav Fomichev <[email protected]>
---
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

2014-01-20 11:40:22

by Stanislav Fomichev

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

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 <[email protected]>
---
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

2014-01-20 13:22:35

by Arnaldo Carvalho de Melo

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

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/[email protected]

I processed the two other patches, thanks,


- Arnaldo

> Signed-off-by: Stanislav Fomichev <[email protected]>
> ---
> 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

2014-01-20 17:19:58

by Andi Kleen

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

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

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

Commit-ID: 3415d8b851307c75a1e8aa16030db9172306df78
Gitweb: http://git.kernel.org/tip/3415d8b851307c75a1e8aa16030db9172306df78
Author: Stanislav Fomichev <[email protected]>
AuthorDate: Mon, 20 Jan 2014 15:39:38 +0400
Committer: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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();

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

Commit-ID: 8bac41cbfe2efe55e2b93673b84761ed7dd75f69
Gitweb: http://git.kernel.org/tip/8bac41cbfe2efe55e2b93673b84761ed7dd75f69
Author: Stanislav Fomichev <[email protected]>
AuthorDate: Mon, 20 Jan 2014 15:39:39 +0400
Committer: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
[ Use goto + err = -1 to do the delete just once, in the normal exit path ]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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,