All of lore.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL 0/8] perf/core improvements and fixes
@ 2015-06-17 21:22 Arnaldo Carvalho de Melo
  2015-06-17 21:22 ` [PATCH 1/8] perf tools: Ignore .config-detected in .gitignore Arnaldo Carvalho de Melo
                   ` (8 more replies)
  0 siblings, 9 replies; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-06-17 21:22 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, Li Zhang, Masami Hiramatsu, Namhyung Kim,
	Naohiro Aota, Peter Zijlstra, pi3orama, Stephane Eranian,
	Sukadev Bhattiprolu, Wang Nan, Zefan Li,
	Arnaldo Carvalho de Melo

Hi Ingo,

	Please consider pulling, this is on top of perf-core-for-mingo, that is
still outstanding,

Thanks!

- Arnaldo

The following changes since commit b031220d520238075bd99513a420e65cf37866ad:

  perf probe: Fix to return error if no probe is added (2015-06-16 11:39:51 -0300)

are available in the git repository at:

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

for you to fetch changes up to 5d484f99aed547e235f2229653c95392a1bc3692:

  perf top: Allow disabling/enabling events dynamicly (2015-06-17 16:50:52 -0300)

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

User visible:

- Allow disabling/enabling events dynamicly in 'perf top':
  a 'perf top' session can instantly become a 'perf report'
  one, i.e. going from dynamic analysis to a static one,
  returning to a dynamic one is possible, to toogle the
  modes, just press CTRL+z. (Arnaldo Carvalho de Melo)

- Greatly speed up 'perf probe --list' by caching debuginfo
  (Masami Hiramatsu)

- Fix 'perf trace' race condition at the end of started
  workloads (Sukadev Bhattiprolu)

- Fix a problem when opening old perf.data with different
  byte order (Wang Nan)

Infrastructure:

- Ignore .config-detected in .gitignore (Wang Nan)

- Move libtraceevent dynamic list to separated LDFLAGS
  variable (Wang Nan)

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

----------------------------------------------------------------
Arnaldo Carvalho de Melo (2):
      perf evlist: Add toggle_enable() method
      perf top: Allow disabling/enabling events dynamicly

Masami Hiramatsu (2):
      perf probe: Show usage even if the last event is skipped
      perf probe: Speed up perf probe --list by caching debuginfo

Sukadev Bhattiprolu (1):
      perf trace: Fix race condition at the end of started workloads

Wang Nan (3):
      perf tools: Ignore .config-detected in .gitignore
      perf tools: Fix a problem when opening old perf.data with different byte order
      perf tools: Move libtraceevent dynamic list to separated LDFLAGS variable

 tools/perf/.gitignore          |  1 +
 tools/perf/Makefile.perf       |  8 ++--
 tools/perf/builtin-top.c       | 52 ++++++++++++++++++--------
 tools/perf/ui/browsers/hists.c |  2 +
 tools/perf/util/evlist.c       | 18 ++++++++-
 tools/perf/util/evlist.h       |  2 +
 tools/perf/util/probe-event.c  | 83 +++++++++++++++++++++++++++++++-----------
 tools/perf/util/session.c      | 50 ++++++++++++++++++-------
 8 files changed, 160 insertions(+), 56 deletions(-)

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

* [PATCH 1/8] perf tools: Ignore .config-detected in .gitignore
  2015-06-17 21:22 [GIT PULL 0/8] perf/core improvements and fixes Arnaldo Carvalho de Melo
@ 2015-06-17 21:22 ` Arnaldo Carvalho de Melo
  2015-06-17 21:22 ` [PATCH 2/8] perf tools: Fix a problem when opening old perf.data with different byte order Arnaldo Carvalho de Melo
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-06-17 21:22 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Wang Nan, Jiri Olsa, Zefan Li, Arnaldo Carvalho de Melo

From: Wang Nan <wangnan0@huawei.com>

Commit fcfd6611fbccdbf2593bd949097a5c0e45cd96da ("tools build: Add
detected config support") dynamically creates .config-detected. Add it
to .gitignore.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Link: http://lkml.kernel.org/r/1434542358-5430-1-git-send-email-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/.gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/.gitignore b/tools/perf/.gitignore
index 812f904193e8..09db62ba5786 100644
--- a/tools/perf/.gitignore
+++ b/tools/perf/.gitignore
@@ -28,3 +28,4 @@ config.mak.autogen
 *-flex.*
 *.pyc
 *.pyo
+.config-detected
-- 
2.1.0


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

* [PATCH 2/8] perf tools: Fix a problem when opening old perf.data with different byte order
  2015-06-17 21:22 [GIT PULL 0/8] perf/core improvements and fixes Arnaldo Carvalho de Melo
  2015-06-17 21:22 ` [PATCH 1/8] perf tools: Ignore .config-detected in .gitignore Arnaldo Carvalho de Melo
@ 2015-06-17 21:22 ` Arnaldo Carvalho de Melo
  2015-06-17 21:22 ` [PATCH 3/8] perf tools: Move libtraceevent dynamic list to separated LDFLAGS variable Arnaldo Carvalho de Melo
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-06-17 21:22 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Wang Nan, Ingo Molnar, Masami Hiramatsu,
	Namhyung Kim, Peter Zijlstra, Zefan Li, pi3orama,
	Arnaldo Carvalho de Melo

From: Wang Nan <wangnan0@huawei.com>

Following error occurs when trying to use 'perf report' on x86_64 to
cross analysis a perf.data generated by an old perf on a big-endian
machine:

 # perf report
 *** Error in `/home/w00229757/perf': free(): invalid next size (fast): 0x00000000032c99f0 ***
 ======= Backtrace: =========
 /lib64/libc.so.6(+0x6eeef)[0x7ff6ff7e2eef]
 /lib64/libc.so.6(+0x78cae)[0x7ff6ff7eccae]
 /lib64/libc.so.6(+0x79987)[0x7ff6ff7ed987]
 /path/to/perf[0x4ac734]
 /path/to/perf[0x4ac829]
 /path/to/perf(perf_header__process_sections+0x129)[0x4ad2c9]
 /path/to/perf(perf_session__read_header+0x2e1)[0x4ad9e1]
 /path/to/perf(perf_session__new+0x168)[0x4bd458]
 /path/to/perf(cmd_report+0xfa0)[0x43eb70]
 /path/to/perf[0x47adc3]
 /path/to/perf(main+0x5f6)[0x42fd06]
 /lib64/libc.so.6(__libc_start_main+0xf5)[0x7ff6ff795bd5]
 /path/to/perf[0x42fe35]
 ======= Memory map: ========
 [SNIP]

The bug is in perf_event__attr_swap(). It swaps all fields in 'struct
perf_event_attr' without checking whether the swapped field exist or
not. In addition, in read_event_desc() allocs memory for attr according
to size read from perf.data.

Therefore, if the perf.data is collected by an old perf (without
aux_watermark, for example), when perf_event__attr_swap() swaping
attr->aux_watermark it destroy malloc's metadata.

This patch introduces boundary checking in perf_event__attr_swap(). It
adds macros bswap_field_64 and bswap_field_32 into
perf_event__attr_swap() to make it only swap exist fields.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1434534999-85347-1-git-send-email-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/session.c | 50 ++++++++++++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index f31e024ddf7d..e1cd17c2afab 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -517,20 +517,42 @@ void perf_event__attr_swap(struct perf_event_attr *attr)
 {
 	attr->type		= bswap_32(attr->type);
 	attr->size		= bswap_32(attr->size);
-	attr->config		= bswap_64(attr->config);
-	attr->sample_period	= bswap_64(attr->sample_period);
-	attr->sample_type	= bswap_64(attr->sample_type);
-	attr->read_format	= bswap_64(attr->read_format);
-	attr->wakeup_events	= bswap_32(attr->wakeup_events);
-	attr->bp_type		= bswap_32(attr->bp_type);
-	attr->bp_addr		= bswap_64(attr->bp_addr);
-	attr->bp_len		= bswap_64(attr->bp_len);
-	attr->branch_sample_type = bswap_64(attr->branch_sample_type);
-	attr->sample_regs_user	 = bswap_64(attr->sample_regs_user);
-	attr->sample_stack_user  = bswap_32(attr->sample_stack_user);
-	attr->aux_watermark	 = bswap_32(attr->aux_watermark);
-
-	swap_bitfield((u8 *) (&attr->read_format + 1), sizeof(u64));
+
+#define bswap_safe(f, n) 					\
+	(attr->size > (offsetof(struct perf_event_attr, f) + 	\
+		       sizeof(attr->f) * (n)))
+#define bswap_field(f, sz) 			\
+do { 						\
+	if (bswap_safe(f, 0))			\
+		attr->f = bswap_##sz(attr->f);	\
+} while(0)
+#define bswap_field_32(f) bswap_field(f, 32)
+#define bswap_field_64(f) bswap_field(f, 64)
+
+	bswap_field_64(config);
+	bswap_field_64(sample_period);
+	bswap_field_64(sample_type);
+	bswap_field_64(read_format);
+	bswap_field_32(wakeup_events);
+	bswap_field_32(bp_type);
+	bswap_field_64(bp_addr);
+	bswap_field_64(bp_len);
+	bswap_field_64(branch_sample_type);
+	bswap_field_64(sample_regs_user);
+	bswap_field_32(sample_stack_user);
+	bswap_field_32(aux_watermark);
+
+	/*
+	 * After read_format are bitfields. Check read_format because
+	 * we are unable to use offsetof on bitfield.
+	 */
+	if (bswap_safe(read_format, 1))
+		swap_bitfield((u8 *) (&attr->read_format + 1),
+			      sizeof(u64));
+#undef bswap_field_64
+#undef bswap_field_32
+#undef bswap_field
+#undef bswap_safe
 }
 
 static void perf_event__hdr_attr_swap(union perf_event *event,
-- 
2.1.0


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

* [PATCH 3/8] perf tools: Move libtraceevent dynamic list to separated LDFLAGS variable
  2015-06-17 21:22 [GIT PULL 0/8] perf/core improvements and fixes Arnaldo Carvalho de Melo
  2015-06-17 21:22 ` [PATCH 1/8] perf tools: Ignore .config-detected in .gitignore Arnaldo Carvalho de Melo
  2015-06-17 21:22 ` [PATCH 2/8] perf tools: Fix a problem when opening old perf.data with different byte order Arnaldo Carvalho de Melo
@ 2015-06-17 21:22 ` Arnaldo Carvalho de Melo
  2015-06-17 21:22 ` [PATCH 4/8] perf probe: Show usage even if the last event is skipped Arnaldo Carvalho de Melo
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-06-17 21:22 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Wang Nan, He Kuang, Masami Hiramatsu, Namhyung Kim,
	Peter Zijlstra, Zefan Li, pi3orama, Arnaldo Carvalho de Melo

From: Wang Nan <wangnan0@huawei.com>

Commit e3d09ec8126fe2c9a3ade661e2126e215ca27a80 ("tools lib traceevent:
Export dynamic symbols used by traceevent plugins") adds libtraceevent
dynamic list directly into LDFLAGS, which makes all targets depend on
that list through LDFLAGS.

This is not good since some of targets like libgtk.so doesn't use plugin
at all, but require the existance of that list because of linker
options.

This patch isolates the -Xlink option into LIBTRACEEVENT_DYNAMIC_LIST_LDFLAGS,
makes only perf and perf.so use it.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: He Kuang <hekuang@huawei.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1434552389-89144-1-git-send-email-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Makefile.perf | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 374378322db9..1af0cfeb7a57 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -174,7 +174,7 @@ LIBTRACEEVENT = $(TE_PATH)libtraceevent.a
 export LIBTRACEEVENT
 
 LIBTRACEEVENT_DYNAMIC_LIST = $(TE_PATH)libtraceevent-dynamic-list
-LDFLAGS += -Xlinker --dynamic-list=$(LIBTRACEEVENT_DYNAMIC_LIST)
+LIBTRACEEVENT_DYNAMIC_LIST_LDFLAGS = -Xlinker --dynamic-list=$(LIBTRACEEVENT_DYNAMIC_LIST)
 
 LIBAPI = $(LIB_PATH)libapi.a
 export LIBAPI
@@ -191,7 +191,8 @@ PYTHON_EXT_SRCS := $(shell grep -v ^\# util/python-ext-sources)
 PYTHON_EXT_DEPS := util/python-ext-sources util/setup.py $(LIBTRACEEVENT) $(LIBAPI)
 
 $(OUTPUT)python/perf.so: $(PYTHON_EXT_SRCS) $(PYTHON_EXT_DEPS) $(LIBTRACEEVENT_DYNAMIC_LIST)
-	$(QUIET_GEN)CFLAGS='$(CFLAGS)' $(PYTHON_WORD) util/setup.py \
+	$(QUIET_GEN)CFLAGS='$(CFLAGS)' LDFLAGS='$(LDFLAGS) $(LIBTRACEEVENT_DYNAMIC_LIST_LDFLAGS)' \
+	  $(PYTHON_WORD) util/setup.py \
 	  --quiet build_ext; \
 	mkdir -p $(OUTPUT)python && \
 	cp $(PYTHON_EXTBUILD_LIB)perf.so $(OUTPUT)python/
@@ -282,7 +283,8 @@ $(PERF_IN): $(OUTPUT)PERF-VERSION-FILE $(OUTPUT)common-cmds.h FORCE
 	$(Q)$(MAKE) $(build)=perf
 
 $(OUTPUT)perf: $(PERFLIBS) $(PERF_IN) $(LIBTRACEEVENT_DYNAMIC_LIST)
-	$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(PERF_IN) $(LIBS) -o $@
+	$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(LIBTRACEEVENT_DYNAMIC_LIST_LDFLAGS) \
+		$(PERF_IN) $(LIBS) -o $@
 
 $(GTK_IN): FORCE
 	$(Q)$(MAKE) $(build)=gtk
-- 
2.1.0


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

* [PATCH 4/8] perf probe: Show usage even if the last event is skipped
  2015-06-17 21:22 [GIT PULL 0/8] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (2 preceding siblings ...)
  2015-06-17 21:22 ` [PATCH 3/8] perf tools: Move libtraceevent dynamic list to separated LDFLAGS variable Arnaldo Carvalho de Melo
@ 2015-06-17 21:22 ` Arnaldo Carvalho de Melo
  2015-06-17 21:22 ` [PATCH 5/8] perf probe: Speed up perf probe --list by caching debuginfo Arnaldo Carvalho de Melo
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-06-17 21:22 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Masami Hiramatsu, David Ahern, Jiri Olsa,
	Namhyung Kim, Naohiro Aota, Peter Zijlstra,
	Arnaldo Carvalho de Melo

From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

When the last part of converted events are blacklisted or out-of-text,
those are skipped and perf probe doesn't show usage examples.  This
fixes it to show the example even if the last part of event list is
skipped.

E.g. without this patch, events are added, but suddenly end:

  # perf probe vfs_*
  vfs_caches_init_early is out of .text, skip it.
  vfs_caches_init is out of .text, skip it.
  Added new events:
    probe:vfs_fallocate  (on vfs_*)
    probe:vfs_open       (on vfs_*)
  ...
    probe:vfs_dentry_acceptable (on vfs_*)
    probe:vfs_load_quota_inode (on vfs_*)
  #

With this fix:

  # perf probe vfs_*
  vfs_caches_init_early is out of .text, skip it.
  vfs_caches_init is out of .text, skip it.
  Added new events:
    probe:vfs_fallocate  (on vfs_*)
  ...
    probe:vfs_load_quota_inode (on vfs_*)

  You can now use it in all perf tools, such as:

	perf record -e probe:vfs_load_quota_inode -aR sleep 1

Note that this can be reproduced ONLY IF the vfs_caches_init* is the
last part of matched symbol list. I've checked this happens on
"3.19.0-generic #18-Ubuntu" kernel binary.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Naohiro Aota <naota@elisp.net>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20150616115057.19906.5502.stgit@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c | 35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 85c8207c25cc..65a1c8252270 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2157,7 +2157,8 @@ static bool kprobe_blacklist__listed(unsigned long address)
 	return !!kprobe_blacklist__find_by_address(&kprobe_blacklist, address);
 }
 
-static int perf_probe_event__sprintf(struct perf_probe_event *pev,
+static int perf_probe_event__sprintf(const char *group, const char *event,
+				     struct perf_probe_event *pev,
 				     const char *module,
 				     struct strbuf *result)
 {
@@ -2170,7 +2171,7 @@ static int perf_probe_event__sprintf(struct perf_probe_event *pev,
 	if (!place)
 		return -EINVAL;
 
-	ret = e_snprintf(buf, 128, "%s:%s", pev->group, pev->event);
+	ret = e_snprintf(buf, 128, "%s:%s", group, event);
 	if (ret < 0)
 		goto out;
 
@@ -2195,13 +2196,14 @@ out:
 }
 
 /* Show an event */
-static int show_perf_probe_event(struct perf_probe_event *pev,
+static int show_perf_probe_event(const char *group, const char *event,
+				 struct perf_probe_event *pev,
 				 const char *module, bool use_stdout)
 {
 	struct strbuf buf = STRBUF_INIT;
 	int ret;
 
-	ret = perf_probe_event__sprintf(pev, module, &buf);
+	ret = perf_probe_event__sprintf(group, event, pev, module, &buf);
 	if (ret >= 0) {
 		if (use_stdout)
 			printf("%s\n", buf.buf);
@@ -2253,7 +2255,8 @@ static int __show_perf_probe_events(int fd, bool is_kprobe,
 								is_kprobe);
 			if (ret < 0)
 				goto next;
-			ret = show_perf_probe_event(&pev, tev.point.module,
+			ret = show_perf_probe_event(pev.group, pev.event,
+						    &pev, tev.point.module,
 						    true);
 		}
 next:
@@ -2438,7 +2441,7 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 	int i, fd, ret;
 	struct probe_trace_event *tev = NULL;
 	char buf[64];
-	const char *event, *group;
+	const char *event = NULL, *group = NULL;
 	struct strlist *namelist;
 	bool safename;
 
@@ -2500,15 +2503,12 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 		/* Add added event name to namelist */
 		strlist__add(namelist, event);
 
-		/* Trick here - save current event/group */
-		event = pev->event;
-		group = pev->group;
-		pev->event = tev->event;
-		pev->group = tev->group;
-		show_perf_probe_event(pev, tev->point.module, false);
-		/* Trick here - restore current event/group */
-		pev->event = (char *)event;
-		pev->group = (char *)group;
+		/* We use tev's name for showing new events */
+		show_perf_probe_event(tev->group, tev->event, pev,
+				      tev->point.module, false);
+		/* Save the last valid name */
+		event = tev->event;
+		group = tev->group;
 
 		/*
 		 * Probes after the first probe which comes from same
@@ -2522,11 +2522,10 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 		warn_uprobe_event_compat(tev);
 
 	/* Note that it is possible to skip all events because of blacklist */
-	if (ret >= 0 && tev->event) {
+	if (ret >= 0 && event) {
 		/* Show how to use the event. */
 		pr_info("\nYou can now use it in all perf tools, such as:\n\n");
-		pr_info("\tperf record -e %s:%s -aR sleep 1\n\n", tev->group,
-			 tev->event);
+		pr_info("\tperf record -e %s:%s -aR sleep 1\n\n", group, event);
 	}
 
 	strlist__delete(namelist);
-- 
2.1.0


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

* [PATCH 5/8] perf probe: Speed up perf probe --list by caching debuginfo
  2015-06-17 21:22 [GIT PULL 0/8] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (3 preceding siblings ...)
  2015-06-17 21:22 ` [PATCH 4/8] perf probe: Show usage even if the last event is skipped Arnaldo Carvalho de Melo
@ 2015-06-17 21:22 ` Arnaldo Carvalho de Melo
  2015-06-17 21:22 ` [PATCH 6/8] perf trace: Fix race condition at the end of started workloads Arnaldo Carvalho de Melo
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-06-17 21:22 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Masami Hiramatsu, David Ahern, Jiri Olsa,
	Namhyung Kim, Naohiro Aota, Peter Zijlstra,
	Arnaldo Carvalho de Melo

From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

Speed up the "perf probe --list" by caching the last used debuginfo.
perf probe --list always open and load debuginfo for each entry of probe
list. This takes very a long time.

E.g. with vfs_* events (total 96 probes)

  [root@localhost perf]# time  ./perf probe -l &> /dev/null

  real    0m25.376s
  user    0m24.381s
  sys     0m1.012s

To solve this issue, this adds debuginfo_cache to cache the
last used debuginfo on memory.

With this fix, the perf-probe --list significantly improves
its speed.

  [root@localhost perf]#  time  ./perf probe -l &> /dev/null

  real    0m0.161s
  user    0m0.136s
  sys     0m0.025s

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Naohiro Aota <naota@elisp.net>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20150617145854.19715.15314.stgit@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c | 48 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 65a1c8252270..076527b639bd 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -429,6 +429,41 @@ static struct debuginfo *open_debuginfo(const char *module, bool silent)
 	return ret;
 }
 
+/* For caching the last debuginfo */
+static struct debuginfo *debuginfo_cache;
+static char *debuginfo_cache_path;
+
+static struct debuginfo *debuginfo_cache__open(const char *module, bool silent)
+{
+	if ((debuginfo_cache_path && !strcmp(debuginfo_cache_path, module)) ||
+	    (!debuginfo_cache_path && !module && debuginfo_cache))
+		goto out;
+
+	/* Copy module path */
+	free(debuginfo_cache_path);
+	if (module) {
+		debuginfo_cache_path = strdup(module);
+		if (!debuginfo_cache_path) {
+			debuginfo__delete(debuginfo_cache);
+			debuginfo_cache = NULL;
+			goto out;
+		}
+	}
+
+	debuginfo_cache = open_debuginfo(module, silent);
+	if (!debuginfo_cache)
+		zfree(&debuginfo_cache_path);
+out:
+	return debuginfo_cache;
+}
+
+static void debuginfo_cache__exit(void)
+{
+	debuginfo__delete(debuginfo_cache);
+	debuginfo_cache = NULL;
+	zfree(&debuginfo_cache_path);
+}
+
 
 static int get_text_start_address(const char *exec, unsigned long *address)
 {
@@ -490,12 +525,11 @@ static int find_perf_probe_point_from_dwarf(struct probe_trace_point *tp,
 	pr_debug("try to find information at %" PRIx64 " in %s\n", addr,
 		 tp->module ? : "kernel");
 
-	dinfo = open_debuginfo(tp->module, verbose == 0);
-	if (dinfo) {
+	dinfo = debuginfo_cache__open(tp->module, verbose == 0);
+	if (dinfo)
 		ret = debuginfo__find_probe_point(dinfo,
 						 (unsigned long)addr, pp);
-		debuginfo__delete(dinfo);
-	} else
+	else
 		ret = -ENOENT;
 
 	if (ret > 0) {
@@ -930,6 +964,10 @@ out:
 
 #else	/* !HAVE_DWARF_SUPPORT */
 
+static void debuginfo_cache__exit(void)
+{
+}
+
 static int
 find_perf_probe_point_from_dwarf(struct probe_trace_point *tp __maybe_unused,
 				 struct perf_probe_point *pp __maybe_unused,
@@ -2266,6 +2304,8 @@ next:
 			break;
 	}
 	strlist__delete(rawlist);
+	/* Cleanup cached debuginfo if needed */
+	debuginfo_cache__exit();
 
 	return ret;
 }
-- 
2.1.0


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

* [PATCH 6/8] perf trace: Fix race condition at the end of started workloads
  2015-06-17 21:22 [GIT PULL 0/8] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (4 preceding siblings ...)
  2015-06-17 21:22 ` [PATCH 5/8] perf probe: Speed up perf probe --list by caching debuginfo Arnaldo Carvalho de Melo
@ 2015-06-17 21:22 ` Arnaldo Carvalho de Melo
  2015-06-17 21:22 ` [PATCH 7/8] perf evlist: Add toggle_enable() method Arnaldo Carvalho de Melo
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-06-17 21:22 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Sukadev Bhattiprolu, Jiri Olsa, Li Zhang,
	Arnaldo Carvalho de Melo

From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>

I get following crash on multiple systems and across several releases
(at least since v3.18).

	Core was generated by `/tmp/perf trace sleep 0.2 '.
	Program terminated with signal SIGSEGV, Segmentation fault.
	#0  perf_mmap__read_head (mm=0x3fff9bf30070) at util/evlist.h:195
	195		u64 head = ACCESS_ONCE(pc->data_head);
	(gdb) bt
	#0  perf_mmap__read_head (mm=0x3fff9bf30070) at util/evlist.h:195
	#1  perf_evlist__mmap_read (evlist=0x10027f11910, idx=<optimized out>)
	    at util/evlist.c:637
	#2  0x000000001003ce4c in trace__run (argv=<optimized out>,
	    argc=<optimized out>, trace=0x3fffd7b28288) at builtin-trace.c:2259
	#3  cmd_trace (argc=<optimized out>, argv=<optimized out>,
	    prefix=<optimized out>) at builtin-trace.c:2799
	#4  0x00000000100657b8 in run_builtin (p=0x10176798 <commands+480>, argc=3,
	    argv=0x3fffd7b2b550) at perf.c:370
	#5  0x00000000100063e8 in handle_internal_command (argv=0x3fffd7b2b550, argc=3)
	    at perf.c:429
	#6  run_argv (argv=0x3fffd7b2af70, argcp=0x3fffd7b2af7c) at perf.c:473
	#7  main (argc=3, argv=0x3fffd7b2b550) at perf.c:588

The problem seems to be a race condition, when the application has just
exited.  Some/all fds associated with the perf-events (tracepoints) go
into a POLLHUP/ POLLERR state and the mmap region associated with those
events are unmapped (in perf_evlist__filter_pollfd()).

But we go back and do a perf_evlist__mmap_read() which assumes that the
mmaps are still valid and we hit the crash.

If the mapping for an event is released, its refcnt is 0 (and ->base
is NULL), so ensure we have non-zero refcount before accessing the map.

Note that perf-record has a similar logic but unlike perf-trace, the
record__mmap_read_all() checks the evlist->mmap[i].base before accessing
the map.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Li Zhang <zhlcindy@linux.vnet.ibm.com>
Link: http://lkml.kernel.org/r/20150612060003.GA19913@us.ibm.com
[ Fixed it up to use atomic_read() ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evlist.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index dc1dc2c181ef..6b58a47a79ec 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -634,11 +634,18 @@ static struct perf_evsel *perf_evlist__event2evsel(struct perf_evlist *evlist,
 union perf_event *perf_evlist__mmap_read(struct perf_evlist *evlist, int idx)
 {
 	struct perf_mmap *md = &evlist->mmap[idx];
-	u64 head = perf_mmap__read_head(md);
+	u64 head;
 	u64 old = md->prev;
 	unsigned char *data = md->base + page_size;
 	union perf_event *event = NULL;
 
+	/*
+	 * Check if event was unmapped due to a POLLHUP/POLLERR.
+	 */
+	if (!atomic_read(&md->refcnt))
+		return NULL;
+
+	head = perf_mmap__read_head(md);
 	if (evlist->overwrite) {
 		/*
 		 * If we're further behind than half the buffer, there's a chance
-- 
2.1.0


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

* [PATCH 7/8] perf evlist: Add toggle_enable() method
  2015-06-17 21:22 [GIT PULL 0/8] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (5 preceding siblings ...)
  2015-06-17 21:22 ` [PATCH 6/8] perf trace: Fix race condition at the end of started workloads Arnaldo Carvalho de Melo
@ 2015-06-17 21:22 ` Arnaldo Carvalho de Melo
  2015-06-17 21:22 ` [PATCH 8/8] perf top: Allow disabling/enabling events dynamicly Arnaldo Carvalho de Melo
  2015-06-18  7:40 ` [GIT PULL 0/8] perf/core improvements and fixes Ingo Molnar
  8 siblings, 0 replies; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-06-17 21:22 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>

For an upcoming feature in 'perf top' we will have a hotkey to
enable/disable events, so remember if the events in the list are
enabled or disabled and allows toggling this state using a new
method.

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-64c4jvdl5feg2zhimxvokqka@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evlist.c | 9 +++++++++
 tools/perf/util/evlist.h | 2 ++
 2 files changed, 11 insertions(+)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 6b58a47a79ec..8366511b45f8 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -297,6 +297,8 @@ void perf_evlist__disable(struct perf_evlist *evlist)
 				      PERF_EVENT_IOC_DISABLE, 0);
 		}
 	}
+
+	evlist->enabled = false;
 }
 
 void perf_evlist__enable(struct perf_evlist *evlist)
@@ -316,6 +318,13 @@ void perf_evlist__enable(struct perf_evlist *evlist)
 				      PERF_EVENT_IOC_ENABLE, 0);
 		}
 	}
+
+	evlist->enabled = true;
+}
+
+void perf_evlist__toggle_enable(struct perf_evlist *evlist)
+{
+	(evlist->enabled ? perf_evlist__disable : perf_evlist__enable)(evlist);
 }
 
 int perf_evlist__disable_event(struct perf_evlist *evlist,
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 955bf31b7dd3..a8489b9d2812 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -41,6 +41,7 @@ struct perf_evlist {
 	int		 nr_groups;
 	int		 nr_mmaps;
 	bool		 overwrite;
+	bool		 enabled;
 	size_t		 mmap_len;
 	int		 id_pos;
 	int		 is_pos;
@@ -139,6 +140,7 @@ void perf_evlist__munmap(struct perf_evlist *evlist);
 
 void perf_evlist__disable(struct perf_evlist *evlist);
 void perf_evlist__enable(struct perf_evlist *evlist);
+void perf_evlist__toggle_enable(struct perf_evlist *evlist);
 
 int perf_evlist__disable_event(struct perf_evlist *evlist,
 			       struct perf_evsel *evsel);
-- 
2.1.0


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

* [PATCH 8/8] perf top: Allow disabling/enabling events dynamicly
  2015-06-17 21:22 [GIT PULL 0/8] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (6 preceding siblings ...)
  2015-06-17 21:22 ` [PATCH 7/8] perf evlist: Add toggle_enable() method Arnaldo Carvalho de Melo
@ 2015-06-17 21:22 ` Arnaldo Carvalho de Melo
  2015-06-18  7:40 ` [GIT PULL 0/8] perf/core improvements and fixes Ingo Molnar
  8 siblings, 0 replies; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-06-17 21:22 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>

Now it is possible to press CTRL+z at anytime and that will disable the
events being monitored, essentially turning 'top' into 'report', with
pressing CTRL+z again making it enable the events again, returning to
the 'top' behaviour, i.e. dynamic + decaying of older samples.

One may want, for instance, play with:

    -d, --delay <n>       number of seconds to delay between refreshes

and:

    -z, --zero            zero history across updates

Plus CTRL+z to see only the events since last zeroing, etc.

Suggested-by: Ingo Molnar <mingo@kernel.org>
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-zq7tnh5462blt2yda0bcxh5b@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-top.c       | 52 +++++++++++++++++++++++++++++-------------
 tools/perf/ui/browsers/hists.c |  2 ++
 2 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 6b987424d015..72d8a7ae5986 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -235,10 +235,13 @@ static void perf_top__show_details(struct perf_top *top)
 
 	more = symbol__annotate_printf(symbol, he->ms.map, top->sym_evsel,
 				       0, top->sym_pcnt_filter, top->print_entries, 4);
-	if (top->zero)
-		symbol__annotate_zero_histogram(symbol, top->sym_evsel->idx);
-	else
-		symbol__annotate_decay_histogram(symbol, top->sym_evsel->idx);
+
+	if (top->evlist->enabled) {
+		if (top->zero)
+			symbol__annotate_zero_histogram(symbol, top->sym_evsel->idx);
+		else
+			symbol__annotate_decay_histogram(symbol, top->sym_evsel->idx);
+	}
 	if (more != 0)
 		printf("%d lines not displayed, maybe increase display entries [e]\n", more);
 out_unlock:
@@ -276,11 +279,13 @@ static void perf_top__print_sym_table(struct perf_top *top)
 		return;
 	}
 
-	if (top->zero) {
-		hists__delete_entries(hists);
-	} else {
-		hists__decay_entries(hists, top->hide_user_symbols,
-				     top->hide_kernel_symbols);
+	if (top->evlist->enabled) {
+		if (top->zero) {
+			hists__delete_entries(hists);
+		} else {
+			hists__decay_entries(hists, top->hide_user_symbols,
+					     top->hide_kernel_symbols);
+		}
 	}
 
 	hists__collapse_resort(hists, NULL);
@@ -545,11 +550,13 @@ static void perf_top__sort_new_samples(void *arg)
 
 	hists = evsel__hists(t->sym_evsel);
 
-	if (t->zero) {
-		hists__delete_entries(hists);
-	} else {
-		hists__decay_entries(hists, t->hide_user_symbols,
-				     t->hide_kernel_symbols);
+	if (t->evlist->enabled) {
+		if (t->zero) {
+			hists__delete_entries(hists);
+		} else {
+			hists__decay_entries(hists, t->hide_user_symbols,
+					     t->hide_kernel_symbols);
+		}
 	}
 
 	hists__collapse_resort(hists, NULL);
@@ -579,8 +586,21 @@ static void *display_thread_tui(void *arg)
 		hists->uid_filter_str = top->record_opts.target.uid_str;
 	}
 
-	perf_evlist__tui_browse_hists(top->evlist, help, &hbt, top->min_percent,
-				      &top->session->header.env);
+	while (true)  {
+		int key = perf_evlist__tui_browse_hists(top->evlist, help, &hbt,
+							top->min_percent,
+							&top->session->header.env);
+
+		if (key != CTRL('z'))
+			break;
+
+		perf_evlist__toggle_enable(top->evlist);
+		/*
+		 * No need to refresh, resort/decay histogram entries
+		 * if we are not collecting samples:
+		 */
+		hbt.refresh = top->evlist->enabled ? top->delay_secs : 0;
+	}
 
 	done = 1;
 	return NULL;
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index e64893f2fd7f..8f7c4d49d327 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1736,6 +1736,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 	"t             Zoom into current Thread\n"
 	"V             Verbose (DSO names in callchains, etc)\n"
 	"z             Toggle zeroing of samples\n"
+	"CTRL+z        Enable/Disable events\n"
 	"/             Filter symbol by name";
 
 	if (browser == NULL)
@@ -1900,6 +1901,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 			/* Fall thru */
 		case 'q':
 		case CTRL('c'):
+		case CTRL('z'):
 			goto out_free_stack;
 		default:
 			continue;
-- 
2.1.0


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

* Re: [GIT PULL 0/8] perf/core improvements and fixes
  2015-06-17 21:22 [GIT PULL 0/8] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (7 preceding siblings ...)
  2015-06-17 21:22 ` [PATCH 8/8] perf top: Allow disabling/enabling events dynamicly Arnaldo Carvalho de Melo
@ 2015-06-18  7:40 ` Ingo Molnar
  2015-06-18 20:18   ` [RFC] hotkey for disabling/enabling events in 'perf top' TUI was " Arnaldo Carvalho de Melo
  8 siblings, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2015-06-18  7:40 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, Li Zhang,
	Masami Hiramatsu, Namhyung Kim, Naohiro Aota, Peter Zijlstra,
	pi3orama, Stephane Eranian, Sukadev Bhattiprolu, Wang Nan,
	Zefan Li, Arnaldo Carvalho de Melo


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

> Hi Ingo,
> 
> 	Please consider pulling, this is on top of perf-core-for-mingo, that is
> still outstanding,
> 
> Thanks!
> 
> - Arnaldo
> 
> The following changes since commit b031220d520238075bd99513a420e65cf37866ad:
> 
>   perf probe: Fix to return error if no probe is added (2015-06-16 11:39:51 -0300)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-core-for-mingo-2
> 
> for you to fetch changes up to 5d484f99aed547e235f2229653c95392a1bc3692:
> 
>   perf top: Allow disabling/enabling events dynamicly (2015-06-17 16:50:52 -0300)
> 
> ----------------------------------------------------------------
> perf/core improvements and fixes:
> 
> User visible:
> 
> - Allow disabling/enabling events dynamicly in 'perf top':
>   a 'perf top' session can instantly become a 'perf report'
>   one, i.e. going from dynamic analysis to a static one,
>   returning to a dynamic one is possible, to toogle the
>   modes, just press CTRL+z. (Arnaldo Carvalho de Melo)

Nice!! :-)

Btw., it would be nice if the status line carried information about whether 
collection is 'frozen' or running, at a glance. A hint might also suggest how to 
unfreeze the session - in case someone pressed Ctrl-Z to suspend the perf top 
session ...

Also, there's now a GUI inconsistency with perf report: which will now exit on 
Ctrl-Z. It should probably print a warning in the status line instead, that 
freezing/unfreezing only works in 'perf top'.

> 
> - Greatly speed up 'perf probe --list' by caching debuginfo
>   (Masami Hiramatsu)
> 
> - Fix 'perf trace' race condition at the end of started
>   workloads (Sukadev Bhattiprolu)
> 
> - Fix a problem when opening old perf.data with different
>   byte order (Wang Nan)
> 
> Infrastructure:
> 
> - Ignore .config-detected in .gitignore (Wang Nan)
> 
> - Move libtraceevent dynamic list to separated LDFLAGS
>   variable (Wang Nan)
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> ----------------------------------------------------------------
> Arnaldo Carvalho de Melo (2):
>       perf evlist: Add toggle_enable() method
>       perf top: Allow disabling/enabling events dynamicly
> 
> Masami Hiramatsu (2):
>       perf probe: Show usage even if the last event is skipped
>       perf probe: Speed up perf probe --list by caching debuginfo
> 
> Sukadev Bhattiprolu (1):
>       perf trace: Fix race condition at the end of started workloads
> 
> Wang Nan (3):
>       perf tools: Ignore .config-detected in .gitignore
>       perf tools: Fix a problem when opening old perf.data with different byte order
>       perf tools: Move libtraceevent dynamic list to separated LDFLAGS variable
> 
>  tools/perf/.gitignore          |  1 +
>  tools/perf/Makefile.perf       |  8 ++--
>  tools/perf/builtin-top.c       | 52 ++++++++++++++++++--------
>  tools/perf/ui/browsers/hists.c |  2 +
>  tools/perf/util/evlist.c       | 18 ++++++++-
>  tools/perf/util/evlist.h       |  2 +
>  tools/perf/util/probe-event.c  | 83 +++++++++++++++++++++++++++++++-----------
>  tools/perf/util/session.c      | 50 ++++++++++++++++++-------
>  8 files changed, 160 insertions(+), 56 deletions(-)

Pulled, thanks a lot Arnaldo!

	Ingo

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

* [RFC] hotkey for disabling/enabling events in 'perf top' TUI was Re: [GIT PULL 0/8] perf/core improvements and fixes
  2015-06-18  7:40 ` [GIT PULL 0/8] perf/core improvements and fixes Ingo Molnar
@ 2015-06-18 20:18   ` Arnaldo Carvalho de Melo
  2015-06-18 20:58     ` Ingo Molnar
  0 siblings, 1 reply; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-06-18 20:18 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Adrian Hunter, Borislav Petkov, David Ahern,
	Don Zickus, Frederic Weisbecker, He Kuang, Jiri Olsa, Li Zhang,
	Masami Hiramatsu, Namhyung Kim, Naohiro Aota, Peter Zijlstra,
	pi3orama, Stephane Eranian, Sukadev Bhattiprolu, Wang Nan,
	Zefan Li

Em Thu, Jun 18, 2015 at 09:40:10AM +0200, Ingo Molnar escreveu:
> * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > User visible:

> > - Allow disabling/enabling events dynamicly in 'perf top':
> >   a 'perf top' session can instantly become a 'perf report'
> >   one, i.e. going from dynamic analysis to a static one,
> >   returning to a dynamic one is possible, to toogle the
> >   modes, just press CTRL+z. (Arnaldo Carvalho de Melo)
 
> Nice!! :-)
 
> Btw., it would be nice if the status line carried information about whether 
> collection is 'frozen' or running, at a glance. A hint might also suggest how to 
> unfreeze the session - in case someone pressed Ctrl-Z to suspend the perf top 
> session ...

Right, and I think we better find other hotkey and make Ctrl+Z work like
with other tools, i.e. suspend:

[acme@zoo linux]$ mutt

[1]+  Stopped                 mutt
[acme@zoo linux]$ vim

[2]+  Stopped                 vim
[acme@zoo linux]$ 

The perf TUI should work like that as well...

Ideas?

We already have:

h/?/F1        Show this window
UP/DOWN/PGUP
PGDN/SPACE    Navigate
q/ESC/CTRL+C  Exit browser

For multiple event sessions:

TAB/UNTAB     Switch events

For symbolic views (--sort has sym):

->            Zoom into DSO/Threads & Annotate current symbol
<-            Zoom out
a             Annotate current symbol
C             Collapse all callchains
d             Zoom into current DSO
D             Show some developer debug info
E             Expand all callchains
F             Toggle percentage of filtered entries
H             Display column headers
i             Show header information
P             Print histograms to perf.hist.N
r             Run available scripts
s             Switch to another data file in PWD
t             Zoom into current Thread
V             Verbose (DSO names in callchains, etc)
z             Toggle zeroing of samples 
/             Filter symbol by name 
 
> Also, there's now a GUI inconsistency with perf report: which will now exit on 
> Ctrl-Z. It should probably print a warning in the status line instead, that 
> freezing/unfreezing only works in 'perf top'.

I'll fix that.

- Arnaldo

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

* Re: [RFC] hotkey for disabling/enabling events in 'perf top' TUI was Re: [GIT PULL 0/8] perf/core improvements and fixes
  2015-06-18 20:18   ` [RFC] hotkey for disabling/enabling events in 'perf top' TUI was " Arnaldo Carvalho de Melo
@ 2015-06-18 20:58     ` Ingo Molnar
  2015-06-18 21:39       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2015-06-18 20:58 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, Li Zhang,
	Masami Hiramatsu, Namhyung Kim, Naohiro Aota, Peter Zijlstra,
	pi3orama, Stephane Eranian, Sukadev Bhattiprolu, Wang Nan,
	Zefan Li


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

> Em Thu, Jun 18, 2015 at 09:40:10AM +0200, Ingo Molnar escreveu:
> > * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > User visible:
> 
> > > - Allow disabling/enabling events dynamicly in 'perf top':
> > >   a 'perf top' session can instantly become a 'perf report'
> > >   one, i.e. going from dynamic analysis to a static one,
> > >   returning to a dynamic one is possible, to toogle the
> > >   modes, just press CTRL+z. (Arnaldo Carvalho de Melo)
>  
> > Nice!! :-)
>  
> > Btw., it would be nice if the status line carried information about whether 
> > collection is 'frozen' or running, at a glance. A hint might also suggest how to 
> > unfreeze the session - in case someone pressed Ctrl-Z to suspend the perf top 
> > session ...
> 
> Right, and I think we better find other hotkey and make Ctrl+Z work like
> with other tools, i.e. suspend:
> 
> [acme@zoo linux]$ mutt
> 
> [1]+  Stopped                 mutt
> [acme@zoo linux]$ vim
> 
> [2]+  Stopped                 vim
> [acme@zoo linux]$ 
> 
> The perf TUI should work like that as well...
> 
> Ideas?
> 
> We already have:
> 
> h/?/F1        Show this window
> UP/DOWN/PGUP
> PGDN/SPACE    Navigate
> q/ESC/CTRL+C  Exit browser
> 
> For multiple event sessions:
> 
> TAB/UNTAB     Switch events
> 
> For symbolic views (--sort has sym):
> 
> ->            Zoom into DSO/Threads & Annotate current symbol
> <-            Zoom out
> a             Annotate current symbol
> C             Collapse all callchains
> d             Zoom into current DSO
> D             Show some developer debug info
> E             Expand all callchains
> F             Toggle percentage of filtered entries
> H             Display column headers
> i             Show header information
> P             Print histograms to perf.hist.N
> r             Run available scripts
> s             Switch to another data file in PWD
> t             Zoom into current Thread
> V             Verbose (DSO names in callchains, etc)
> z             Toggle zeroing of samples 
> /             Filter symbol by name 

Is 'f' (for 'freeze') still available?

Thanks,

	Ingo

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

* Re: [RFC] hotkey for disabling/enabling events in 'perf top' TUI was Re: [GIT PULL 0/8] perf/core improvements and fixes
  2015-06-18 20:58     ` Ingo Molnar
@ 2015-06-18 21:39       ` Arnaldo Carvalho de Melo
  2015-06-19  6:27         ` Ingo Molnar
  0 siblings, 1 reply; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-06-18 21:39 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Adrian Hunter, Borislav Petkov, David Ahern,
	Don Zickus, Frederic Weisbecker, He Kuang, Jiri Olsa, Li Zhang,
	Masami Hiramatsu, Namhyung Kim, Naohiro Aota, Peter Zijlstra,
	pi3orama, Stephane Eranian, Sukadev Bhattiprolu, Wang Nan,
	Zefan Li

Em Thu, Jun 18, 2015 at 10:58:13PM +0200, Ingo Molnar escreveu:
> * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > Btw., it would be nice if the status line carried information about whether 
> > > collection is 'frozen' or running, at a glance. A hint might also suggest how to 
> > > unfreeze the session - in case someone pressed Ctrl-Z to suspend the perf top 
> > > session ...
> > 
> > Right, and I think we better find other hotkey and make Ctrl+Z work like
> > with other tools, i.e. suspend:
> > 
> > [acme@zoo linux]$ mutt
> > 
> > [1]+  Stopped                 mutt
> > [acme@zoo linux]$ vim
> > 
> > [2]+  Stopped                 vim
> > [acme@zoo linux]$ 
> > 
> > The perf TUI should work like that as well...
> > 
> > Ideas?
> > 
> > We already have:
> > 
> > h/?/F1        Show this window
> > UP/DOWN/PGUP
<SNIP>
> > z             Toggle zeroing of samples 
> > /             Filter symbol by name 
> 
> Is 'f' (for 'freeze') still available?

It is available in both the 'report'/'top' (aka the "hists" browser) and
in the annotate browser, so I'll go with it, and leave CTRL+z alone,
then make it it suspend.

- Arnaldo

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

* Re: [RFC] hotkey for disabling/enabling events in 'perf top' TUI was Re: [GIT PULL 0/8] perf/core improvements and fixes
  2015-06-18 21:39       ` Arnaldo Carvalho de Melo
@ 2015-06-19  6:27         ` Ingo Molnar
  2015-06-19 20:07           ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2015-06-19  6:27 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, Li Zhang,
	Masami Hiramatsu, Namhyung Kim, Naohiro Aota, Peter Zijlstra,
	pi3orama, Stephane Eranian, Sukadev Bhattiprolu, Wang Nan,
	Zefan Li


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

> Em Thu, Jun 18, 2015 at 10:58:13PM +0200, Ingo Molnar escreveu:
> > * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > > Btw., it would be nice if the status line carried information about whether 
> > > > collection is 'frozen' or running, at a glance. A hint might also suggest how to 
> > > > unfreeze the session - in case someone pressed Ctrl-Z to suspend the perf top 
> > > > session ...
> > > 
> > > Right, and I think we better find other hotkey and make Ctrl+Z work like
> > > with other tools, i.e. suspend:
> > > 
> > > [acme@zoo linux]$ mutt
> > > 
> > > [1]+  Stopped                 mutt
> > > [acme@zoo linux]$ vim
> > > 
> > > [2]+  Stopped                 vim
> > > [acme@zoo linux]$ 
> > > 
> > > The perf TUI should work like that as well...
> > > 
> > > Ideas?
> > > 
> > > We already have:
> > > 
> > > h/?/F1        Show this window
> > > UP/DOWN/PGUP
> <SNIP>
> > > z             Toggle zeroing of samples 
> > > /             Filter symbol by name 
> > 
> > Is 'f' (for 'freeze') still available?
> 
> It is available in both the 'report'/'top' (aka the "hists" browser) and in the 
> annotate browser, so I'll go with it, and leave CTRL+z alone, then make it it 
> suspend.

Sounds good to me!

Thanks,

	Ingo

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

* Re: [RFC] hotkey for disabling/enabling events in 'perf top' TUI was Re: [GIT PULL 0/8] perf/core improvements and fixes
  2015-06-19  6:27         ` Ingo Molnar
@ 2015-06-19 20:07           ` Arnaldo Carvalho de Melo
  2015-06-19 23:05             ` Ingo Molnar
  0 siblings, 1 reply; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-06-19 20:07 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Adrian Hunter, Borislav Petkov, David Ahern,
	Don Zickus, Frederic Weisbecker, He Kuang, Jiri Olsa, Li Zhang,
	Masami Hiramatsu, Namhyung Kim, Naohiro Aota, Peter Zijlstra,
	pi3orama, Stephane Eranian, Sukadev Bhattiprolu, Wang Nan,
	Zefan Li

Em Fri, Jun 19, 2015 at 08:27:11AM +0200, Ingo Molnar escreveu:
> * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > Em Thu, Jun 18, 2015 at 10:58:13PM +0200, Ingo Molnar escreveu:
> > > * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > > z             Toggle zeroing of samples 
> > > > /             Filter symbol by name 

> > > Is 'f' (for 'freeze') still available?

> > It is available in both the 'report'/'top' (aka the "hists" browser) and in the 
> > annotate browser, so I'll go with it, and leave CTRL+z alone, then make it it 
> > suspend.

> Sounds good to me!

I'm fixing these issues now and a mildly crazy idea ocurred to me: now
we can go from 'top' to 'report' and back, but only if we start in
'perf top', but I think we could go from 'perf report' to 'perf top'
mode too, i.e. start with a perf.data file, then enable collecting more
samples that would then be added to the existing histograms, etc.

Unsure if this would be useful tho ;-) Its just that it may be easy to
do and would be another step into having it all integrated.

Anyway, back to work...

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

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

* Re: [RFC] hotkey for disabling/enabling events in 'perf top' TUI was Re: [GIT PULL 0/8] perf/core improvements and fixes
  2015-06-19 20:07           ` Arnaldo Carvalho de Melo
@ 2015-06-19 23:05             ` Ingo Molnar
  2015-06-22 14:52               ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2015-06-19 23:05 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, Li Zhang,
	Masami Hiramatsu, Namhyung Kim, Naohiro Aota, Peter Zijlstra,
	pi3orama, Stephane Eranian, Sukadev Bhattiprolu, Wang Nan,
	Zefan Li


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

> Em Fri, Jun 19, 2015 at 08:27:11AM +0200, Ingo Molnar escreveu:
> > * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > Em Thu, Jun 18, 2015 at 10:58:13PM +0200, Ingo Molnar escreveu:
> > > > * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > > > z             Toggle zeroing of samples 
> > > > > /             Filter symbol by name 
> 
> > > > Is 'f' (for 'freeze') still available?
> 
> > > It is available in both the 'report'/'top' (aka the "hists" browser) and in the 
> > > annotate browser, so I'll go with it, and leave CTRL+z alone, then make it it 
> > > suspend.
> 
> > Sounds good to me!
> 
> I'm fixing these issues now and a mildly crazy idea ocurred to me: now we can go 
> from 'top' to 'report' and back, but only if we start in 'perf top', but I think 
> we could go from 'perf report' to 'perf top' mode too, i.e. start with a 
> perf.data file, then enable collecting more samples that would then be added to 
> the existing histograms, etc.
> 
> Unsure if this would be useful tho ;-) Its just that it may be easy to do and 
> would be another step into having it all integrated.

So I think the following would be useful for perf report: if we recorded the 
precise command line used, in the perf.data.

So if someone types:

    perf record -e cache-misses make -j16 kernel

then we'd have the whole command line in the perf.data:

    "perf record -e cache-misses make -j16 kernel"

and if there was a hotkey to take new samples, using the exact same workload.

This is non-trivial though.

Going from 'perf report' to 'perf top' would be intuitive if the 'perf record' 
before was 'perf top' alike, for example:

    perf record -a sleep 10

or:

    perf record -a
    <Ctrl-C>

in that case going to 'perf top' is a natural extension of the profiling session.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

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

* Re: [RFC] hotkey for disabling/enabling events in 'perf top' TUI was Re: [GIT PULL 0/8] perf/core improvements and fixes
  2015-06-19 23:05             ` Ingo Molnar
@ 2015-06-22 14:52               ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 17+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-06-22 14:52 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Adrian Hunter, Borislav Petkov, David Ahern,
	Don Zickus, Frederic Weisbecker, He Kuang, Jiri Olsa, Li Zhang,
	Masami Hiramatsu, Namhyung Kim, Naohiro Aota, Peter Zijlstra,
	pi3orama, Stephane Eranian, Sukadev Bhattiprolu, Wang Nan,
	Zefan Li

Em Sat, Jun 20, 2015 at 01:05:40AM +0200, Ingo Molnar escreveu:
> * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > Em Fri, Jun 19, 2015 at 08:27:11AM +0200, Ingo Molnar escreveu:
> > > * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > > Em Thu, Jun 18, 2015 at 10:58:13PM +0200, Ingo Molnar escreveu:
> > > > > * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > > > > z             Toggle zeroing of samples 
> > > > > > /             Filter symbol by name 

> > > > > Is 'f' (for 'freeze') still available?

> > > > It is available in both the 'report'/'top' (aka the "hists" browser) and in the 
> > > > annotate browser, so I'll go with it, and leave CTRL+z alone, then make it it 
> > > > suspend.

> > > Sounds good to me!

> > I'm fixing these issues now and a mildly crazy idea ocurred to me: now we can go 
> > from 'top' to 'report' and back, but only if we start in 'perf top', but I think 
> > we could go from 'perf report' to 'perf top' mode too, i.e. start with a 
> > perf.data file, then enable collecting more samples that would then be added to 
> > the existing histograms, etc.
> > 
> > Unsure if this would be useful tho ;-) Its just that it may be easy to do and 
> > would be another step into having it all integrated.
> 
> So I think the following would be useful for perf report: if we recorded the 
> precise command line used, in the perf.data.
> 
> So if someone types:
> 
>     perf record -e cache-misses make -j16 kernel
> 
> then we'd have the whole command line in the perf.data:
> 
>     "perf record -e cache-misses make -j16 kernel"
> 
> and if there was a hotkey to take new samples, using the exact same workload.
> 
> This is non-trivial though.

Right, can be tricky, but we could start with the low hanging fruit:
system wide sessions, then to the other more focussed "live targets",
i.e. existing pids (thread "families"), tids (single threads), CPUs,
cgroups, etc.

Easier because we don't have to recreate a workload, where we may need
to be at a particular cwd, etc.
 
> Going from 'perf report' to 'perf top' would be intuitive if the 'perf record' 
> before was 'perf top' alike, for example:
> 
>     perf record -a sleep 10
> 
> or:
> 
>     perf record -a
>     <Ctrl-C>
> 
> in that case going to 'perf top' is a natural extension of the profiling session.

Agreed, wrote the low hanging fruit part before reading the rest of the
message, obviously ;-)

I.e. ultimately what I would like to achieve would be to merge
builtin-top.c with buildin-report.c and make just the tool name decide
if it starts dynamicly or statically.

At some point 'record' would get merged too, and fully integrated in the
workflow, i.e. one could have at least the following modes:

- record: do nothing more than store the ring buffer in a safe place for
          later processing, be fast at that!

- top: do not record anything, process it straight away and put it into
       histogram buckets according to --sort

- report: do not record anything, process something recorded previously,
          according to --sort

- record + top: do both of these, allow stopping both and looking at
                the last session, or previous ones, allow creating
                slices, etc.

Annotate is already integrated in all this, you can go from either
dynamic or static to annotation and it will show the percentages/periods
per line both staticly and dynamicly.

For slicing, 'probe' integration will come in handy, but the plate is
full already for a while 8-)

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

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

end of thread, other threads:[~2015-06-22 14:53 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-17 21:22 [GIT PULL 0/8] perf/core improvements and fixes Arnaldo Carvalho de Melo
2015-06-17 21:22 ` [PATCH 1/8] perf tools: Ignore .config-detected in .gitignore Arnaldo Carvalho de Melo
2015-06-17 21:22 ` [PATCH 2/8] perf tools: Fix a problem when opening old perf.data with different byte order Arnaldo Carvalho de Melo
2015-06-17 21:22 ` [PATCH 3/8] perf tools: Move libtraceevent dynamic list to separated LDFLAGS variable Arnaldo Carvalho de Melo
2015-06-17 21:22 ` [PATCH 4/8] perf probe: Show usage even if the last event is skipped Arnaldo Carvalho de Melo
2015-06-17 21:22 ` [PATCH 5/8] perf probe: Speed up perf probe --list by caching debuginfo Arnaldo Carvalho de Melo
2015-06-17 21:22 ` [PATCH 6/8] perf trace: Fix race condition at the end of started workloads Arnaldo Carvalho de Melo
2015-06-17 21:22 ` [PATCH 7/8] perf evlist: Add toggle_enable() method Arnaldo Carvalho de Melo
2015-06-17 21:22 ` [PATCH 8/8] perf top: Allow disabling/enabling events dynamicly Arnaldo Carvalho de Melo
2015-06-18  7:40 ` [GIT PULL 0/8] perf/core improvements and fixes Ingo Molnar
2015-06-18 20:18   ` [RFC] hotkey for disabling/enabling events in 'perf top' TUI was " Arnaldo Carvalho de Melo
2015-06-18 20:58     ` Ingo Molnar
2015-06-18 21:39       ` Arnaldo Carvalho de Melo
2015-06-19  6:27         ` Ingo Molnar
2015-06-19 20:07           ` Arnaldo Carvalho de Melo
2015-06-19 23:05             ` Ingo Molnar
2015-06-22 14:52               ` Arnaldo Carvalho de Melo

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.