linux-kernel.vger.kernel.org archive mirror
 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 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).