linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL 0/9] perf/core improvements and fixes
@ 2015-12-11 14:22 Arnaldo Carvalho de Melo
  2015-12-11 14:22 ` [PATCH 1/9] perf top: Do not convert address for perf_top__record_precise_ip() Arnaldo Carvalho de Melo
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-11 14:22 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
	Alexei Starovoitov, Brendan Gregg, David Ahern, David S . Miller,
	Jiri Olsa, Masami Hiramatsu, Namhyung Kim, Peter Zijlstra,
	pi3orama, Wang Nan, Zefan Li, Arnaldo Carvalho de Melo

Hi Ingo,

	Please consider pulling,

- Arnaldo

The following changes since commit e7a7865cc0da306542db0b9205cb0a467f59e33d:

  perf symbols: Fix dso__load_sym to put dso (2015-12-10 16:29:32 -0300)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-core-for-mingo

for you to fetch changes up to 93b0ba3c60da89043ce2b9f601cd2b3da408903b:

  perf tools: Clear struct machine during machine__init() (2015-12-11 09:32:41 -0300)

----------------------------------------------------------------
perf/core improvements and fixes:

User visible:

- Fix 'perf top' annotation in --stdio (Namhyung Kim)

- Support hw breakpoint events (mem:0xAddress) in the default output mode in
  'perf script' (Wang Nan)

Infrastructure:

- Do not hold the hists lock while emitting one specific warning (Namhyung Kim)

- Fetch map names from correct strtab, worked so far because llvm/clang
  uses just one string table (Wang Nan)

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

----------------------------------------------------------------
Namhyung Kim (4):
      perf top: Do not convert address for perf_top__record_precise_ip()
      perf top: Access hists->lock only if needed
      perf top: Fix annotation on --stdio
      perf top: Cleanup condition in perf_top__record_precise_ip()

Wang Nan (5):
      tools lib bpf: Check return value of strdup when reading map names
      tools lib bpf: Fetch map names from correct strtab
      perf data: Add u32_hex data type
      perf script: Add support for PERF_TYPE_BREAKPOINT
      perf tools: Clear struct machine during machine__init()

 tools/lib/bpf/libbpf.c            | 24 +++++++++++++-----
 tools/perf/builtin-script.c       | 14 +++++++++++
 tools/perf/builtin-top.c          | 52 +++++++++++++++++----------------------
 tools/perf/util/data-convert-bt.c |  2 ++
 tools/perf/util/machine.c         |  1 +
 5 files changed, 57 insertions(+), 36 deletions(-)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/9] perf top: Do not convert address for perf_top__record_precise_ip()
  2015-12-11 14:22 [GIT PULL 0/9] perf/core improvements and fixes Arnaldo Carvalho de Melo
@ 2015-12-11 14:22 ` Arnaldo Carvalho de Melo
  2015-12-11 14:22 ` [PATCH 2/9] perf top: Access hists->lock only if needed Arnaldo Carvalho de Melo
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-11 14:22 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, David Ahern, Jiri Olsa,
	Peter Zijlstra, Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung@kernel.org>

We call map->unmap_ip() before the function and call map->map_ip()
inside the function.  This is meaningless and look strange since only
one of the two checks 'map'.  Let's use al->addr directly.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1449802616-16170-1-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-top.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 785aa2dd8f0b..3b0978e5578a 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -189,8 +189,6 @@ static void perf_top__record_precise_ip(struct perf_top *top,
 	if (pthread_mutex_trylock(&notes->lock))
 		return;
 
-	ip = he->ms.map->map_ip(he->ms.map, ip);
-
 	if (ui__has_annotation())
 		err = hist_entry__inc_addr_samples(he, counter, ip);
 
@@ -687,14 +685,8 @@ static int hist_iter__top_callback(struct hist_entry_iter *iter,
 	struct hist_entry *he = iter->he;
 	struct perf_evsel *evsel = iter->evsel;
 
-	if (sort__has_sym && single) {
-		u64 ip = al->addr;
-
-		if (al->map)
-			ip = al->map->unmap_ip(al->map, ip);
-
-		perf_top__record_precise_ip(top, he, evsel->idx, ip);
-	}
+	if (sort__has_sym && single)
+		perf_top__record_precise_ip(top, he, evsel->idx, al->addr);
 
 	hist__account_cycles(iter->sample->branch_stack, al, iter->sample,
 		     !(top->record_opts.branch_stack & PERF_SAMPLE_BRANCH_ANY));
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/9] perf top: Access hists->lock only if needed
  2015-12-11 14:22 [GIT PULL 0/9] perf/core improvements and fixes Arnaldo Carvalho de Melo
  2015-12-11 14:22 ` [PATCH 1/9] perf top: Do not convert address for perf_top__record_precise_ip() Arnaldo Carvalho de Melo
@ 2015-12-11 14:22 ` Arnaldo Carvalho de Melo
  2015-12-11 14:22 ` [PATCH 3/9] perf top: Fix annotation on --stdio Arnaldo Carvalho de Melo
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-11 14:22 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, David Ahern, Jiri Olsa,
	Peter Zijlstra, Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung@kernel.org>

The perf_top__record_precise_ip() releases and regrabs the
he->hists->lock because it can sleep if there's an error.  But it should
be done conditionally as it slows down the fast path.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1449802616-16170-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-top.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 3b0978e5578a..586798acf7db 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -194,21 +194,23 @@ static void perf_top__record_precise_ip(struct perf_top *top,
 
 	pthread_mutex_unlock(&notes->lock);
 
-	/*
-	 * This function is now called with he->hists->lock held.
-	 * Release it before going to sleep.
-	 */
-	pthread_mutex_unlock(&he->hists->lock);
+	if (unlikely(err)) {
+		/*
+		 * This function is now called with he->hists->lock held.
+		 * Release it before going to sleep.
+		 */
+		pthread_mutex_unlock(&he->hists->lock);
+
+		if (err == -ERANGE && !he->ms.map->erange_warned)
+			ui__warn_map_erange(he->ms.map, sym, ip);
+		else if (err == -ENOMEM) {
+			pr_err("Not enough memory for annotating '%s' symbol!\n",
+			       sym->name);
+			sleep(1);
+		}
 
-	if (err == -ERANGE && !he->ms.map->erange_warned)
-		ui__warn_map_erange(he->ms.map, sym, ip);
-	else if (err == -ENOMEM) {
-		pr_err("Not enough memory for annotating '%s' symbol!\n",
-		       sym->name);
-		sleep(1);
+		pthread_mutex_lock(&he->hists->lock);
 	}
-
-	pthread_mutex_lock(&he->hists->lock);
 }
 
 static void perf_top__show_details(struct perf_top *top)
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/9] perf top: Fix annotation on --stdio
  2015-12-11 14:22 [GIT PULL 0/9] perf/core improvements and fixes Arnaldo Carvalho de Melo
  2015-12-11 14:22 ` [PATCH 1/9] perf top: Do not convert address for perf_top__record_precise_ip() Arnaldo Carvalho de Melo
  2015-12-11 14:22 ` [PATCH 2/9] perf top: Access hists->lock only if needed Arnaldo Carvalho de Melo
@ 2015-12-11 14:22 ` Arnaldo Carvalho de Melo
  2015-12-11 14:22 ` [PATCH 4/9] perf top: Cleanup condition in perf_top__record_precise_ip() Arnaldo Carvalho de Melo
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-11 14:22 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, David Ahern, Jiri Olsa,
	Peter Zijlstra, Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung@kernel.org>

The ui__has_annotation() inside perf_top__record_precise_ip() should be
removed since it returns true only for TUI (and when sort key has
symbol).  However the 'perf top --stdio' also supports annotation for a
symbol which was specified by 's' key action.

Actually it already does the necessary checks before calling the
function.  So it's ok to get rid of the check here.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1449802616-16170-3-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-top.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 586798acf7db..f447e5531f8b 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -189,8 +189,7 @@ static void perf_top__record_precise_ip(struct perf_top *top,
 	if (pthread_mutex_trylock(&notes->lock))
 		return;
 
-	if (ui__has_annotation())
-		err = hist_entry__inc_addr_samples(he, counter, ip);
+	err = hist_entry__inc_addr_samples(he, counter, ip);
 
 	pthread_mutex_unlock(&notes->lock);
 
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/9] perf top: Cleanup condition in perf_top__record_precise_ip()
  2015-12-11 14:22 [GIT PULL 0/9] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (2 preceding siblings ...)
  2015-12-11 14:22 ` [PATCH 3/9] perf top: Fix annotation on --stdio Arnaldo Carvalho de Melo
@ 2015-12-11 14:22 ` Arnaldo Carvalho de Melo
  2015-12-11 14:22 ` [PATCH 5/9] tools lib bpf: Check return value of strdup when reading map names Arnaldo Carvalho de Melo
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-11 14:22 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, David Ahern, Jiri Olsa,
	Peter Zijlstra, Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung@kernel.org>

The 'he' cannot be NULL since it's caller hist_iter__top_callback() is
called only if iter->he is not NULL (see hist_entry_iter__add).  So
setting 'sym' before the condition to simplify the code.

Also make it clearer that the top->symbol_filter_entry check is only
meaningful on stdio mode (i.e. when use_browser is 0).

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1449802616-16170-4-git-send-email-namhyung@kernel.org
[ Complete the simplification replacing one more he->ms.sym with sym ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-top.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index f447e5531f8b..92fe963e43c4 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -175,15 +175,14 @@ static void perf_top__record_precise_ip(struct perf_top *top,
 					int counter, u64 ip)
 {
 	struct annotation *notes;
-	struct symbol *sym;
+	struct symbol *sym = he->ms.sym;
 	int err = 0;
 
-	if (he == NULL || he->ms.sym == NULL ||
-	    ((top->sym_filter_entry == NULL ||
-	      top->sym_filter_entry->ms.sym != he->ms.sym) && use_browser != 1))
+	if (sym == NULL || (use_browser == 0 &&
+			    (top->sym_filter_entry == NULL ||
+			     top->sym_filter_entry->ms.sym != sym)))
 		return;
 
-	sym = he->ms.sym;
 	notes = symbol__annotation(sym);
 
 	if (pthread_mutex_trylock(&notes->lock))
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/9] tools lib bpf: Check return value of strdup when reading map names
  2015-12-11 14:22 [GIT PULL 0/9] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (3 preceding siblings ...)
  2015-12-11 14:22 ` [PATCH 4/9] perf top: Cleanup condition in perf_top__record_precise_ip() Arnaldo Carvalho de Melo
@ 2015-12-11 14:22 ` Arnaldo Carvalho de Melo
  2015-12-11 14:22 ` [PATCH 6/9] tools lib bpf: Fetch map names from correct strtab Arnaldo Carvalho de Melo
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-11 14:22 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Wang Nan, Zefan Li, pi3orama,
	Arnaldo Carvalho de Melo

From: Wang Nan <wangnan0@huawei.com>

Commit 561bbccac72d08babafaa33fd7fa9100ec4c9fb6 ("tools lib bpf:
Extract and collect map names from BPF object file") forgets checking
return value of strdup(). This patch fixes it. It also checks names
pointer before strcmp() for safety.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Fixes: 561bbccac72d ("tools lib bpf: Extract and collect map names from BPF object file")
Link: http://lkml.kernel.org/r/1449541544-67621-2-git-send-email-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/bpf/libbpf.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index a298614ad091..16485ab05fc1 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -527,14 +527,14 @@ bpf_object__init_maps(struct bpf_object *obj, void *data,
 	return 0;
 }
 
-static void
+static int
 bpf_object__init_maps_name(struct bpf_object *obj, int maps_shndx)
 {
 	int i;
 	Elf_Data *symbols = obj->efile.symbols;
 
 	if (!symbols || maps_shndx < 0)
-		return;
+		return -EINVAL;
 
 	for (i = 0; i < symbols->d_size / sizeof(GElf_Sym); i++) {
 		GElf_Sym sym;
@@ -556,9 +556,14 @@ bpf_object__init_maps_name(struct bpf_object *obj, int maps_shndx)
 			continue;
 		}
 		obj->maps[map_idx].name = strdup(map_name);
+		if (!obj->maps[map_idx].name) {
+			pr_warning("failed to alloc map name\n");
+			return -ENOMEM;
+		}
 		pr_debug("map %zu is \"%s\"\n", map_idx,
 			 obj->maps[map_idx].name);
 	}
+	return 0;
 }
 
 static int bpf_object__elf_collect(struct bpf_object *obj)
@@ -663,7 +668,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
 	}
 
 	if (maps_shndx >= 0)
-		bpf_object__init_maps_name(obj, maps_shndx);
+		err = bpf_object__init_maps_name(obj, maps_shndx);
 out:
 	return err;
 }
@@ -1372,7 +1377,7 @@ bpf_object__get_map_by_name(struct bpf_object *obj, const char *name)
 	struct bpf_map *pos;
 
 	bpf_map__for_each(pos, obj) {
-		if (strcmp(pos->name, name) == 0)
+		if (pos->name && !strcmp(pos->name, name))
 			return pos;
 	}
 	return NULL;
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 6/9] tools lib bpf: Fetch map names from correct strtab
  2015-12-11 14:22 [GIT PULL 0/9] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (4 preceding siblings ...)
  2015-12-11 14:22 ` [PATCH 5/9] tools lib bpf: Check return value of strdup when reading map names Arnaldo Carvalho de Melo
@ 2015-12-11 14:22 ` Arnaldo Carvalho de Melo
  2015-12-11 14:22 ` [PATCH 7/9] perf data: Add u32_hex data type Arnaldo Carvalho de Melo
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-11 14:22 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Wang Nan, Zefan Li, pi3orama,
	Arnaldo Carvalho de Melo

From: Wang Nan <wangnan0@huawei.com>

Namhyung Kim pointed out a potential problem in original code that it
fetches names of maps from section header string table, which is used
to store section names.

Original code doesn't cause error because of a LLVM behavior that, it
combines shstrtab into strtab. For example:

 $ echo 'int func() {return 0;}' | x86_64-oe-linux-clang -x c -o temp.o -c -
 $ readelf -h ./temp.o
 ELF Header:
   Magic:   7f 45 4c 46 02 01 01 03 00 00 00 00 00 00 00 00
   ...
   Section header string table index: 1
 $ readelf -S ./temp.o
 There are 10 section headers, starting at offset 0x288:

 Section Headers:
   [Nr] Name              Type             Address           Offset
        Size              EntSize          Flags  Link  Info  Align
   [ 0]                   NULL             0000000000000000  00000000
        0000000000000000  0000000000000000           0     0     0
   [ 1] .strtab           STRTAB           0000000000000000  00000230
        0000000000000051  0000000000000000           0     0     1
        ...
 $ readelf -p .strtab ./temp.o

 String dump of section '.strtab':
   [     1]  .text
   [     7]  .comment
   [    10]  .bss
   [    15]  .note.GNU-stack
   [    25]  .rela.eh_frame
   [    34]  func
   [    39]  .strtab
   [    41]  .symtab
   [    49]  .data
   [    4f]  -

 $ readelf -p .shstrtab ./temp.o
 readelf: Warning: Section '.shstrtab' was not dumped because it does not exist!

Where, 'section header string table index' points to '.strtab', and
symbol names are also stored there.

However, in case of gcc:

 $ echo 'int func() {return 0;}' | gcc -x c -o temp.o -c -
 $ readelf -p .shstrtab ./temp.o

 String dump of section '.shstrtab':
   [     1]  .symtab
   [     9]  .strtab
   [    11]  .shstrtab
   [    1b]  .text
   [    21]  .data
   [    27]  .bss
   [    2c]  .comment
   [    35]  .note.GNU-stack
   [    45]  .rela.eh_frame
 $ readelf -p .strtab ./temp.o

 String dump of section '.strtab':
   [     1]  func

They are separated sections.

Although original code doesn't cause error, we'd better use canonical
method for fetching symbol names to avoid potential behavior changing.
This patch learns from readelf's code, fetches string from sh_link
of .symbol section.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Reported-and-Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1449541544-67621-3-git-send-email-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/bpf/libbpf.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 16485ab05fc1..8334a5a9d5d7 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -195,6 +195,7 @@ struct bpf_object {
 		Elf *elf;
 		GElf_Ehdr ehdr;
 		Elf_Data *symbols;
+		size_t strtabidx;
 		struct {
 			GElf_Shdr shdr;
 			Elf_Data *data;
@@ -547,7 +548,7 @@ bpf_object__init_maps_name(struct bpf_object *obj, int maps_shndx)
 			continue;
 
 		map_name = elf_strptr(obj->efile.elf,
-				      obj->efile.ehdr.e_shstrndx,
+				      obj->efile.strtabidx,
 				      sym.st_name);
 		map_idx = sym.st_value / sizeof(struct bpf_map_def);
 		if (map_idx >= obj->nr_maps) {
@@ -630,8 +631,10 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
 				pr_warning("bpf: multiple SYMTAB in %s\n",
 					   obj->path);
 				err = -LIBBPF_ERRNO__FORMAT;
-			} else
+			} else {
 				obj->efile.symbols = data;
+				obj->efile.strtabidx = sh.sh_link;
+			}
 		} else if ((sh.sh_type == SHT_PROGBITS) &&
 			   (sh.sh_flags & SHF_EXECINSTR) &&
 			   (data->d_size > 0)) {
@@ -667,6 +670,10 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
 			goto out;
 	}
 
+	if (!obj->efile.strtabidx || obj->efile.strtabidx >= idx) {
+		pr_warning("Corrupted ELF file: index of strtab invalid\n");
+		return LIBBPF_ERRNO__FORMAT;
+	}
 	if (maps_shndx >= 0)
 		err = bpf_object__init_maps_name(obj, maps_shndx);
 out:
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 7/9] perf data: Add u32_hex data type
  2015-12-11 14:22 [GIT PULL 0/9] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (5 preceding siblings ...)
  2015-12-11 14:22 ` [PATCH 6/9] tools lib bpf: Fetch map names from correct strtab Arnaldo Carvalho de Melo
@ 2015-12-11 14:22 ` Arnaldo Carvalho de Melo
  2015-12-11 14:22 ` [PATCH 8/9] perf script: Add support for PERF_TYPE_BREAKPOINT Arnaldo Carvalho de Melo
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-11 14:22 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Wang Nan, David S. Miller, Alexei Starovoitov,
	Brendan Gregg, Jiri Olsa, Masami Hiramatsu, Namhyung Kim,
	Zefan Li, pi3orama, Arnaldo Carvalho de Melo

From: Wang Nan <wangnan0@huawei.com>

Add hexadecimal u32 to base data type, which is useful for raw output
because raw data is u32 aligned.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1449541544-67621-12-git-send-email-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/data-convert-bt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/data-convert-bt.c b/tools/perf/util/data-convert-bt.c
index 5bfc1198ab46..34cd1e4039d3 100644
--- a/tools/perf/util/data-convert-bt.c
+++ b/tools/perf/util/data-convert-bt.c
@@ -63,6 +63,7 @@ struct ctf_writer {
 			struct bt_ctf_field_type	*s32;
 			struct bt_ctf_field_type	*u32;
 			struct bt_ctf_field_type	*string;
+			struct bt_ctf_field_type	*u32_hex;
 			struct bt_ctf_field_type	*u64_hex;
 		};
 		struct bt_ctf_field_type *array[6];
@@ -982,6 +983,7 @@ do {							\
 	CREATE_INT_TYPE(cw->data.u64, 64, false, false);
 	CREATE_INT_TYPE(cw->data.s32, 32, true,  false);
 	CREATE_INT_TYPE(cw->data.u32, 32, false, false);
+	CREATE_INT_TYPE(cw->data.u32_hex, 32, false, true);
 	CREATE_INT_TYPE(cw->data.u64_hex, 64, false, true);
 
 	cw->data.string  = bt_ctf_field_type_string_create();
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 8/9] perf script: Add support for PERF_TYPE_BREAKPOINT
  2015-12-11 14:22 [GIT PULL 0/9] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (6 preceding siblings ...)
  2015-12-11 14:22 ` [PATCH 7/9] perf data: Add u32_hex data type Arnaldo Carvalho de Melo
@ 2015-12-11 14:22 ` Arnaldo Carvalho de Melo
  2015-12-11 14:22 ` [PATCH 9/9] perf tools: Clear struct machine during machine__init() Arnaldo Carvalho de Melo
  2015-12-14  8:32 ` [GIT PULL 0/9] perf/core improvements and fixes Ingo Molnar
  9 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-11 14:22 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Wang Nan, Adrian Hunter, David S. Miller, Jiri Olsa,
	Namhyung Kim, Zefan Li, pi3orama, Arnaldo Carvalho de Melo

From: Wang Nan <wangnan0@huawei.com>

Useful for getting stack traces for hardware breakpoint events.

Test result:

Before this patch:
 # ~/perf record -g -e mem:0x600980 ./sample
 [ perf record: Woken up 1 times to write data ]
 [ perf record: Captured and wrote 0.011 MB perf.data (12 samples) ]

 # ~/perf script

 # ~/perf script -F comm,tid,pid,time,event,ip,sym,dso
 sample 22520/22520 97457.836294: mem:0x600980:
          5a4ad8 __clear_user (/lib/modules/4.3.0-rc4+/build/vmlinux)
 ...
          3f41ba sys_execve (/lib/modules/4.3.0-rc4+/build/vmlinux)
          979395 return_from_execve (/lib/modules/4.3.0-rc4+/build/vmlinux)
    7f1b59719cf7 [unknown] ([unknown])

 sample 22520/22520 97457.836648: mem:0x600980:
             532 main (/home/w00229757/DataBreakpoints/sample)
           21bd5 __libc_start_main (/tmp/oxygen_root-root/lib64/libc-2.18.so)
 ...

After this patch:
 # ~/perf script
 sample 22520 97457.836294: mem:0x600980:
                   5a4ad8 __clear_user (/lib/modules/4.3.0-rc4+/build/vmlinux)
 ...
                   3f41ba sys_execve (/lib/modules/4.3.0-rc4+/build/vmlinux)
                   979395 return_from_execve (/lib/modules/4.3.0-rc4+/build/vmlinux)
             7f1b59719cf7 [unknown] ([unknown])

 sample 22520 97457.836648: mem:0x600980:
                      532 main (/home/w00229757/DataBreakpoints/sample)
                    21bd5 __libc_start_main (/tmp/oxygen_root-root/lib64/libc-2.18.so)

Committer note:

So, further testing, lets do it for a kernel global variable,
tcp_hashinfo:

  # grep -w tcp_hashinfo /proc/kallsyms
  ffffffff8202fc00 B tcp_hashinfo
  #

Note: allow specifying mem:tcp_hashinfo:

  # perf record -g -e mem:0xffffffff81c65ac0 -a
  ^C[ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.790 MB perf.data ]
  #
  # perf evlist
  mem:0xffffffff8202fc00
  # perf evlist -v
  mem:0xffffffff8202fc00: type: 5, size: 112, { sample_period, sample_freq }: 1, sample_type: IP|TID|TIME|CALLCHAIN|CPU, disabled: 1, inherit: 1, mmap: 1, comm: 1, task: 1, sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1, bp_type: 3, { bp_addr, config1 }: 0xffffffff8202fc00, { bp_len, config2 }: 0x4
  #

Then, after this patch:

  # perf script
  swapper 0 [000] 171036.986988: mem:0xffffffff8202fc00:
    8a0fb5 __inet_lookup_established (/lib/modules/4.3.0+/build/vmlinux)
    8bc09d tcp_v4_early_demux (/lib/modules/4.3.0+/build/vmlinux)
    896def ip_rcv_finish (/lib/modules/4.3.0+/build/vmlinux)
    8976c2 ip_rcv (/lib/modules/4.3.0+/build/vmlinux)
    855eba __netif_receive_skb_core (/lib/modules/4.3.0+/build/vmlinux)
    8565d8 __netif_receive_skb (/lib/modules/4.3.0+/build/vmlinux)
    8572a8 process_backlog (/lib/modules/4.3.0+/build/vmlinux)
    856b11 net_rx_action (/lib/modules/4.3.0+/build/vmlinux)
    2a284b __do_softirq (/lib/modules/4.3.0+/build/vmlinux)
    2a2ba3 irq_exit (/lib/modules/4.3.0+/build/vmlinux)
    96b7a4 do_IRQ (/lib/modules/4.3.0+/build/vmlinux)
    969807 ret_from_intr (/lib/modules/4.3.0+/build/vmlinux)
    804c27 cpuidle_enter (/lib/modules/4.3.0+/build/vmlinux)
    2ded22 call_cpuidle (/lib/modules/4.3.0+/build/vmlinux)
    2defb6 cpu_startup_entry (/lib/modules/4.3.0+/build/vmlinux)
    95d5bc rest_init (/lib/modules/4.3.0+/build/vmlinux)
   1163ffa start_kernel ([kernel.vmlinux].init.text)
   11634d7 x86_64_start_reservations ([kernel.vmlinux].init.text)
   1163623 x86_64_start_kernel ([kernel.vmlinux].init.text)

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1449541544-67621-16-git-send-email-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-script.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 3c3f8d0e3064..d259e9aa3a71 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -130,6 +130,18 @@ static struct {
 
 		.invalid_fields = PERF_OUTPUT_TRACE,
 	},
+
+	[PERF_TYPE_BREAKPOINT] = {
+		.user_set = false,
+
+		.fields = PERF_OUTPUT_COMM | PERF_OUTPUT_TID |
+			      PERF_OUTPUT_CPU | PERF_OUTPUT_TIME |
+			      PERF_OUTPUT_EVNAME | PERF_OUTPUT_IP |
+			      PERF_OUTPUT_SYM | PERF_OUTPUT_DSO |
+			      PERF_OUTPUT_PERIOD,
+
+		.invalid_fields = PERF_OUTPUT_TRACE,
+	},
 };
 
 static bool output_set_by_user(void)
@@ -1129,6 +1141,8 @@ static int parse_output_fields(const struct option *opt __maybe_unused,
 			type = PERF_TYPE_TRACEPOINT;
 		else if (!strcmp(str, "raw"))
 			type = PERF_TYPE_RAW;
+		else if (!strcmp(str, "break"))
+			type = PERF_TYPE_BREAKPOINT;
 		else {
 			fprintf(stderr, "Invalid event type in field string.\n");
 			rc = -EINVAL;
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 9/9] perf tools: Clear struct machine during machine__init()
  2015-12-11 14:22 [GIT PULL 0/9] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (7 preceding siblings ...)
  2015-12-11 14:22 ` [PATCH 8/9] perf script: Add support for PERF_TYPE_BREAKPOINT Arnaldo Carvalho de Melo
@ 2015-12-11 14:22 ` Arnaldo Carvalho de Melo
  2015-12-14  8:32 ` [GIT PULL 0/9] perf/core improvements and fixes Ingo Molnar
  9 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-12-11 14:22 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Wang Nan, Jiri Olsa, Namhyung Kim, Zefan Li,
	pi3orama, Arnaldo Carvalho de Melo

From: Wang Nan <wangnan0@huawei.com>

There are so many test cases use stack allocated 'struct machine'.
Including:
  test__hists_link
  test__hists_filter
  test__mmap_thread_lookup
  test__thread_mg_share
  test__hists_output
  test__hists_cumulate

Also, in non-test code (for example, machine__new_host()) there are
code use 'malloc()' to alloc struct machine.

These are dangerous operations, cause some tests fail or hung in
machines__exit(). For example, in

 machines__exit ->
   machine__destroy_kernel_maps ->
     map_groups__remove ->
       maps__remove ->
         pthread_rwlock_wrlock

a incorrectly initialized lock causes unintended behavior.

This patch memset(0) that structure in machine__init() to ensure all
fields in 'struct machine' are initialized to zero.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1449541544-67621-17-git-send-email-wangnan0@huawei.com
[ Use memset, see 'man bzero' ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/machine.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index f5882b8c8db9..1407d5107480 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -25,6 +25,7 @@ static void dsos__init(struct dsos *dsos)
 
 int machine__init(struct machine *machine, const char *root_dir, pid_t pid)
 {
+	memset(machine, 0, sizeof(*machine));
 	map_groups__init(&machine->kmaps, machine);
 	RB_CLEAR_NODE(&machine->rb_node);
 	dsos__init(&machine->dsos);
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [GIT PULL 0/9] perf/core improvements and fixes
  2015-12-11 14:22 [GIT PULL 0/9] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (8 preceding siblings ...)
  2015-12-11 14:22 ` [PATCH 9/9] perf tools: Clear struct machine during machine__init() Arnaldo Carvalho de Melo
@ 2015-12-14  8:32 ` Ingo Molnar
  9 siblings, 0 replies; 11+ messages in thread
From: Ingo Molnar @ 2015-12-14  8:32 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Adrian Hunter, Alexei Starovoitov, Brendan Gregg,
	David Ahern, David S . Miller, Jiri Olsa, Masami Hiramatsu,
	Namhyung Kim, Peter Zijlstra, pi3orama, Wang Nan, Zefan Li,
	Arnaldo Carvalho de Melo


* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Hi Ingo,
> 
> 	Please consider pulling,
> 
> - Arnaldo
> 
> The following changes since commit e7a7865cc0da306542db0b9205cb0a467f59e33d:
> 
>   perf symbols: Fix dso__load_sym to put dso (2015-12-10 16:29:32 -0300)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-core-for-mingo
> 
> for you to fetch changes up to 93b0ba3c60da89043ce2b9f601cd2b3da408903b:
> 
>   perf tools: Clear struct machine during machine__init() (2015-12-11 09:32:41 -0300)
> 
> ----------------------------------------------------------------
> perf/core improvements and fixes:
> 
> User visible:
> 
> - Fix 'perf top' annotation in --stdio (Namhyung Kim)
> 
> - Support hw breakpoint events (mem:0xAddress) in the default output mode in
>   'perf script' (Wang Nan)
> 
> Infrastructure:
> 
> - Do not hold the hists lock while emitting one specific warning (Namhyung Kim)
> 
> - Fetch map names from correct strtab, worked so far because llvm/clang
>   uses just one string table (Wang Nan)
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> ----------------------------------------------------------------
> Namhyung Kim (4):
>       perf top: Do not convert address for perf_top__record_precise_ip()
>       perf top: Access hists->lock only if needed
>       perf top: Fix annotation on --stdio
>       perf top: Cleanup condition in perf_top__record_precise_ip()
> 
> Wang Nan (5):
>       tools lib bpf: Check return value of strdup when reading map names
>       tools lib bpf: Fetch map names from correct strtab
>       perf data: Add u32_hex data type
>       perf script: Add support for PERF_TYPE_BREAKPOINT
>       perf tools: Clear struct machine during machine__init()
> 
>  tools/lib/bpf/libbpf.c            | 24 +++++++++++++-----
>  tools/perf/builtin-script.c       | 14 +++++++++++
>  tools/perf/builtin-top.c          | 52 +++++++++++++++++----------------------
>  tools/perf/util/data-convert-bt.c |  2 ++
>  tools/perf/util/machine.c         |  1 +
>  5 files changed, 57 insertions(+), 36 deletions(-)

Pulled, thanks a lot Arnaldo!

	Ingo

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-12-14  8:32 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-11 14:22 [GIT PULL 0/9] perf/core improvements and fixes Arnaldo Carvalho de Melo
2015-12-11 14:22 ` [PATCH 1/9] perf top: Do not convert address for perf_top__record_precise_ip() Arnaldo Carvalho de Melo
2015-12-11 14:22 ` [PATCH 2/9] perf top: Access hists->lock only if needed Arnaldo Carvalho de Melo
2015-12-11 14:22 ` [PATCH 3/9] perf top: Fix annotation on --stdio Arnaldo Carvalho de Melo
2015-12-11 14:22 ` [PATCH 4/9] perf top: Cleanup condition in perf_top__record_precise_ip() Arnaldo Carvalho de Melo
2015-12-11 14:22 ` [PATCH 5/9] tools lib bpf: Check return value of strdup when reading map names Arnaldo Carvalho de Melo
2015-12-11 14:22 ` [PATCH 6/9] tools lib bpf: Fetch map names from correct strtab Arnaldo Carvalho de Melo
2015-12-11 14:22 ` [PATCH 7/9] perf data: Add u32_hex data type Arnaldo Carvalho de Melo
2015-12-11 14:22 ` [PATCH 8/9] perf script: Add support for PERF_TYPE_BREAKPOINT Arnaldo Carvalho de Melo
2015-12-11 14:22 ` [PATCH 9/9] perf tools: Clear struct machine during machine__init() Arnaldo Carvalho de Melo
2015-12-14  8:32 ` [GIT PULL 0/9] perf/core improvements and fixes Ingo Molnar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).