perf/core improvements and fixes

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)

----------------------------------------------------------------
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(-)

From: Wang Nan <[email protected]>

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

---
 tools/perf/.gitignore | 1 +
 1 file changed, 1 insertion(+)

index 812f904193e8..09db62ba5786 100644
@@ -28,3 +28,4 @@ config.mak.autogen
 *-flex.*
 *.pyc
 *.pyo
+.config-detected

2.1.0

From: Wang Nan <[email protected]>

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.

---
 tools/perf/util/session.c | 50 ++++++++++++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 14 deletions(-)

index f31e024ddf7d..e1cd17c2afab 100644
@@ -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

From: Wang Nan <[email protected]>

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.

---
 tools/perf/Makefile.perf | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

index 374378322db9..1af0cfeb7a57 100644
@@ -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

From: Masami Hiramatsu <[email protected]>

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.

---
 tools/perf/util/probe-event.c | 35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

index 85c8207c25cc..65a1c8252270 100644
@@ -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

From: Masami Hiramatsu <[email protected]>

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

---
 tools/perf/util/probe-event.c | 48 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 4 deletions(-)

index 65a1c8252270..076527b639bd 100644
@@ -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

From: Sukadev Bhattiprolu <[email protected]>

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.

---
 tools/perf/util/evlist.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

index dc1dc2c181ef..6b58a47a79ec 100644
@@ -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

From: Arnaldo Carvalho de Melo <[email protected]>

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.

---
 tools/perf/util/evlist.c | 9 +++++++++
 tools/perf/util/evlist.h | 2 ++
 2 files changed, 11 insertions(+)

index 6b58a47a79ec..8366511b45f8 100644
@@ -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,

index 955bf31b7dd3..a8489b9d2812 100644
@@ -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

From: Arnaldo Carvalho de Melo <[email protected]>

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.

---
 tools/perf/builtin-top.c       | 52 +++++++++++++++++++++++++++++-------------
 tools/perf/ui/browsers/hists.c |  2 ++
 2 files changed, 38 insertions(+), 16 deletions(-)

index 6b987424d015..72d8a7ae5986 100644
@@ -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;

index e64893f2fd7f..8f7c4d49d327 100644
@@ -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

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'.

Pulled, thanks a lot Arnaldo!

Ingo

Em Thu, Jun 18, 2015 at 09:40:10AM +0200, Ingo Molnar escreveu:

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 

I'll fix that.

- Arnaldo

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

Thanks,

Ingo

Em Thu, Jun 18, 2015 at 10:58:13PM +0200, Ingo Molnar escreveu:

<SNIP>

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

Sounds good to me!

Thanks,

Ingo

Em Fri, Jun 19, 2015 at 08:27:11AM +0200, Ingo Molnar escreveu:

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/

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/

Em Sat, Jun 20, 2015 at 01:05:40AM +0200, Ingo Molnar escreveu:

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.

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/