All of lore.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL 00/17] perf/core improvements and fixes
@ 2015-05-14 22:37 Arnaldo Carvalho de Melo
  2015-05-14 22:37 ` [PATCH 01/17] perf kmem: Fix compiler warning about may be accessing uninitialized variable Arnaldo Carvalho de Melo
                   ` (17 more replies)
  0 siblings, 18 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-14 22:37 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
	Borislav Petkov, David Ahern, Don Zickus, Frederic Weisbecker,
	He Kuang, Jiri Olsa, Masami Hiramatsu, Namhyung Kim,
	Naveen N . Rao, Peter Zijlstra, Stephane Eranian, Steven Rostedt,
	Taeung Song, Vinson Lee, Wang Nan, Arnaldo Carvalho de Melo

Hi Ingo,

	Please consider pulling,

- Arnaldo

The following changes since commit a41f3c8cd4e28dcbebd8ec27a9602c86cfa5f009:

  perf/x86/intel/uncore: Add Broadwell-U uncore IMC PMU support (2015-05-11 11:57:47 +0200)

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 70923bd26c732ac2d9e248d80214df6dfd75f78c:

  perf tools: Make flex/bison calls honour V=1 (2015-05-14 19:27:47 -0300)

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

User visible:

- Add --range option to show a variable's location range in 'perf probe',
  helping in collecting variables in probes when there is a mismatch
  between assembly and source code (He Kuang)

- Show better error message when failed to find variable in 'perf probe' (He Kuang)

- Fix 'perf report --thread' handling and document it better (Namhyung Kim)

Infrastructure:

- Fix to get negative exit codes in 'perf test' test routines (He Kuang)

- Make flex/bison calls honour V=1 (Jiri Olsa)

- Ignore tail calls to probed functions in 'perf probe' (Naveen N. Rao)

- Fix refcount expectations in map_group share 'perf test' (Arnaldo Carvalho de Melo)

Build Fixes:

- Fix 'perf kmem' build due to compiler thinking uninitialized var is
  being accessed (Arnaldo Carvalho de Melo)

- Provide le16toh if not defined, to fix the libtraceevent build on
  older distros (Arnaldo Carvalho de Melo)

- Fix 'perf trace' build on older distros by providing some CLOEXEC, NONBLOCK
  defines (Arnaldo Carvalho de Melo)

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

----------------------------------------------------------------
Arnaldo Carvalho de Melo (6):
      perf kmem: Fix compiler warning about may be accessing uninitialized variable
      perf tests: Show refcounting broken expectations in thread-mg-share test
      perf machine: No need to keep a refcnt for last_match
      perf tests: Fix map_groups refcount test
      tools lib traceevent: Provide le16toh define for older systems
      perf trace: Fix the build on older distros

He Kuang (5):
      perf trace: Removed duplicated NULL test
      perf probe: Remove length limitation for showing available variables
      perf probe: Add --range option to show a variable's location range
      perf probe: Show better error message when failed to find variable
      perf tests: Fix to get negative exit codes

Jiri Olsa (1):
      perf tools: Make flex/bison calls honour V=1

Namhyung Kim (4):
      perf tools: Document relation of per-thread event count feature
      perf report: Force tty output if -T/--thread option is given
      perf report: Do not restrict -T option by other options
      perf report: Fix some option handling on --stdio

Naveen N. Rao (1):
      perf probe: Ignore tail calls to probed functions

 tools/lib/traceevent/plugin_cfg80211.c   |  13 ++
 tools/perf/Documentation/perf-record.txt |   3 +-
 tools/perf/Documentation/perf-report.txt |   3 +-
 tools/perf/builtin-kmem.c                |   2 +-
 tools/perf/builtin-probe.c               |   2 +
 tools/perf/builtin-report.c              |  17 ++-
 tools/perf/builtin-trace.c               |  32 ++++-
 tools/perf/tests/builtin-test.c          |   2 +-
 tools/perf/tests/tests.h                 |   9 ++
 tools/perf/tests/thread-mg-share.c       |  29 +++--
 tools/perf/util/Build                    |   8 +-
 tools/perf/util/cache.h                  |   1 -
 tools/perf/util/dwarf-aux.c              | 204 +++++++++++++++++++++++++++----
 tools/perf/util/dwarf-aux.h              |  10 +-
 tools/perf/util/environment.c            |   1 -
 tools/perf/util/machine.c                |   8 +-
 tools/perf/util/pager.c                  |   5 -
 tools/perf/util/probe-event.h            |   1 +
 tools/perf/util/probe-finder.c           |  90 ++++++++++----
 19 files changed, 346 insertions(+), 94 deletions(-)

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

* [PATCH 01/17] perf kmem: Fix compiler warning about may be accessing uninitialized variable
  2015-05-14 22:37 [GIT PULL 00/17] perf/core improvements and fixes Arnaldo Carvalho de Melo
@ 2015-05-14 22:37 ` Arnaldo Carvalho de Melo
  2015-05-14 22:37 ` [PATCH 02/17] perf tools: Document relation of per-thread event count feature Arnaldo Carvalho de Melo
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-14 22:37 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
	Borislav Petkov, David Ahern, Don Zickus, Frederic Weisbecker,
	Jiri Olsa, Namhyung Kim, Stephane Eranian

From: Arnaldo Carvalho de Melo <acme@redhat.com>

The last argument to strtok_r doesn't need to be initialized, its just a
placeholder to make this routine reentrant, but gcc doesn't know about
that and complains, breaking the build, fix it by setting it to NULL.

Fixes: 0e11115644b3 ("perf kmem: Print gfp flags in human readable string")
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-iyyvkbnkrd9g19f6ta9zfkem@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-kmem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index e0173c7f17b1..254614b10c4a 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -713,7 +713,7 @@ static int parse_gfp_flags(struct perf_evsel *evsel, struct perf_sample *sample,
 		.size = sample->raw_size,
 	};
 	struct trace_seq seq;
-	char *str, *pos;
+	char *str, *pos = NULL;
 
 	if (nr_gfps) {
 		struct gfp_flag key = {
-- 
2.1.0


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

* [PATCH 02/17] perf tools: Document relation of per-thread event count feature
  2015-05-14 22:37 [GIT PULL 00/17] perf/core improvements and fixes Arnaldo Carvalho de Melo
  2015-05-14 22:37 ` [PATCH 01/17] perf kmem: Fix compiler warning about may be accessing uninitialized variable Arnaldo Carvalho de Melo
@ 2015-05-14 22:37 ` Arnaldo Carvalho de Melo
  2015-05-14 22:37 ` [PATCH 03/17] perf report: Force tty output if -T/--thread option is given Arnaldo Carvalho de Melo
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-14 22:37 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 record -s' and 'perf report -T' should be used together to see
per-thread event counts.  Document the relation of these commands.

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/1431184784-30525-1-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-record.txt | 3 ++-
 tools/perf/Documentation/perf-report.txt | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 57dd57bcef95..280533ebf9df 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -147,7 +147,8 @@ OPTIONS
 
 -s::
 --stat::
-	Per thread counts.
+	Record per-thread event counts.  Use it with 'perf report -T' to see
+	the values.
 
 -d::
 --data::
diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index 27190ed06f9c..c33b69f3374f 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -34,7 +34,8 @@ OPTIONS
 
 -T::
 --threads::
-	Show per-thread event counters
+	Show per-thread event counters.  The input data file should be recorded
+	with -s option.
 -c::
 --comms=::
 	Only consider symbols in these comms. CSV that understands
-- 
2.1.0


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

* [PATCH 03/17] perf report: Force tty output if -T/--thread option is given
  2015-05-14 22:37 [GIT PULL 00/17] perf/core improvements and fixes Arnaldo Carvalho de Melo
  2015-05-14 22:37 ` [PATCH 01/17] perf kmem: Fix compiler warning about may be accessing uninitialized variable Arnaldo Carvalho de Melo
  2015-05-14 22:37 ` [PATCH 02/17] perf tools: Document relation of per-thread event count feature Arnaldo Carvalho de Melo
@ 2015-05-14 22:37 ` Arnaldo Carvalho de Melo
  2015-05-14 22:37 ` [PATCH 04/17] perf trace: Removed duplicated NULL test Arnaldo Carvalho de Melo
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-14 22:37 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 -T/--thread option is supported only on --stdio mode (at least for
now).  So enforce the tty output if the option was requested.

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/1431184784-30525-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-report.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 62b49ca0fc7b..4d642db642c5 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -815,8 +815,8 @@ repeat:
 		goto error;
 	}
 
-	/* Force tty output for header output. */
-	if (report.header || report.header_only)
+	/* Force tty output for header output and per-thread stat. */
+	if (report.header || report.header_only || report.show_threads)
 		use_browser = 0;
 
 	if (strcmp(input_name, "-") != 0)
-- 
2.1.0


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

* [PATCH 04/17] perf trace: Removed duplicated NULL test
  2015-05-14 22:37 [GIT PULL 00/17] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (2 preceding siblings ...)
  2015-05-14 22:37 ` [PATCH 03/17] perf report: Force tty output if -T/--thread option is given Arnaldo Carvalho de Melo
@ 2015-05-14 22:37 ` Arnaldo Carvalho de Melo
  2015-05-14 22:37 ` [PATCH 05/17] perf probe: Remove length limitation for showing available variables Arnaldo Carvalho de Melo
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-14 22:37 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, He Kuang, Jiri Olsa, Peter Zijlstra, Wang Nan,
	Arnaldo Carvalho de Melo

From: He Kuang <hekuang@huawei.com>

No need to test trace.evlist against NULL twice.

Signed-off-by: He Kuang <hekuang@huawei.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1431347316-30401-2-git-send-email-hekuang@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-trace.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index cbfdb9523868..96a2ebafc755 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -2721,11 +2721,10 @@ int cmd_trace(int argc, const char **argv, const char *prefix __maybe_unused)
 	signal(SIGFPE, sighandler_dump_stack);
 
 	trace.evlist = perf_evlist__new();
-	if (trace.evlist == NULL)
-		return -ENOMEM;
 
 	if (trace.evlist == NULL) {
 		pr_err("Not enough memory to run!\n");
+		err = -ENOMEM;
 		goto out;
 	}
 
-- 
2.1.0


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

* [PATCH 05/17] perf probe: Remove length limitation for showing available variables
  2015-05-14 22:37 [GIT PULL 00/17] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (3 preceding siblings ...)
  2015-05-14 22:37 ` [PATCH 04/17] perf trace: Removed duplicated NULL test Arnaldo Carvalho de Melo
@ 2015-05-14 22:37 ` Arnaldo Carvalho de Melo
  2015-05-14 22:37 ` [PATCH 06/17] perf probe: Add --range option to show a variable's location range Arnaldo Carvalho de Melo
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-14 22:37 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, He Kuang, Jiri Olsa, Peter Zijlstra, Wang Nan,
	Arnaldo Carvalho de Melo

From: He Kuang <hekuang@huawei.com>

Use struct strbuf instead of bare char[] to remove the length limitation
of variables in variable_list, so they will not disappear due to
overlength, and make preparation for adding more description for
variables.

Signed-off-by: He Kuang <hekuang@huawei.com>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1431336304-16863-1-git-send-email-hekuang@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/dwarf-aux.c    | 50 +++++++++++++++++++-----------------------
 tools/perf/util/dwarf-aux.h    |  4 ++--
 tools/perf/util/probe-finder.c | 17 ++++++++------
 3 files changed, 34 insertions(+), 37 deletions(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 16d46e26edac..737c9dbe5643 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -848,19 +848,17 @@ Dwarf_Die *die_find_member(Dwarf_Die *st_die, const char *name,
 /**
  * die_get_typename - Get the name of given variable DIE
  * @vr_die: a variable DIE
- * @buf: a buffer for result type name
- * @len: a max-length of @buf
+ * @buf: a strbuf for result type name
  *
- * Get the name of @vr_die and stores it to @buf. Return the actual length
- * of type name if succeeded. Return -E2BIG if @len is not enough long, and
- * Return -ENOENT if failed to find type name.
+ * Get the name of @vr_die and stores it to @buf. Return 0 if succeeded.
+ * and Return -ENOENT if failed to find type name.
  * Note that the result will stores typedef name if possible, and stores
  * "*(function_type)" if the type is a function pointer.
  */
-int die_get_typename(Dwarf_Die *vr_die, char *buf, int len)
+int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf)
 {
 	Dwarf_Die type;
-	int tag, ret, ret2;
+	int tag, ret;
 	const char *tmp = "";
 
 	if (__die_get_real_type(vr_die, &type) == NULL)
@@ -871,8 +869,8 @@ int die_get_typename(Dwarf_Die *vr_die, char *buf, int len)
 		tmp = "*";
 	else if (tag == DW_TAG_subroutine_type) {
 		/* Function pointer */
-		ret = snprintf(buf, len, "(function_type)");
-		return (ret >= len) ? -E2BIG : ret;
+		strbuf_addf(buf, "(function_type)");
+		return 0;
 	} else {
 		if (!dwarf_diename(&type))
 			return -ENOENT;
@@ -883,39 +881,35 @@ int die_get_typename(Dwarf_Die *vr_die, char *buf, int len)
 		else if (tag == DW_TAG_enumeration_type)
 			tmp = "enum ";
 		/* Write a base name */
-		ret = snprintf(buf, len, "%s%s", tmp, dwarf_diename(&type));
-		return (ret >= len) ? -E2BIG : ret;
-	}
-	ret = die_get_typename(&type, buf, len);
-	if (ret > 0) {
-		ret2 = snprintf(buf + ret, len - ret, "%s", tmp);
-		ret = (ret2 >= len - ret) ? -E2BIG : ret2 + ret;
+		strbuf_addf(buf, "%s%s", tmp, dwarf_diename(&type));
+		return 0;
 	}
+	ret = die_get_typename(&type, buf);
+	if (ret == 0)
+		strbuf_addf(buf, "%s", tmp);
+
 	return ret;
 }
 
 /**
  * die_get_varname - Get the name and type of given variable DIE
  * @vr_die: a variable DIE
- * @buf: a buffer for type and variable name
- * @len: the max-length of @buf
+ * @buf: a strbuf for type and variable name
  *
  * Get the name and type of @vr_die and stores it in @buf as "type\tname".
  */
-int die_get_varname(Dwarf_Die *vr_die, char *buf, int len)
+int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf)
 {
-	int ret, ret2;
+	int ret;
 
-	ret = die_get_typename(vr_die, buf, len);
+	ret = die_get_typename(vr_die, buf);
 	if (ret < 0) {
 		pr_debug("Failed to get type, make it unknown.\n");
-		ret = snprintf(buf, len, "(unknown_type)");
-	}
-	if (ret > 0) {
-		ret2 = snprintf(buf + ret, len - ret, "\t%s",
-				dwarf_diename(vr_die));
-		ret = (ret2 >= len - ret) ? -E2BIG : ret2 + ret;
+		strbuf_addf(buf, "(unknown_type)");
 	}
-	return ret;
+
+	strbuf_addf(buf, "\t%s", dwarf_diename(vr_die));
+
+	return 0;
 }
 
diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
index 50a3cdc55fd7..60676fda4824 100644
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -117,8 +117,8 @@ extern Dwarf_Die *die_find_member(Dwarf_Die *st_die, const char *name,
 				  Dwarf_Die *die_mem);
 
 /* Get the name of given variable DIE */
-extern int die_get_typename(Dwarf_Die *vr_die, char *buf, int len);
+extern int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf);
 
 /* Get the name and type of given variable DIE, stored as "type\tname" */
-extern int die_get_varname(Dwarf_Die *vr_die, char *buf, int len);
+extern int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf);
 #endif
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 8b9e274f940c..d7c2e90ab4ce 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1255,14 +1255,11 @@ int debuginfo__find_trace_events(struct debuginfo *dbg,
 	return (ret < 0) ? ret : tf.ntevs;
 }
 
-#define MAX_VAR_LEN 64
-
 /* Collect available variables in this scope */
 static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
 {
 	struct available_var_finder *af = data;
 	struct variable_list *vl;
-	char buf[MAX_VAR_LEN];
 	int tag, ret;
 
 	vl = &af->vls[af->nvls - 1];
@@ -1274,10 +1271,16 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
 						af->pf.fb_ops, &af->pf.sp_die,
 						NULL);
 		if (ret == 0) {
-			ret = die_get_varname(die_mem, buf, MAX_VAR_LEN);
-			pr_debug2("Add new var: %s\n", buf);
-			if (ret > 0)
-				strlist__add(vl->vars, buf);
+			struct strbuf buf;
+
+			strbuf_init(&buf, 64);
+			ret = die_get_varname(die_mem, &buf);
+			pr_debug2("Add new var: %s\n", buf.buf);
+			if (ret == 0) {
+				strlist__add(vl->vars,
+					strbuf_detach(&buf, NULL));
+			}
+			strbuf_release(&buf);
 		}
 	}
 
-- 
2.1.0


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

* [PATCH 06/17] perf probe: Add --range option to show a variable's location range
  2015-05-14 22:37 [GIT PULL 00/17] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (4 preceding siblings ...)
  2015-05-14 22:37 ` [PATCH 05/17] perf probe: Remove length limitation for showing available variables Arnaldo Carvalho de Melo
@ 2015-05-14 22:37 ` Arnaldo Carvalho de Melo
  2015-06-25 12:30   ` Alexei Starovoitov
  2015-05-14 22:37 ` [PATCH 07/17] perf probe: Show better error message when failed to find variable Arnaldo Carvalho de Melo
                   ` (11 subsequent siblings)
  17 siblings, 1 reply; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-14 22:37 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, He Kuang, Jiri Olsa, Peter Zijlstra, Wang Nan,
	Arnaldo Carvalho de Melo

From: He Kuang <hekuang@huawei.com>

It is not easy for users to get the accurate byte offset or the line
number where a local variable can be probed.

With '--range' option, local variables in the scope of the probe point
are showed with a byte offset range, and can be added according to this
range information.

For example, there are some variables in the function
generic_perform_write():

  <generic_perform_write@mm/filemap.c:0>
  0  ssize_t generic_perform_write(struct file *file,
  1                                 struct iov_iter *i, loff_t pos)
  2  {
  3          struct address_space *mapping = file->f_mapping;
  4          const struct address_space_operations *a_ops = mapping->a_ops;
  ...
  42                 status = a_ops->write_begin(file, mapping, pos, bytes, flags,
                                               &page, &fsdata);
  44                 if (unlikely(status < 0))

But we fail when we try to probe the variable 'a_ops' at line 42 or 44.

  $ perf probe --add 'generic_perform_write:42 a_ops'
  Failed to find the location of a_ops at this address.
    Perhaps, it has been optimized out.

This is because the source code do not match the assembly, so a variable
may not be available in the source code line where it appears.

After this patch, we can lookup the accurate byte offset range of a
variable, 'INV' indicates that this variable is not valid at the given
point, but available in the scope:

  $ perf probe --vars 'generic_perform_write:42' --range
  Available variables at generic_perform_write:42
    @<generic_perform_write+141>
       [INV] ssize_t written @<generic_perform_write+[324-331]>
       [INV] struct address_space_operations*        a_ops   @<generic_perform_write+[55-61,170-176,223-246]>
       [VAL] (unknown_type)  fsdata  @<generic_perform_write+[70-307,346-411]>
       [VAL] loff_t  pos     @<generic_perform_write+[0-286,286-336,346-411]>
       [VAL] long int        status  @<generic_perform_write+[83-342,346-411]>
       [VAL] long unsigned int       bytes   @<generic_perform_write+[122-311,320-338,346-403,403-411]>
       [VAL] struct address_space*   mapping @<generic_perform_write+[35-344,346-411]>
       [VAL] struct iov_iter*        i       @<generic_perform_write+[0-340,346-411]>
       [VAL] struct page*    page    @<generic_perform_write+[70-307,346-411]>

Then it is more clear for us to add a probe with this variable:

  $ perf probe --add 'generic_perform_write+170 a_ops'
  Added new event:
    probe:generic_perform_write (on generic_perform_write+170 with a_ops)

Signed-off-by: He Kuang <hekuang@huawei.com>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1431336304-16863-2-git-send-email-hekuang@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-probe.c     |   2 +
 tools/perf/util/dwarf-aux.c    | 121 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/dwarf-aux.h    |   2 +
 tools/perf/util/probe-event.h  |   1 +
 tools/perf/util/probe-finder.c |  57 ++++++++++++++-----
 5 files changed, 170 insertions(+), 13 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 7fa2c7a1086a..1272559fa22d 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -372,6 +372,8 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 		     "Show accessible variables on PROBEDEF", opt_show_vars),
 	OPT_BOOLEAN('\0', "externs", &probe_conf.show_ext_vars,
 		    "Show external variables too (with --vars only)"),
+	OPT_BOOLEAN('\0', "range", &probe_conf.show_location_range,
+		"Show variables location range in scope (with --vars only)"),
 	OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name,
 		   "file", "vmlinux pathname"),
 	OPT_STRING('s', "source", &symbol_conf.source_prefix,
diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 737c9dbe5643..afa09719336b 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -913,3 +913,124 @@ int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf)
 	return 0;
 }
 
+/**
+ * die_get_var_innermost_scope - Get innermost scope range of given variable DIE
+ * @sp_die: a subprogram DIE
+ * @vr_die: a variable DIE
+ * @buf: a strbuf for variable byte offset range
+ *
+ * Get the innermost scope range of @vr_die and stores it in @buf as
+ * "@<function_name+[NN-NN,NN-NN]>".
+ */
+static int die_get_var_innermost_scope(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
+				struct strbuf *buf)
+{
+	Dwarf_Die *scopes;
+	int count;
+	size_t offset = 0;
+	Dwarf_Addr base;
+	Dwarf_Addr start, end;
+	Dwarf_Addr entry;
+	int ret;
+	bool first = true;
+	const char *name;
+
+	ret = dwarf_entrypc(sp_die, &entry);
+	if (ret)
+		return ret;
+
+	name = dwarf_diename(sp_die);
+	if (!name)
+		return -ENOENT;
+
+	count = dwarf_getscopes_die(vr_die, &scopes);
+
+	/* (*SCOPES)[1] is the DIE for the scope containing that scope */
+	if (count <= 1) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	while ((offset = dwarf_ranges(&scopes[1], offset, &base,
+				&start, &end)) > 0) {
+		start -= entry;
+		end -= entry;
+
+		if (first) {
+			strbuf_addf(buf, "@<%s+[%lu-%lu",
+				name, start, end);
+			first = false;
+		} else {
+			strbuf_addf(buf, ",%lu-%lu",
+				start, end);
+		}
+	}
+
+	if (!first)
+		strbuf_addf(buf, "]>");
+
+out:
+	free(scopes);
+	return ret;
+}
+
+/**
+ * die_get_var_range - Get byte offset range of given variable DIE
+ * @sp_die: a subprogram DIE
+ * @vr_die: a variable DIE
+ * @buf: a strbuf for type and variable name and byte offset range
+ *
+ * Get the byte offset range of @vr_die and stores it in @buf as
+ * "@<function_name+[NN-NN,NN-NN]>".
+ */
+int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die, struct strbuf *buf)
+{
+	int ret = 0;
+	Dwarf_Addr base;
+	Dwarf_Addr start, end;
+	Dwarf_Addr entry;
+	Dwarf_Op *op;
+	size_t nops;
+	size_t offset = 0;
+	Dwarf_Attribute attr;
+	bool first = true;
+	const char *name;
+
+	ret = dwarf_entrypc(sp_die, &entry);
+	if (ret)
+		return ret;
+
+	name = dwarf_diename(sp_die);
+	if (!name)
+		return -ENOENT;
+
+	if (dwarf_attr(vr_die, DW_AT_location, &attr) == NULL)
+		return -EINVAL;
+
+	while ((offset = dwarf_getlocations(
+				&attr, offset, &base,
+				&start, &end, &op, &nops)) > 0) {
+		if (start == 0) {
+			/* Single Location Descriptions */
+			ret = die_get_var_innermost_scope(sp_die, vr_die, buf);
+			return ret;
+		}
+
+		/* Location Lists */
+		start -= entry;
+		end -= entry;
+		if (first) {
+			strbuf_addf(buf, "@<%s+[%lu-%lu",
+				name, start, end);
+			first = false;
+		} else {
+			strbuf_addf(buf, ",%lu-%lu",
+				start, end);
+		}
+	}
+
+	if (!first)
+		strbuf_addf(buf, "]>");
+
+	return ret;
+}
diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
index 60676fda4824..c154c0b80880 100644
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -121,4 +121,6 @@ extern int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf);
 
 /* Get the name and type of given variable DIE, stored as "type\tname" */
 extern int die_get_varname(Dwarf_Die *vr_die, struct strbuf *buf);
+extern int die_get_var_range(Dwarf_Die *sp_die, Dwarf_Die *vr_die,
+			struct strbuf *buf);
 #endif
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 1e2faa3559d2..537eb329c2cf 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -9,6 +9,7 @@
 /* Probe related configurations */
 struct probe_conf {
 	bool	show_ext_vars;
+	bool	show_location_range;
 	bool	force_add;
 	bool	no_inlines;
 	int	max_probes;
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index d7c2e90ab4ce..5804086425aa 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -177,7 +177,7 @@ static int convert_variable_location(Dwarf_Die *vr_die, Dwarf_Addr addr,
 	Dwarf_Word offs = 0;
 	bool ref = false;
 	const char *regs;
-	int ret;
+	int ret, ret2 = 0;
 
 	if (dwarf_attr(vr_die, DW_AT_external, &attr) != NULL)
 		goto static_var;
@@ -187,9 +187,19 @@ static int convert_variable_location(Dwarf_Die *vr_die, Dwarf_Addr addr,
 		return -EINVAL;	/* Broken DIE ? */
 	if (dwarf_getlocation_addr(&attr, addr, &op, &nops, 1) <= 0) {
 		ret = dwarf_entrypc(sp_die, &tmp);
-		if (ret || addr != tmp ||
-		    dwarf_tag(vr_die) != DW_TAG_formal_parameter ||
-		    dwarf_highpc(sp_die, &tmp))
+		if (ret)
+			return -ENOENT;
+
+		if (probe_conf.show_location_range &&
+			(dwarf_tag(vr_die) == DW_TAG_variable)) {
+			ret2 = -ERANGE;
+		} else if (addr != tmp ||
+			dwarf_tag(vr_die) != DW_TAG_formal_parameter) {
+			return -ENOENT;
+		}
+
+		ret = dwarf_highpc(sp_die, &tmp);
+		if (ret)
 			return -ENOENT;
 		/*
 		 * This is fuzzed by fentry mcount. We try to find the
@@ -210,7 +220,7 @@ found:
 	if (op->atom == DW_OP_addr) {
 static_var:
 		if (!tvar)
-			return 0;
+			return ret2;
 		/* Static variables on memory (not stack), make @varname */
 		ret = strlen(dwarf_diename(vr_die));
 		tvar->value = zalloc(ret + 2);
@@ -220,7 +230,7 @@ static_var:
 		tvar->ref = alloc_trace_arg_ref((long)offs);
 		if (tvar->ref == NULL)
 			return -ENOMEM;
-		return 0;
+		return ret2;
 	}
 
 	/* If this is based on frame buffer, set the offset */
@@ -250,14 +260,14 @@ static_var:
 	}
 
 	if (!tvar)
-		return 0;
+		return ret2;
 
 	regs = get_arch_regstr(regn);
 	if (!regs) {
 		/* This should be a bug in DWARF or this tool */
 		pr_warning("Mapping for the register number %u "
 			   "missing on this architecture.\n", regn);
-		return -ERANGE;
+		return -ENOTSUP;
 	}
 
 	tvar->value = strdup(regs);
@@ -269,7 +279,7 @@ static_var:
 		if (tvar->ref == NULL)
 			return -ENOMEM;
 	}
-	return 0;
+	return ret2;
 }
 
 #define BYTES_TO_BITS(nb)	((nb) * BITS_PER_LONG / sizeof(long))
@@ -1270,13 +1280,34 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
 		ret = convert_variable_location(die_mem, af->pf.addr,
 						af->pf.fb_ops, &af->pf.sp_die,
 						NULL);
-		if (ret == 0) {
+		if (ret == 0 || ret == -ERANGE) {
+			int ret2;
+			bool externs = !af->child;
 			struct strbuf buf;
 
 			strbuf_init(&buf, 64);
-			ret = die_get_varname(die_mem, &buf);
-			pr_debug2("Add new var: %s\n", buf.buf);
-			if (ret == 0) {
+
+			if (probe_conf.show_location_range) {
+				if (!externs) {
+					if (ret)
+						strbuf_addf(&buf, "[INV]\t");
+					else
+						strbuf_addf(&buf, "[VAL]\t");
+				} else
+					strbuf_addf(&buf, "[EXT]\t");
+			}
+
+			ret2 = die_get_varname(die_mem, &buf);
+
+			if (!ret2 && probe_conf.show_location_range &&
+				!externs) {
+				strbuf_addf(&buf, "\t");
+				ret2 = die_get_var_range(&af->pf.sp_die,
+							die_mem, &buf);
+			}
+
+			pr_debug("Add new var: %s\n", buf.buf);
+			if (ret2 == 0) {
 				strlist__add(vl->vars,
 					strbuf_detach(&buf, NULL));
 			}
-- 
2.1.0


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

* [PATCH 07/17] perf probe: Show better error message when failed to find variable
  2015-05-14 22:37 [GIT PULL 00/17] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (5 preceding siblings ...)
  2015-05-14 22:37 ` [PATCH 06/17] perf probe: Add --range option to show a variable's location range Arnaldo Carvalho de Melo
@ 2015-05-14 22:37 ` Arnaldo Carvalho de Melo
  2015-05-14 22:38 ` [PATCH 08/17] perf tests: Fix to get negative exit codes Arnaldo Carvalho de Melo
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-14 22:37 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, He Kuang, Jiri Olsa, Peter Zijlstra, Wang Nan,
	Arnaldo Carvalho de Melo

From: He Kuang <hekuang@huawei.com>

Indicate to check variable location range in error message when we got
failed to find the variable.

Before this patch:

  $ perf probe --add 'generic_perform_write+118 bytes'
  Failed to find the location of bytes at this address.
   Perhaps, it has been optimized out.
    Error: Failed to add events.

After this patch:

  $ perf probe --add 'generic_perform_write+118 bytes'
  Failed to find the location of the 'bytes' variable at this address.
   Perhaps it has been optimized out.
   Use -V with the --range option to show 'bytes' location range.
    Error: Failed to add events.

Signed-off-by: He Kuang <hekuang@huawei.com>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1431336304-16863-3-git-send-email-hekuang@huawei.com
[ Improve the error message based on lkml thread ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-finder.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 5804086425aa..590a24a7f555 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -527,10 +527,12 @@ static int convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf)
 
 	ret = convert_variable_location(vr_die, pf->addr, pf->fb_ops,
 					&pf->sp_die, pf->tvar);
-	if (ret == -ENOENT || ret == -EINVAL)
-		pr_err("Failed to find the location of %s at this address.\n"
-		       " Perhaps, it has been optimized out.\n", pf->pvar->var);
-	else if (ret == -ENOTSUP)
+	if (ret == -ENOENT || ret == -EINVAL) {
+		pr_err("Failed to find the location of the '%s' variable at this address.\n"
+		       " Perhaps it has been optimized out.\n"
+		       " Use -V with the --range option to show '%s' location range.\n",
+		       pf->pvar->var, pf->pvar->var);
+	} else if (ret == -ENOTSUP)
 		pr_err("Sorry, we don't support this variable location yet.\n");
 	else if (ret == 0 && pf->pvar->field) {
 		ret = convert_variable_fields(vr_die, pf->pvar->var,
-- 
2.1.0


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

* [PATCH 08/17] perf tests: Fix to get negative exit codes
  2015-05-14 22:37 [GIT PULL 00/17] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (6 preceding siblings ...)
  2015-05-14 22:37 ` [PATCH 07/17] perf probe: Show better error message when failed to find variable Arnaldo Carvalho de Melo
@ 2015-05-14 22:38 ` Arnaldo Carvalho de Melo
  2015-05-14 22:38 ` [PATCH 09/17] perf report: Do not restrict -T option by other options Arnaldo Carvalho de Melo
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-14 22:38 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, He Kuang, Peter Zijlstra, Wang Nan,
	Arnaldo Carvalho de Melo

From: He Kuang <hekuang@huawei.com>

WEXITSTATUS consists of the least significant 8 bits of the status
argument, so we should convert the value to signed char if we have valid
negative exit codes. And the return value of test->func() contains
negative values:

  enum {
          TEST_OK   =  0,
          TEST_FAIL = -1,
          TEST_SKIP = -2,
  };

Before this patch:

  $ perf test -v 1
  ...
  test child finished with 254
  ---- end ----
  vmlinux symtab matches kallsyms: FAILED!

After this patch:

  $ perf test -v 1
  ...
  test child finished with -2
  ---- end ----
  vmlinux symtab matches kallsyms: Skip

Signed-off-by: He Kuang <hekuang@huawei.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1431347316-30401-1-git-send-email-hekuang@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/builtin-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 4f4098167112..f42af98a5c16 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -219,7 +219,7 @@ static int run_test(struct test *test)
 	wait(&status);
 
 	if (WIFEXITED(status)) {
-		err = WEXITSTATUS(status);
+		err = (signed char)WEXITSTATUS(status);
 		pr_debug("test child finished with %d\n", err);
 	} else if (WIFSIGNALED(status)) {
 		err = -1;
-- 
2.1.0


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

* [PATCH 09/17] perf report: Do not restrict -T option by other options
  2015-05-14 22:37 [GIT PULL 00/17] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (7 preceding siblings ...)
  2015-05-14 22:38 ` [PATCH 08/17] perf tests: Fix to get negative exit codes Arnaldo Carvalho de Melo
@ 2015-05-14 22:38 ` Arnaldo Carvalho de Melo
  2015-05-14 22:38 ` [PATCH 10/17] perf tests: Show refcounting broken expectations in thread-mg-share test Arnaldo Carvalho de Melo
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-14 22:38 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>

It seems there's no reason to suppress per-thread event stat by -T
option when -s or -p option is used.  Make it work with those options.

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/1431351879-23798-1-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-report.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 4d642db642c5..92fca2157e5e 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -333,15 +333,14 @@ static int perf_evlist__tty_browse_hists(struct perf_evlist *evlist,
 	}
 
 	if (sort_order == NULL &&
-	    parent_pattern == default_parent_pattern) {
+	    parent_pattern == default_parent_pattern)
 		fprintf(stdout, "#\n# (%s)\n#\n", help);
 
-		if (rep->show_threads) {
-			bool style = !strcmp(rep->pretty_printing_style, "raw");
-			perf_read_values_display(stdout, &rep->show_threads_values,
-						 style);
-			perf_read_values_destroy(&rep->show_threads_values);
-		}
+	if (rep->show_threads) {
+		bool style = !strcmp(rep->pretty_printing_style, "raw");
+		perf_read_values_display(stdout, &rep->show_threads_values,
+					 style);
+		perf_read_values_destroy(&rep->show_threads_values);
 	}
 
 	return 0;
-- 
2.1.0


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

* [PATCH 10/17] perf tests: Show refcounting broken expectations in thread-mg-share test
  2015-05-14 22:37 [GIT PULL 00/17] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (8 preceding siblings ...)
  2015-05-14 22:38 ` [PATCH 09/17] perf report: Do not restrict -T option by other options Arnaldo Carvalho de Melo
@ 2015-05-14 22:38 ` Arnaldo Carvalho de Melo
  2015-05-14 22:38 ` [PATCH 11/17] perf machine: No need to keep a refcnt for last_match Arnaldo Carvalho de Melo
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-14 22:38 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
	Borislav Petkov, David Ahern, Don Zickus, Frederic Weisbecker,
	Jiri Olsa, Namhyung Kim, Stephane Eranian

From: Arnaldo Carvalho de Melo <acme@redhat.com>

To help understand the failure.

  [acme@zoo linux]$ perf test -v 30
  30: Test thread mg sharing                                 :
  --- start ---
  test child forked, pid 12275
  FAILED tests/thread-mg-share.c:68 wrong refcnt (4 != 3)
  test child finished with -1
  ---- end ----
  Test thread mg sharing: FAILED!
  [acme@zoo linux]$

This is under investigation, the thread__delete() calls were replaced
with thread__put(), and those cause mismatches because now we need to be
more judicious with the thread lifetime management.

I.e. previously the thread__delete() would drop the map_group refcount,
but now since thread__put doesn't call thread__delete() necessarily.
because we have other refcount holders, the map_group refcount will not
be as we expected when this test was implemented.

Will be fixed soon...

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-9y8e3f7ukzco5loxvnlitpfq@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/tests.h           |  9 +++++++++
 tools/perf/tests/thread-mg-share.c | 12 ++++++------
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 52758a33f64c..a10eaf5c4767 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -9,6 +9,15 @@ do {									 \
 	}								 \
 } while (0)
 
+#define TEST_ASSERT_EQUAL(text, val, expected)				 \
+do {									 \
+	if (val != expected) {						 \
+		pr_debug("FAILED %s:%d %s (%d != %d)\n",		 \
+			 __FILE__, __LINE__, text, val, expected);	 \
+		return -1;						 \
+	}								 \
+} while (0)
+
 enum {
 	TEST_OK   =  0,
 	TEST_FAIL = -1,
diff --git a/tools/perf/tests/thread-mg-share.c b/tools/perf/tests/thread-mg-share.c
index dc05bd62b4a3..04ecceed4e5b 100644
--- a/tools/perf/tests/thread-mg-share.c
+++ b/tools/perf/tests/thread-mg-share.c
@@ -43,7 +43,7 @@ int test__thread_mg_share(void)
 			leader && t1 && t2 && t3 && other);
 
 	mg = leader->mg;
-	TEST_ASSERT_VAL("wrong refcnt", mg->refcnt == 4);
+	TEST_ASSERT_EQUAL("wrong refcnt", mg->refcnt, 4);
 
 	/* test the map groups pointer is shared */
 	TEST_ASSERT_VAL("map groups don't match", mg == t1->mg);
@@ -59,25 +59,25 @@ int test__thread_mg_share(void)
 	TEST_ASSERT_VAL("failed to find other leader", other_leader);
 
 	other_mg = other->mg;
-	TEST_ASSERT_VAL("wrong refcnt", other_mg->refcnt == 2);
+	TEST_ASSERT_EQUAL("wrong refcnt", other_mg->refcnt, 2);
 
 	TEST_ASSERT_VAL("map groups don't match", other_mg == other_leader->mg);
 
 	/* release thread group */
 	thread__put(leader);
-	TEST_ASSERT_VAL("wrong refcnt", mg->refcnt == 3);
+	TEST_ASSERT_EQUAL("wrong refcnt", mg->refcnt, 3);
 
 	thread__put(t1);
-	TEST_ASSERT_VAL("wrong refcnt", mg->refcnt == 2);
+	TEST_ASSERT_EQUAL("wrong refcnt", mg->refcnt, 2);
 
 	thread__put(t2);
-	TEST_ASSERT_VAL("wrong refcnt", mg->refcnt == 1);
+	TEST_ASSERT_EQUAL("wrong refcnt", mg->refcnt, 1);
 
 	thread__put(t3);
 
 	/* release other group  */
 	thread__put(other_leader);
-	TEST_ASSERT_VAL("wrong refcnt", other_mg->refcnt == 1);
+	TEST_ASSERT_EQUAL("wrong refcnt", other_mg->refcnt, 1);
 
 	thread__put(other);
 
-- 
2.1.0


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

* [PATCH 11/17] perf machine: No need to keep a refcnt for last_match
  2015-05-14 22:37 [GIT PULL 00/17] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (9 preceding siblings ...)
  2015-05-14 22:38 ` [PATCH 10/17] perf tests: Show refcounting broken expectations in thread-mg-share test Arnaldo Carvalho de Melo
@ 2015-05-14 22:38 ` Arnaldo Carvalho de Melo
  2015-05-14 22:38 ` [PATCH 12/17] perf tests: Fix map_groups refcount test Arnaldo Carvalho de Melo
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-14 22:38 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
	Borislav Petkov, David Ahern, Don Zickus, Frederic Weisbecker,
	Jiri Olsa, Namhyung Kim, Stephane Eranian

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Since it is all associated with the refcount for keeping the thread
in the rbtree, it is excessive and unecessarily complex to hold a
refcont when changing machine->last_match.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-98kuesmfwtvhsrzx7ttyb0kt@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/machine.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 8b0b307d91f4..34bf89f7f4f3 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -364,7 +364,7 @@ static struct thread *____machine__findnew_thread(struct machine *machine,
 			return th;
 		}
 
-		thread__zput(machine->last_match);
+		machine->last_match = NULL;
 	}
 
 	while (*p != NULL) {
@@ -372,7 +372,7 @@ static struct thread *____machine__findnew_thread(struct machine *machine,
 		th = rb_entry(parent, struct thread, rb_node);
 
 		if (th->tid == tid) {
-			machine->last_match = thread__get(th);
+			machine->last_match = th;
 			machine__update_thread_pid(machine, th, pid);
 			return th;
 		}
@@ -409,7 +409,7 @@ static struct thread *____machine__findnew_thread(struct machine *machine,
 		 * It is now in the rbtree, get a ref
 		 */
 		thread__get(th);
-		machine->last_match = thread__get(th);
+		machine->last_match = th;
 	}
 
 	return th;
@@ -1309,7 +1309,7 @@ out_problem:
 static void __machine__remove_thread(struct machine *machine, struct thread *th, bool lock)
 {
 	if (machine->last_match == th)
-		thread__zput(machine->last_match);
+		machine->last_match = NULL;
 
 	BUG_ON(th->refcnt.counter == 0);
 	if (lock)
-- 
2.1.0


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

* [PATCH 12/17] perf tests: Fix map_groups refcount test
  2015-05-14 22:37 [GIT PULL 00/17] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (10 preceding siblings ...)
  2015-05-14 22:38 ` [PATCH 11/17] perf machine: No need to keep a refcnt for last_match Arnaldo Carvalho de Melo
@ 2015-05-14 22:38 ` Arnaldo Carvalho de Melo
  2015-05-14 22:38 ` [PATCH 13/17] perf probe: Ignore tail calls to probed functions Arnaldo Carvalho de Melo
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-14 22:38 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
	Borislav Petkov, David Ahern, Don Zickus, Frederic Weisbecker,
	Jiri Olsa, Namhyung Kim, Stephane Eranian

From: Arnaldo Carvalho de Melo <acme@redhat.com>

When introducing reference counting for struct thread instances I forgot
to remove the synthetic threads from the machine's rbtree so that it
then the threads would have just one reference and thus the
thread__put() replacing the thread__delete() really turns into a
thread__delete() (thread->refcnt == 1 at thread__put() time) and thus
drop the thread->mg refcount, as expected by the this test.

Fix it by calling machine__remove_thread() (the counterpart of
machine__findnew_thread()) on all the synthetic threads after the
checks that involves the rbtree were done.

Before:

  # perf test -v mg
  30: Test thread mg sharing                                 :
  --- start ---
  test child forked, pid 26995
  FAILED tests/thread-mg-share.c:68 wrong refcnt (4 != 3)
  test child finished with -1
  ---- end ----
  Test thread mg sharing: FAILED!
  #

After:

  # perf test mg
  30: Test thread mg sharing: Ok
  #

Fixes: b91fc39f4ad7 ("perf machine: Protect the machine->threads with a rwlock")
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-uoqq0fjei90ohhhcboz6ay33@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/thread-mg-share.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/tools/perf/tests/thread-mg-share.c b/tools/perf/tests/thread-mg-share.c
index 04ecceed4e5b..c0ed56f7efc6 100644
--- a/tools/perf/tests/thread-mg-share.c
+++ b/tools/perf/tests/thread-mg-share.c
@@ -58,6 +58,18 @@ int test__thread_mg_share(void)
 	other_leader = machine__find_thread(machine, 4, 4);
 	TEST_ASSERT_VAL("failed to find other leader", other_leader);
 
+	/*
+	 * Ok, now that all the rbtree related operations were done,
+	 * lets remove all of them from there so that we can do the
+	 * refcounting tests.
+	 */
+	machine__remove_thread(machine, leader);
+	machine__remove_thread(machine, t1);
+	machine__remove_thread(machine, t2);
+	machine__remove_thread(machine, t3);
+	machine__remove_thread(machine, other);
+	machine__remove_thread(machine, other_leader);
+
 	other_mg = other->mg;
 	TEST_ASSERT_EQUAL("wrong refcnt", other_mg->refcnt, 2);
 
@@ -81,11 +93,6 @@ int test__thread_mg_share(void)
 
 	thread__put(other);
 
-	/*
-	 * Cannot call machine__delete_threads(machine) now,
-	 * because we've already released all the threads.
-	 */
-
 	machines__exit(&machines);
 	return 0;
 }
-- 
2.1.0


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

* [PATCH 13/17] perf probe: Ignore tail calls to probed functions
  2015-05-14 22:37 [GIT PULL 00/17] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (11 preceding siblings ...)
  2015-05-14 22:38 ` [PATCH 12/17] perf tests: Fix map_groups refcount test Arnaldo Carvalho de Melo
@ 2015-05-14 22:38 ` Arnaldo Carvalho de Melo
  2015-05-14 22:38 ` [PATCH 14/17] perf report: Fix some option handling on --stdio Arnaldo Carvalho de Melo
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-14 22:38 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Naveen N. Rao, Arnaldo Carvalho de Melo

From: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>

perf probe currently errors out if there are any tail calls to probed
functions:

[root@rhel71be]# perf probe do_fork
Failed to find probe point in any functions.
  Error: Failed to add events.

Fix this by teaching perf to ignore tail calls.

Without patch:

  [root@rhel71be perf]# ./perf probe -v do_fork
  probe-definition(0): do_fork symbol:do_fork file:(null) line:0 offset:0
  return:0 lazy:(null)
  0 arguments
  Looking at the vmlinux_path (7 entries long)
  symsrc__init: build id mismatch for /boot/vmlinux.
  Using /usr/lib/debug/lib/modules/3.10.0-201.el7.ppc64/vmlinux for symbols
  Open Debuginfo file:
  /usr/lib/debug/lib/modules/3.10.0-201.el7.ppc64/vmlinux
  Try to find probe point from debuginfo.
  found inline addr: 0xc0000000000bb9b0
  Probe point found: do_fork+0
  found inline addr: 0xc0000000000bbe20
  Probe point found: kernel_thread+48
  found inline addr: 0xc0000000000bbe5c
  Probe point found: sys_fork+28
  found inline addr: 0xc0000000000bbfac
  Probe point found: sys_vfork+44
  found inline addr: 0xc0000000000bc27c
  Failed to find probe point in any functions.
  An error occurred in debuginfo analysis (-2).
  Error: Failed to add events. Reason: No such file or directory (Code: -2)

With patch:

  [root@rhel71be perf]# ./perf probe -v do_fork
  probe-definition(0): do_fork symbol:do_fork file:(null) line:0 offset:0
  return:0 lazy:(null)
  0 arguments
  Looking at the vmlinux_path (7 entries long)
  symsrc__init: build id mismatch for /boot/vmlinux.
  Using /usr/lib/debug/lib/modules/3.10.0-201.el7.ppc64/vmlinux for symbols
  Open Debuginfo file:
  /usr/lib/debug/lib/modules/3.10.0-201.el7.ppc64/vmlinux
  Try to find probe point from debuginfo.
  found inline addr: 0xc0000000000bb9b0
  Probe point found: do_fork+0
  found inline addr: 0xc0000000000bbe20
  Probe point found: kernel_thread+48
  found inline addr: 0xc0000000000bbe5c
  Probe point found: sys_fork+28
  found inline addr: 0xc0000000000bbfac
  Probe point found: sys_vfork+44
  found inline addr: 0xc0000000000bc27c
  Ignoring tail call from SyS_clone
  Found 4 probe_trace_events.
  Opening /sys/kernel/debug/tracing/kprobe_events write=1
  No kprobe blacklist support, ignored
  Added new events:
  Writing event: p:probe/do_fork _text+768432
  Failed to write event: Invalid argument
    Error: Failed to add events. Reason: Invalid argument (Code: -22)

[Ignore the error about failure to write event - this kernel is missing
a patch to resolve _text properly]

The reason to ignore tail calls is that the address does not belong to
any function frame. In the example above, the address in SyS_clone is
0xc0000000000bc27c, but looking at the debug-info:

 <1><830081>: Abbrev Number: 133 (DW_TAG_subprogram)
    <830083>   DW_AT_external    : 1
    <830083>   DW_AT_name        : (indirect string, offset: 0x3cea3): SyS_clone
    <830087>   DW_AT_decl_file   : 7
    <830088>   DW_AT_decl_line   : 1689
    <83008a>   DW_AT_prototyped  : 1
    <83008a>   DW_AT_type        : <0x8110eb>
    <83008e>   DW_AT_low_pc      : 0xc0000000000bc270
    <830096>   DW_AT_high_pc     : 0xc
    <83009e>   DW_AT_frame_base  : 1 byte block: 9c 	(DW_OP_call_frame_cfa)
    <8300a0>   DW_AT_GNU_all_call_sites: 1
    <8300a0>   DW_AT_sibling     : <0x830178>
<snip>
 <3><830147>: Abbrev Number: 125 (DW_TAG_GNU_call_site)
    <830148>   DW_AT_low_pc      : 0xc0000000000bc27c
    <830150>   DW_AT_GNU_tail_call: 1
    <830150>   DW_AT_abstract_origin: <0x82e7e1>

The frame ends at 0xc0000000000bc27c. I suppose this is why this
particular call is a "tail" call. FWIW, systemtap seems to ignore these
as well and requires users to explicitly place probes at these call
sites if necessary. I print out the caller so that users know.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Link: http://lkml.kernel.org/r/1430394151-15928-1-git-send-email-naveen.n.rao@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/dwarf-aux.c    | 37 +++++++++++++++++++++++++++++++++++++
 tools/perf/util/dwarf-aux.h    |  4 ++++
 tools/perf/util/probe-finder.c | 12 +++++++++---
 3 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index afa09719336b..eb47abdcf0ac 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -433,6 +433,43 @@ struct __addr_die_search_param {
 	Dwarf_Die	*die_mem;
 };
 
+static int __die_search_func_tail_cb(Dwarf_Die *fn_die, void *data)
+{
+	struct __addr_die_search_param *ad = data;
+	Dwarf_Addr addr = 0;
+
+	if (dwarf_tag(fn_die) == DW_TAG_subprogram &&
+	    !dwarf_highpc(fn_die, &addr) &&
+	    addr == ad->addr) {
+		memcpy(ad->die_mem, fn_die, sizeof(Dwarf_Die));
+		return DWARF_CB_ABORT;
+	}
+	return DWARF_CB_OK;
+}
+
+/**
+ * die_find_tailfunc - Search for a non-inlined function with tail call at
+ * given address
+ * @cu_die: a CU DIE which including @addr
+ * @addr: target address
+ * @die_mem: a buffer for result DIE
+ *
+ * Search for a non-inlined function DIE with tail call at @addr. Stores the
+ * DIE to @die_mem and returns it if found. Returns NULL if failed.
+ */
+Dwarf_Die *die_find_tailfunc(Dwarf_Die *cu_die, Dwarf_Addr addr,
+				    Dwarf_Die *die_mem)
+{
+	struct __addr_die_search_param ad;
+	ad.addr = addr;
+	ad.die_mem = die_mem;
+	/* dwarf_getscopes can't find subprogram. */
+	if (!dwarf_getfuncs(cu_die, __die_search_func_tail_cb, &ad, 0))
+		return NULL;
+	else
+		return die_mem;
+}
+
 /* die_find callback for non-inlined function search */
 static int __die_search_func_cb(Dwarf_Die *fn_die, void *data)
 {
diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
index c154c0b80880..c42ec366f2a7 100644
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -85,6 +85,10 @@ extern Dwarf_Die *die_find_child(Dwarf_Die *rt_die,
 extern Dwarf_Die *die_find_realfunc(Dwarf_Die *cu_die, Dwarf_Addr addr,
 				    Dwarf_Die *die_mem);
 
+/* Search a non-inlined function with tail call at given address */
+Dwarf_Die *die_find_tailfunc(Dwarf_Die *cu_die, Dwarf_Addr addr,
+				    Dwarf_Die *die_mem);
+
 /* Search the top inlined function including given address */
 extern Dwarf_Die *die_find_top_inlinefunc(Dwarf_Die *sp_die, Dwarf_Addr addr,
 					  Dwarf_Die *die_mem);
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 590a24a7f555..c50da392e256 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -674,9 +674,15 @@ static int call_probe_finder(Dwarf_Die *sc_die, struct probe_finder *pf)
 	/* If not a real subprogram, find a real one */
 	if (!die_is_func_def(sc_die)) {
 		if (!die_find_realfunc(&pf->cu_die, pf->addr, &pf->sp_die)) {
-			pr_warning("Failed to find probe point in any "
-				   "functions.\n");
-			return -ENOENT;
+			if (die_find_tailfunc(&pf->cu_die, pf->addr, &pf->sp_die)) {
+				pr_warning("Ignoring tail call from %s\n",
+						dwarf_diename(&pf->sp_die));
+				return 0;
+			} else {
+				pr_warning("Failed to find probe point in any "
+					   "functions.\n");
+				return -ENOENT;
+			}
 		}
 	} else
 		memcpy(&pf->sp_die, sc_die, sizeof(Dwarf_Die));
-- 
2.1.0


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

* [PATCH 14/17] perf report: Fix some option handling on --stdio
  2015-05-14 22:37 [GIT PULL 00/17] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (12 preceding siblings ...)
  2015-05-14 22:38 ` [PATCH 13/17] perf probe: Ignore tail calls to probed functions Arnaldo Carvalho de Melo
@ 2015-05-14 22:38 ` Arnaldo Carvalho de Melo
  2015-05-14 22:38 ` [PATCH 15/17] tools lib traceevent: Provide le16toh define for older systems Arnaldo Carvalho de Melo
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-14 22:38 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, David Ahern, Jiri Olsa,
	Peter Zijlstra, Taeung Song, Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung@kernel.org>

There's a bug that perf report sometimes ignore some options on --stdio
output.  This bug is triggered only if a related config variable is set.
For example, let's assume we have a following config file.

  $ cat ~/.perfconfig
  [call-graph]
    print-type = graph
  [hist]
    percentage = absolute

Then, following perf config will not honor some options.

  $ perf record -ag sleep 1
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.199 MB perf.data (77 samples) ]

  $ perf report -g none --stdio
  # To display the perf.data header info, please use --header/--header-only options.
  #
  # Samples: 77  of event 'cycles'
  # Event count (approx.): 25425383
  #
  # Overhead  Command          Shared Object            Symbol
  # ........  ...............  .......................  ..............
  #
      16.34%  swapper          [kernel.vmlinux]         [k] intel_idle
                      |
                      ---intel_idle
                         cpuidle_enter_state
                         cpuidle_enter
                         cpu_startup_entry
   ...

With '-g none' option, it should not show callchains, but it still shows
callchains.  However it works as expected on --tui output.

Similarly, '--percentage relative' option is not work and still shows a
absolute percentage values.

Looking at the source, I found that those setting were overwritten by
config variables when setup_pager() called.  The setup_pager() is to
start a pager process so that it can manage long lines of output on the
stdio mode.  But as it calls the perf_config() after parsing arguments,
the settings were overwritten regardless of command line options.

The reason it calls perf_config() is to find the 'pager_program' which
might be set by a config variable, I guess.  However current perf code
does not provide the config variable for it, so it's just meaningless
IMHO.  Eliminating the call makes the option working as expected.

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>
Cc: Taeung Song <treeze.taeung@gmail.com>
Link: http://lkml.kernel.org/r/1431529406-6762-1-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/cache.h       | 1 -
 tools/perf/util/environment.c | 1 -
 tools/perf/util/pager.c       | 5 -----
 3 files changed, 7 deletions(-)

diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index fbcca21d66ab..c861373aaed3 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -30,7 +30,6 @@ extern const char *perf_config_dirname(const char *, const char *);
 
 /* pager.c */
 extern void setup_pager(void);
-extern const char *pager_program;
 extern int pager_in_use(void);
 extern int pager_use_color;
 
diff --git a/tools/perf/util/environment.c b/tools/perf/util/environment.c
index 275b0ee345f5..7405123692f1 100644
--- a/tools/perf/util/environment.c
+++ b/tools/perf/util/environment.c
@@ -5,5 +5,4 @@
  */
 #include "cache.h"
 
-const char *pager_program;
 int pager_use_color = 1;
diff --git a/tools/perf/util/pager.c b/tools/perf/util/pager.c
index 31ee02d4e988..53ef006a951c 100644
--- a/tools/perf/util/pager.c
+++ b/tools/perf/util/pager.c
@@ -50,11 +50,6 @@ void setup_pager(void)
 
 	if (!isatty(1))
 		return;
-	if (!pager) {
-		if (!pager_program)
-			perf_config(perf_default_config, NULL);
-		pager = pager_program;
-	}
 	if (!pager)
 		pager = getenv("PAGER");
 	if (!(pager || access("/usr/bin/pager", X_OK)))
-- 
2.1.0


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

* [PATCH 15/17] tools lib traceevent: Provide le16toh define for older systems
  2015-05-14 22:37 [GIT PULL 00/17] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (13 preceding siblings ...)
  2015-05-14 22:38 ` [PATCH 14/17] perf report: Fix some option handling on --stdio Arnaldo Carvalho de Melo
@ 2015-05-14 22:38 ` Arnaldo Carvalho de Melo
  2015-05-14 22:38 ` [PATCH 16/17] perf trace: Fix the build on older distros Arnaldo Carvalho de Melo
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-14 22:38 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
	Borislav Petkov, David Ahern, Don Zickus, Frederic Weisbecker,
	Jiri Olsa, Namhyung Kim, Stephane Eranian, Steven Rostedt

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Where such macro is not present, so just copy its definition from
glibc's endian.h and define it if not already.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Link: http://lkml.kernel.org/n/tip-4j90i2na07ppidt0z6cbuxr7@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/traceevent/plugin_cfg80211.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tools/lib/traceevent/plugin_cfg80211.c b/tools/lib/traceevent/plugin_cfg80211.c
index 4592d8438318..ec57d0c1fbc2 100644
--- a/tools/lib/traceevent/plugin_cfg80211.c
+++ b/tools/lib/traceevent/plugin_cfg80211.c
@@ -4,6 +4,19 @@
 #include <endian.h>
 #include "event-parse.h"
 
+/*
+ * From glibc endian.h, for older systems where it is not present, e.g.: RHEL5,
+ * Fedora6.
+ */
+#ifndef le16toh
+# if __BYTE_ORDER == __LITTLE_ENDIAN
+#  define le16toh(x) (x)
+# else
+#  define le16toh(x) __bswap_16 (x)
+# endif
+#endif
+
+
 static unsigned long long
 process___le16_to_cpup(struct trace_seq *s, unsigned long long *args)
 {
-- 
2.1.0


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

* [PATCH 16/17] perf trace: Fix the build on older distros
  2015-05-14 22:37 [GIT PULL 00/17] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (14 preceding siblings ...)
  2015-05-14 22:38 ` [PATCH 15/17] tools lib traceevent: Provide le16toh define for older systems Arnaldo Carvalho de Melo
@ 2015-05-14 22:38 ` Arnaldo Carvalho de Melo
  2015-05-14 22:38 ` [PATCH 17/17] perf tools: Make flex/bison calls honour V=1 Arnaldo Carvalho de Melo
  2015-05-15  6:39 ` [GIT PULL 00/17] perf/core improvements and fixes Ingo Molnar
  17 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-14 22:38 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
	Borislav Petkov, David Ahern, Don Zickus, Frederic Weisbecker,
	Jiri Olsa, Namhyung Kim, Stephane Eranian, Vinson Lee

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Such as RHEL5, where CLOEXEC, NONBLOCK flags are not present, use a
ifdef+define approach instead to make it build on all distros.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Vinson Lee <vlee@twitter.com>
Link: http://lkml.kernel.org/n/tip-pioazikk9d9oz5qdeor3eldu@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-trace.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 96a2ebafc755..a05490d06374 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -16,7 +16,6 @@
 
 #include <libaudit.h>
 #include <stdlib.h>
-#include <sys/eventfd.h>
 #include <sys/mman.h>
 #include <linux/futex.h>
 
@@ -41,6 +40,34 @@
 # define EFD_SEMAPHORE		1
 #endif
 
+#ifndef EFD_NONBLOCK
+# define EFD_NONBLOCK		00004000
+#endif
+
+#ifndef EFD_CLOEXEC
+# define EFD_CLOEXEC		02000000
+#endif
+
+#ifndef O_CLOEXEC
+# define O_CLOEXEC		02000000
+#endif
+
+#ifndef SOCK_DCCP
+# define SOCK_DCCP		6
+#endif
+
+#ifndef SOCK_CLOEXEC
+# define SOCK_CLOEXEC		02000000
+#endif
+
+#ifndef SOCK_NONBLOCK
+# define SOCK_NONBLOCK		00004000
+#endif
+
+#ifndef MSG_CMSG_CLOEXEC
+# define MSG_CMSG_CLOEXEC	0x40000000
+#endif
+
 struct tp_field {
 	int offset;
 	union {
-- 
2.1.0


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

* [PATCH 17/17] perf tools: Make flex/bison calls honour V=1
  2015-05-14 22:37 [GIT PULL 00/17] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (15 preceding siblings ...)
  2015-05-14 22:38 ` [PATCH 16/17] perf trace: Fix the build on older distros Arnaldo Carvalho de Melo
@ 2015-05-14 22:38 ` Arnaldo Carvalho de Melo
  2015-05-15  6:39 ` [GIT PULL 00/17] perf/core improvements and fixes Ingo Molnar
  17 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-05-14 22:38 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Jiri Olsa, Jiri Olsa, Adrian Hunter,
	Borislav Petkov, David Ahern, Don Zickus, Frederic Weisbecker,
	Namhyung Kim, Stephane Eranian, Arnaldo Carvalho de Melo

From: Jiri Olsa <jolsa@kernel.org>

Reported-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-dnc2ggwhffdpuvijwq4rkic9@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/Build | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index d552203aead0..6966d0743bf7 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -102,19 +102,19 @@ CFLAGS_exec_cmd.o += -DPERF_EXEC_PATH="BUILD_STR($(perfexecdir_SQ))" -DPREFIX="B
 
 $(OUTPUT)util/parse-events-flex.c: util/parse-events.l $(OUTPUT)util/parse-events-bison.c
 	$(call rule_mkdir)
-	@$(call echo-cmd,flex)$(FLEX) -o $@ --header-file=$(OUTPUT)util/parse-events-flex.h $(PARSER_DEBUG_FLEX) util/parse-events.l
+	$(Q)$(call echo-cmd,flex)$(FLEX) -o $@ --header-file=$(OUTPUT)util/parse-events-flex.h $(PARSER_DEBUG_FLEX) util/parse-events.l
 
 $(OUTPUT)util/parse-events-bison.c: util/parse-events.y
 	$(call rule_mkdir)
-	@$(call echo-cmd,bison)$(BISON) -v util/parse-events.y -d $(PARSER_DEBUG_BISON) -o $@ -p parse_events_
+	$(Q)$(call echo-cmd,bison)$(BISON) -v util/parse-events.y -d $(PARSER_DEBUG_BISON) -o $@ -p parse_events_
 
 $(OUTPUT)util/pmu-flex.c: util/pmu.l $(OUTPUT)util/pmu-bison.c
 	$(call rule_mkdir)
-	@$(call echo-cmd,flex)$(FLEX) -o $@ --header-file=$(OUTPUT)util/pmu-flex.h util/pmu.l
+	$(Q)$(call echo-cmd,flex)$(FLEX) -o $@ --header-file=$(OUTPUT)util/pmu-flex.h util/pmu.l
 
 $(OUTPUT)util/pmu-bison.c: util/pmu.y
 	$(call rule_mkdir)
-	@$(call echo-cmd,bison)$(BISON) -v util/pmu.y -d -o $@ -p perf_pmu_
+	$(Q)$(call echo-cmd,bison)$(BISON) -v util/pmu.y -d -o $@ -p perf_pmu_
 
 CFLAGS_parse-events-flex.o  += -w
 CFLAGS_pmu-flex.o           += -w
-- 
2.1.0


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

* Re: [GIT PULL 00/17] perf/core improvements and fixes
  2015-05-14 22:37 [GIT PULL 00/17] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (16 preceding siblings ...)
  2015-05-14 22:38 ` [PATCH 17/17] perf tools: Make flex/bison calls honour V=1 Arnaldo Carvalho de Melo
@ 2015-05-15  6:39 ` Ingo Molnar
  17 siblings, 0 replies; 20+ messages in thread
From: Ingo Molnar @ 2015-05-15  6:39 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Adrian Hunter, Borislav Petkov, David Ahern,
	Don Zickus, Frederic Weisbecker, He Kuang, Jiri Olsa,
	Masami Hiramatsu, Namhyung Kim, Naveen N . Rao, Peter Zijlstra,
	Stephane Eranian, Steven Rostedt, Taeung Song, Vinson Lee,
	Wang Nan, Arnaldo Carvalho de Melo


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

> Hi Ingo,
> 
> 	Please consider pulling,
> 
> - Arnaldo
> 
> The following changes since commit a41f3c8cd4e28dcbebd8ec27a9602c86cfa5f009:
> 
>   perf/x86/intel/uncore: Add Broadwell-U uncore IMC PMU support (2015-05-11 11:57:47 +0200)
> 
> 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 70923bd26c732ac2d9e248d80214df6dfd75f78c:
> 
>   perf tools: Make flex/bison calls honour V=1 (2015-05-14 19:27:47 -0300)
> 
> ----------------------------------------------------------------
> perf/core improvements and fixes:
> 
> User visible:
> 
> - Add --range option to show a variable's location range in 'perf probe',
>   helping in collecting variables in probes when there is a mismatch
>   between assembly and source code (He Kuang)
> 
> - Show better error message when failed to find variable in 'perf probe' (He Kuang)
> 
> - Fix 'perf report --thread' handling and document it better (Namhyung Kim)
> 
> Infrastructure:
> 
> - Fix to get negative exit codes in 'perf test' test routines (He Kuang)
> 
> - Make flex/bison calls honour V=1 (Jiri Olsa)
> 
> - Ignore tail calls to probed functions in 'perf probe' (Naveen N. Rao)
> 
> - Fix refcount expectations in map_group share 'perf test' (Arnaldo Carvalho de Melo)
> 
> Build Fixes:
> 
> - Fix 'perf kmem' build due to compiler thinking uninitialized var is
>   being accessed (Arnaldo Carvalho de Melo)
> 
> - Provide le16toh if not defined, to fix the libtraceevent build on
>   older distros (Arnaldo Carvalho de Melo)
> 
> - Fix 'perf trace' build on older distros by providing some CLOEXEC, NONBLOCK
>   defines (Arnaldo Carvalho de Melo)
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> ----------------------------------------------------------------
> Arnaldo Carvalho de Melo (6):
>       perf kmem: Fix compiler warning about may be accessing uninitialized variable
>       perf tests: Show refcounting broken expectations in thread-mg-share test
>       perf machine: No need to keep a refcnt for last_match
>       perf tests: Fix map_groups refcount test
>       tools lib traceevent: Provide le16toh define for older systems
>       perf trace: Fix the build on older distros
> 
> He Kuang (5):
>       perf trace: Removed duplicated NULL test
>       perf probe: Remove length limitation for showing available variables
>       perf probe: Add --range option to show a variable's location range
>       perf probe: Show better error message when failed to find variable
>       perf tests: Fix to get negative exit codes
> 
> Jiri Olsa (1):
>       perf tools: Make flex/bison calls honour V=1
> 
> Namhyung Kim (4):
>       perf tools: Document relation of per-thread event count feature
>       perf report: Force tty output if -T/--thread option is given
>       perf report: Do not restrict -T option by other options
>       perf report: Fix some option handling on --stdio
> 
> Naveen N. Rao (1):
>       perf probe: Ignore tail calls to probed functions
> 
>  tools/lib/traceevent/plugin_cfg80211.c   |  13 ++
>  tools/perf/Documentation/perf-record.txt |   3 +-
>  tools/perf/Documentation/perf-report.txt |   3 +-
>  tools/perf/builtin-kmem.c                |   2 +-
>  tools/perf/builtin-probe.c               |   2 +
>  tools/perf/builtin-report.c              |  17 ++-
>  tools/perf/builtin-trace.c               |  32 ++++-
>  tools/perf/tests/builtin-test.c          |   2 +-
>  tools/perf/tests/tests.h                 |   9 ++
>  tools/perf/tests/thread-mg-share.c       |  29 +++--
>  tools/perf/util/Build                    |   8 +-
>  tools/perf/util/cache.h                  |   1 -
>  tools/perf/util/dwarf-aux.c              | 204 +++++++++++++++++++++++++++----
>  tools/perf/util/dwarf-aux.h              |  10 +-
>  tools/perf/util/environment.c            |   1 -
>  tools/perf/util/machine.c                |   8 +-
>  tools/perf/util/pager.c                  |   5 -
>  tools/perf/util/probe-event.h            |   1 +
>  tools/perf/util/probe-finder.c           |  90 ++++++++++----
>  19 files changed, 346 insertions(+), 94 deletions(-)

Pulled, thanks a lot Arnaldo!

	Ingo

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

* Re: [PATCH 06/17] perf probe: Add --range option to show a variable's location range
  2015-05-14 22:37 ` [PATCH 06/17] perf probe: Add --range option to show a variable's location range Arnaldo Carvalho de Melo
@ 2015-06-25 12:30   ` Alexei Starovoitov
  0 siblings, 0 replies; 20+ messages in thread
From: Alexei Starovoitov @ 2015-06-25 12:30 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, linux-kernel, He Kuang, Jiri Olsa, Peter Zijlstra,
	Wang Nan, Arnaldo Carvalho de Melo

On Thu, May 14, 2015 at 07:37:58PM -0300, Arnaldo Carvalho de Melo wrote:
> From: He Kuang <hekuang@huawei.com>
> 
> It is not easy for users to get the accurate byte offset or the line
> number where a local variable can be probed.
> 
> With '--range' option, local variables in the scope of the probe point
> are showed with a byte offset range, and can be added according to this
> range information.
...
> +	while ((offset = dwarf_getlocations(
> +				&attr, offset, &base,
> +				&start, &end, &op, &nops)) > 0) {

dwarf_getlocations only exists in the new libdw, so perf stopped
building with libdw from ubuntu 12.04.
Yeah, it's old, but can we add a weak dwarf_getlocations(), so
that we don't need to force everyone upgrading elfutils/libdw
for that feature?


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

end of thread, other threads:[~2015-06-25 12:31 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-14 22:37 [GIT PULL 00/17] perf/core improvements and fixes Arnaldo Carvalho de Melo
2015-05-14 22:37 ` [PATCH 01/17] perf kmem: Fix compiler warning about may be accessing uninitialized variable Arnaldo Carvalho de Melo
2015-05-14 22:37 ` [PATCH 02/17] perf tools: Document relation of per-thread event count feature Arnaldo Carvalho de Melo
2015-05-14 22:37 ` [PATCH 03/17] perf report: Force tty output if -T/--thread option is given Arnaldo Carvalho de Melo
2015-05-14 22:37 ` [PATCH 04/17] perf trace: Removed duplicated NULL test Arnaldo Carvalho de Melo
2015-05-14 22:37 ` [PATCH 05/17] perf probe: Remove length limitation for showing available variables Arnaldo Carvalho de Melo
2015-05-14 22:37 ` [PATCH 06/17] perf probe: Add --range option to show a variable's location range Arnaldo Carvalho de Melo
2015-06-25 12:30   ` Alexei Starovoitov
2015-05-14 22:37 ` [PATCH 07/17] perf probe: Show better error message when failed to find variable Arnaldo Carvalho de Melo
2015-05-14 22:38 ` [PATCH 08/17] perf tests: Fix to get negative exit codes Arnaldo Carvalho de Melo
2015-05-14 22:38 ` [PATCH 09/17] perf report: Do not restrict -T option by other options Arnaldo Carvalho de Melo
2015-05-14 22:38 ` [PATCH 10/17] perf tests: Show refcounting broken expectations in thread-mg-share test Arnaldo Carvalho de Melo
2015-05-14 22:38 ` [PATCH 11/17] perf machine: No need to keep a refcnt for last_match Arnaldo Carvalho de Melo
2015-05-14 22:38 ` [PATCH 12/17] perf tests: Fix map_groups refcount test Arnaldo Carvalho de Melo
2015-05-14 22:38 ` [PATCH 13/17] perf probe: Ignore tail calls to probed functions Arnaldo Carvalho de Melo
2015-05-14 22:38 ` [PATCH 14/17] perf report: Fix some option handling on --stdio Arnaldo Carvalho de Melo
2015-05-14 22:38 ` [PATCH 15/17] tools lib traceevent: Provide le16toh define for older systems Arnaldo Carvalho de Melo
2015-05-14 22:38 ` [PATCH 16/17] perf trace: Fix the build on older distros Arnaldo Carvalho de Melo
2015-05-14 22:38 ` [PATCH 17/17] perf tools: Make flex/bison calls honour V=1 Arnaldo Carvalho de Melo
2015-05-15  6:39 ` [GIT PULL 00/17] perf/core improvements and fixes Ingo Molnar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.