perf tools: Introduce trim function

PATCH Applied Jiri Olsa 5 participants 9 messages Apr 7, 2016 – Apr 13, 2016 perf events

lore

Share & tools

lore

Jiri Olsa

author reviewer

2016-04-07 07:11 UTC #0 lore raw

PATCH 1/3 perf tools: Introduce trim function

To be used in cases for both sides trim.

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/ui/browsers/hists.c | 3 +--
 tools/perf/ui/stdio/hist.c     | 3 +--
 tools/perf/util/util.h         | 5 +++++
 3 files changed, 7 insertions(+), 4 deletions(-)

tools/perf/ui/browsers/hists.c +1 -2

index 2a83414159a6..e70df2e54d66 100644
@@ -1607,9 +1607,8 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *brows
 
 			ret = fmt->header(fmt, &dummy_hpp, hists_to_evsel(hists));
 			dummy_hpp.buf[ret] = '\0';
-			rtrim(dummy_hpp.buf);
 
-			start = ltrim(dummy_hpp.buf);
+			start = trim(dummy_hpp.buf);
 			ret = strlen(start);
 
 			if (start != dummy_hpp.buf)

tools/perf/ui/stdio/hist.c +1 -2

index 7aff5acf3265..560eb47d56f9 100644
@@ -569,9 +569,8 @@ static int print_hierarchy_header(struct hists *hists, struct perf_hpp *hpp,
 			first_col = false;
 
 			fmt->header(fmt, hpp, hists_to_evsel(hists));
-			rtrim(hpp->buf);
 
-			header_width += fprintf(fp, "%s", ltrim(hpp->buf));
+			header_width += fprintf(fp, "%s", trim(hpp->buf));
 		}
 	}

tools/perf/util/util.h +5 -0

index 8298d607c738..3bf3de86d429 100644
@@ -254,6 +254,11 @@ int hex2u64(const char *ptr, u64 *val);
 char *ltrim(char *s);
 char *rtrim(char *s);
 
+static inline char *trim(char *s)
+{
+	return ltrim(rtrim(s));
+}
+
 void dump_stack(void);
 void sighandler_dump_stack(int sig);

Signature · 1 line

2.4.11

Jiri Olsa

author reviewer

2016-04-07 07:11 UTC #1 lore raw

PATCH 2/3 perf tools: Add dedicated unwind addr_space member into thread struct

Milian reported issue with thread::priv, which was double booked by perf trace and DWARF unwind code. So using those together is impossible at the moment.

Moving DWARF unwind private data into separate variable so perf trace can keep using thread::priv.

Reported-by Milian Wolff <[email protected]>

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/util/thread.h           |  6 ++++++
 tools/perf/util/unwind-libunwind.c | 25 +++++++++----------------
 2 files changed, 15 insertions(+), 16 deletions(-)

tools/perf/util/thread.h +6 -0

index a0ac0317affb..e214207bb13a 100644
@@ -9,6 +9,9 @@
 #include "symbol.h"
 #include <strlist.h>
 #include <intlist.h>
+#ifdef HAVE_LIBUNWIND_SUPPORT
+#include <libunwind.h>
+#endif
 
 struct thread_stack;

@@ -32,6 +35,9 @@ struct thread {
 
 	void			*priv;
 	struct thread_stack	*ts;
+#ifdef HAVE_LIBUNWIND_SUPPORT
+	unw_addr_space_t	addr_space;
+#endif
 };
 
 struct machine;

tools/perf/util/unwind-libunwind.c +9 ~~-16~~

index ee7e372297e5..63687d3a344e 100644
@@ -32,6 +32,7 @@
 #include "symbol.h"
 #include "util.h"
 #include "debug.h"
+#include "asm/bug.h"
 
 extern int
 UNW_OBJ(dwarf_search_unwind_table) (unw_addr_space_t as,

@@ -580,43 +581,33 @@ static unw_accessors_t accessors = {
 
 int unwind__prepare_access(struct thread *thread)
 {
-	unw_addr_space_t addr_space;
-
 	if (callchain_param.record_mode != CALLCHAIN_DWARF)
 		return 0;
 
-	addr_space = unw_create_addr_space(&accessors, 0);
-	if (!addr_space) {
+	thread->addr_space = unw_create_addr_space(&accessors, 0);
+	if (!thread->addr_space) {
 		pr_err("unwind: Can't create unwind address space.\n");
 		return -ENOMEM;
 	}
 
-	unw_set_caching_policy(addr_space, UNW_CACHE_GLOBAL);
-	thread__set_priv(thread, addr_space);
-
+	unw_set_caching_policy(thread->addr_space, UNW_CACHE_GLOBAL);
 	return 0;
 }
 
 void unwind__flush_access(struct thread *thread)
 {
-	unw_addr_space_t addr_space;
-
 	if (callchain_param.record_mode != CALLCHAIN_DWARF)
 		return;
 
-	addr_space = thread__priv(thread);
-	unw_flush_cache(addr_space, 0, 0);
+	unw_flush_cache(thread->addr_space, 0, 0);
 }
 
 void unwind__finish_access(struct thread *thread)
 {
-	unw_addr_space_t addr_space;
-
 	if (callchain_param.record_mode != CALLCHAIN_DWARF)
 		return;
 
-	addr_space = thread__priv(thread);
-	unw_destroy_addr_space(addr_space);
+	unw_destroy_addr_space(thread->addr_space);
 }
 
 static int get_entries(struct unwind_info *ui, unwind_entry_cb_t cb,

@@ -639,7 +630,9 @@ static int get_entries(struct unwind_info *ui, unwind_entry_cb_t cb,
 	 * unwind itself.
 	 */
 	if (max_stack - 1 > 0) {
-		addr_space = thread__priv(ui->thread);
+		WARN_ONCE(!ui->thread, "WARNING: ui->thread is NULL");
+		addr_space = ui->thread->addr_space;
+
 		if (addr_space == NULL)
 			return -1;

Signature · 1 line

2.4.11

tip-bot for Jiri Olsa

merge bot

2016-04-13 07:17 UTC #2 lore raw

[tip:perf/core] perf tools: Add dedicated unwind addr_space member into thread struct

perf tools: Add dedicated unwind addr_space member into thread struct

Commit-ID:  e583d70c54976f81855c7ca763b036bad399f4e0
Gitweb:     http://git.kernel.org/tip/e583d70c54976f81855c7ca763b036bad399f4e0
Author:     Jiri Olsa <[email protected]>
AuthorDate: Thu, 7 Apr 2016 09:11:12 +0200
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Fri, 8 Apr 2016 09:58:02 -0300

perf tools: Add dedicated unwind addr_space member into thread struct

Milian reported issue with thread::priv, which was double booked by perf trace and DWARF unwind code. So using those together is impossible at the moment.

Moving DWARF unwind private data into separate variable so perf trace can keep using thread::priv.

Reported-and-Tested-by Milian Wolff <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

Cc Andreas Hollmann <[email protected]>

Cc David Ahern <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/util/thread.h           |  6 ++++++
 tools/perf/util/unwind-libunwind.c | 25 +++++++++----------------
 2 files changed, 15 insertions(+), 16 deletions(-)

tools/perf/util/thread.h +6 -0

index a0ac031..e214207 100644
@@ -9,6 +9,9 @@
 #include "symbol.h"
 #include <strlist.h>
 #include <intlist.h>
+#ifdef HAVE_LIBUNWIND_SUPPORT
+#include <libunwind.h>
+#endif
 
 struct thread_stack;

@@ -32,6 +35,9 @@ struct thread {
 
 	void			*priv;
 	struct thread_stack	*ts;
+#ifdef HAVE_LIBUNWIND_SUPPORT
+	unw_addr_space_t	addr_space;
+#endif
 };
 
 struct machine;

tools/perf/util/unwind-libunwind.c +9 ~~-16~~

index ee7e372..63687d3 100644
@@ -32,6 +32,7 @@
 #include "symbol.h"
 #include "util.h"
 #include "debug.h"
+#include "asm/bug.h"
 
 extern int
 UNW_OBJ(dwarf_search_unwind_table) (unw_addr_space_t as,

@@ -580,43 +581,33 @@ static unw_accessors_t accessors = {
 
 int unwind__prepare_access(struct thread *thread)
 {
-	unw_addr_space_t addr_space;
-
 	if (callchain_param.record_mode != CALLCHAIN_DWARF)
 		return 0;
 
-	addr_space = unw_create_addr_space(&accessors, 0);
-	if (!addr_space) {
+	thread->addr_space = unw_create_addr_space(&accessors, 0);
+	if (!thread->addr_space) {
 		pr_err("unwind: Can't create unwind address space.\n");
 		return -ENOMEM;
 	}
 
-	unw_set_caching_policy(addr_space, UNW_CACHE_GLOBAL);
-	thread__set_priv(thread, addr_space);
-
+	unw_set_caching_policy(thread->addr_space, UNW_CACHE_GLOBAL);
 	return 0;
 }
 
 void unwind__flush_access(struct thread *thread)
 {
-	unw_addr_space_t addr_space;
-
 	if (callchain_param.record_mode != CALLCHAIN_DWARF)
 		return;
 
-	addr_space = thread__priv(thread);
-	unw_flush_cache(addr_space, 0, 0);
+	unw_flush_cache(thread->addr_space, 0, 0);
 }
 
 void unwind__finish_access(struct thread *thread)
 {
-	unw_addr_space_t addr_space;
-
 	if (callchain_param.record_mode != CALLCHAIN_DWARF)
 		return;
 
-	addr_space = thread__priv(thread);
-	unw_destroy_addr_space(addr_space);
+	unw_destroy_addr_space(thread->addr_space);
 }
 
 static int get_entries(struct unwind_info *ui, unwind_entry_cb_t cb,

@@ -639,7 +630,9 @@ static int get_entries(struct unwind_info *ui, unwind_entry_cb_t cb,
 	 * unwind itself.
 	 */
 	if (max_stack - 1 > 0) {
-		addr_space = thread__priv(ui->thread);
+		WARN_ONCE(!ui->thread, "WARNING: ui->thread is NULL");
+		addr_space = ui->thread->addr_space;
+
 		if (addr_space == NULL)
 			return -1;

Jiri Olsa

author reviewer

2016-04-07 07:11 UTC #3 lore raw

PATCH 3/3 perf script: Process event update events

In reply to Jiri Olsa

Andreas reported following command produces no output:

  $ cat test.py
  #!/usr/bin/env python

def stat__krava(cpu, thread, time, val, ena, run):
print "event %s cpu %d, thread %d, time %d, val %d, ena %d, run %d" % \
("krava", cpu, thread, time, val, ena, run)

  $ perf stat -a -I 1000 -e cycles,"cpu/config=0x6530160,name=krava/" record | perf script -s test.py

The reason is that perf script does not process event update events and will never get the event name update thus the python callback is never called.

The fix is just to add already existing callback we use in the perf stat report.

Reported-by Andreas Hollmann <[email protected]>

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/builtin-script.c | 1 +
 1 file changed, 1 insertion(+)

tools/perf/builtin-script.c +1 -0

index 3770c3dffe5e..59009aa7e2ca 100644
@@ -1961,6 +1961,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
 			.exit		 = perf_event__process_exit,
 			.fork		 = perf_event__process_fork,
 			.attr		 = process_attr,
+			.event_update   = perf_event__process_event_update,
 			.tracing_data	 = perf_event__process_tracing_data,
 			.build_id	 = perf_event__process_build_id,
 			.id_index	 = perf_event__process_id_index,

Signature · 1 line

2.4.11

tip-bot for Jiri Olsa

merge bot

2016-04-13 07:18 UTC #4 lore raw

[tip:perf/core] perf script: Process event update events

perf script: Process event update events

Commit-ID:  91daee306a51ca7b4d3ca7fdcf7472b0ed2c80c1
Gitweb:     http://git.kernel.org/tip/91daee306a51ca7b4d3ca7fdcf7472b0ed2c80c1
Author:     Jiri Olsa <[email protected]>
AuthorDate: Thu, 7 Apr 2016 09:11:13 +0200
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Fri, 8 Apr 2016 09:58:12 -0300

perf script: Process event update events

Andreas reported following command produces no output:

  # cat test.py
  #!/usr/bin/env python

def stat__krava(cpu, thread, time, val, ena, run):
print "event %s cpu %d, thread %d, time %d, val %d, ena %d, run %d" % \
("krava", cpu, thread, time, val, ena, run)

  # perf stat -a -I 1000 -e cycles,"cpu/config=0x6530160,name=krava/" record | perf script -s test.py
  ^C
  #

The reason is that 'perf script' does not process event update events and will never get the event name update thus the python callback is never called.

The fix is just to add already existing callback we use in 'perf stat report'.

Committer note:

After the patch:

  # perf stat -a -I 1000 -e cycles,"cpu/config=0x6530160,name=krava/" record | perf script -s test.py
  event krava cpu -1, thread -1, time 1000239179, val 1789051, ena 4000690920, run 4000690920
  event krava cpu -1, thread -1, time 2000479061, val 2391338, ena 4000879596, run 4000879596
  event krava cpu -1, thread -1, time 3000740802, val 1939121, ena 4000977209, run 4000977209
  event krava cpu -1, thread -1, time 4001006730, val 2356115, ena 4001000489, run 4001000489
  ^C
  #

Reported-by Andreas Hollmann <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

Tested-by Arnaldo Carvalho de Melo <[email protected]>

Cc David Ahern <[email protected]>

Cc Milian Wolff <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/builtin-script.c | 1 +
 1 file changed, 1 insertion(+)

tools/perf/builtin-script.c +1 -0

index 3770c3d..59009aa 100644
@@ -1961,6 +1961,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
 			.exit		 = perf_event__process_exit,
 			.fork		 = perf_event__process_fork,
 			.attr		 = process_attr,
+			.event_update   = perf_event__process_event_update,
 			.tracing_data	 = perf_event__process_tracing_data,
 			.build_id	 = perf_event__process_build_id,
 			.id_index	 = perf_event__process_id_index,

Arnaldo Carvalho de Melo

maintainer Applied

2016-04-07 13:50 UTC #5 lore raw

In reply to Jiri Olsa

Em Thu, Apr 07, 2016 at 09:11:11AM +0200, Jiri Olsa escreveu:

Quoted 4 lines

To be used in cases for both sides trim.
Link http://lkml.kernel.org/n/[email protected] open here
Signed-off-by Jiri Olsa <[email protected]>

Thanks, applied.

Will try Milian's 'perf trace' callchain patch soon, any updates on that one?

- Arnaldo

Quoted 53 lines

 tools/perf/ui/browsers/hists.c | 3 +--
 tools/perf/ui/stdio/hist.c     | 3 +--
 tools/perf/util/util.h         | 5 +++++
 3 files changed, 7 insertions(+), 4 deletions(-)

tools/perf/ui/browsers/hists.c +1 -2

index 2a83414159a6..e70df2e54d66 100644
@@ -1607,9 +1607,8 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *brows
 
 			ret = fmt->header(fmt, &dummy_hpp, hists_to_evsel(hists));
 			dummy_hpp.buf[ret] = '\0';
-			rtrim(dummy_hpp.buf);
 
-			start = ltrim(dummy_hpp.buf);
+			start = trim(dummy_hpp.buf);
 			ret = strlen(start);
 
 			if (start != dummy_hpp.buf)

tools/perf/ui/stdio/hist.c +1 -2

index 7aff5acf3265..560eb47d56f9 100644
@@ -569,9 +569,8 @@ static int print_hierarchy_header(struct hists *hists, struct perf_hpp *hpp,
 			first_col = false;
 
 			fmt->header(fmt, hpp, hists_to_evsel(hists));
-			rtrim(hpp->buf);
 
-			header_width += fprintf(fp, "%s", ltrim(hpp->buf));
+			header_width += fprintf(fp, "%s", trim(hpp->buf));
 		}
 	}

tools/perf/util/util.h +5 -0

index 8298d607c738..3bf3de86d429 100644
@@ -254,6 +254,11 @@ int hex2u64(const char *ptr, u64 *val);
 char *ltrim(char *s);
 char *rtrim(char *s);
 
+static inline char *trim(char *s)
+{
+	return ltrim(rtrim(s));
+}
+
 void dump_stack(void);
 void sighandler_dump_stack(int sig);

Signature · 1 line

2.4.11

Jiri Olsa

2016-04-07 14:03 UTC #6 lore raw

Quoted 9 lines · Arnaldo Carvalho de Melo

Em Thu, Apr 07, 2016 at 09:11:11AM +0200, Jiri Olsa escreveu:
Quoted 4 lines
To be used in cases for both sides trim.
Link http://lkml.kernel.org/n/[email protected] open here
Signed-off-by Jiri Olsa <[email protected]>
Thanks, applied.
Will try Milian's 'perf trace' callchain patch soon, any updates on that one?

nope, nothing yet

jirka

Milian Wolff

2016-04-07 15:50 UTC #7 lore raw

Quoted 8 lines · CEST Jiri Olsa

Quoted 7 lines · Arnaldo Carvalho de Melo
Em Thu, Apr 07, 2016 at 09:11:11AM +0200, Jiri Olsa escreveu:
Quoted 5 lines
To be used in cases for both sides trim.
Link:
http://lkml.kernel.org/n/[email protected]
Signed-off-by Jiri Olsa <[email protected]>

Quoted 5 lines

Quoted 4 lines
Thanks, applied.
Will try Milian's 'perf trace' callchain patch soon, any updates on that one?
nope, nothing yet

I'll send a working solution based on Jiri's fix later when I'm back home. In the meantime, please have a look at my other patch regarding perf inject merging of trace events for sys_enter/exit.

Thanks

Signature · 4 lines

Milian Wolff | [email protected] | Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts

tip-bot for Jiri Olsa

merge bot

2016-04-13 07:17 UTC #8 lore raw

In reply to Jiri Olsa

perf tools: Introduce trim function

Commit-ID:  7d6a7e782558323364bc0ae59f3523175c10b258
Gitweb:     http://git.kernel.org/tip/7d6a7e782558323364bc0ae59f3523175c10b258
Author:     Jiri Olsa <[email protected]>
AuthorDate: Thu, 7 Apr 2016 09:11:11 +0200
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 7 Apr 2016 10:21:49 -0300

perf tools: Introduce trim function

To be used in cases for both sides trim.

Signed-off-by Jiri Olsa <[email protected]>

Cc Andreas Hollmann <[email protected]>

Cc David Ahern <[email protected]>

Cc Milian Wolff <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/ui/browsers/hists.c | 3 +--
 tools/perf/ui/stdio/hist.c     | 3 +--
 tools/perf/util/util.h         | 5 +++++
 3 files changed, 7 insertions(+), 4 deletions(-)

tools/perf/ui/browsers/hists.c +1 -2

index 2a83414..e70df2e 100644
@@ -1607,9 +1607,8 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *brows
 
 			ret = fmt->header(fmt, &dummy_hpp, hists_to_evsel(hists));
 			dummy_hpp.buf[ret] = '\0';
-			rtrim(dummy_hpp.buf);
 
-			start = ltrim(dummy_hpp.buf);
+			start = trim(dummy_hpp.buf);
 			ret = strlen(start);
 
 			if (start != dummy_hpp.buf)

tools/perf/ui/stdio/hist.c +1 -2

index 7aff5ac..560eb47 100644
@@ -569,9 +569,8 @@ static int print_hierarchy_header(struct hists *hists, struct perf_hpp *hpp,
 			first_col = false;
 
 			fmt->header(fmt, hpp, hists_to_evsel(hists));
-			rtrim(hpp->buf);
 
-			header_width += fprintf(fp, "%s", ltrim(hpp->buf));
+			header_width += fprintf(fp, "%s", trim(hpp->buf));
 		}
 	}

tools/perf/util/util.h +5 -0

index 8298d60..3bf3de8 100644
@@ -254,6 +254,11 @@ int hex2u64(const char *ptr, u64 *val);
 char *ltrim(char *s);
 char *rtrim(char *s);
 
+static inline char *trim(char *s)
+{
+	return ltrim(rtrim(s));
+}
+
 void dump_stack(void);
 void sighandler_dump_stack(int sig);