linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL 0/6] perf/core fixes and improvements
@ 2011-08-16 14:41 Arnaldo Carvalho de Melo
  2011-08-17 21:36 ` Ingo Molnar
  0 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-08-16 14:41 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, David Ahern,
	Frederic Weisbecker, Ingo Molnar, Peter Zijlstra,
	Stephane Eranian, acme

Hi Ingo,

        Please consider pulling from:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux perf/core

Regards,

- Arnaldo

Stephane Eranian (6):
  perf list: Fix exit value
  perf evlist: Fix missing event name init for default event (v2)
  perf tools: Fix error handling of unknown events
  perf annotate: Make output more readable
  perf annotate: Add --symfs option
  perf stat: Add -o and --append options

 tools/perf/Documentation/perf-annotate.txt |   11 ++
 tools/perf/Documentation/perf-stat.txt     |    7 ++
 tools/perf/builtin-annotate.c              |    6 +
 tools/perf/builtin-stat.c                  |  155 ++++++++++++++++-----------
 tools/perf/util/annotate.c                 |    5 +-
 tools/perf/util/color.c                    |    2 +-
 tools/perf/util/evlist.c                   |   11 ++-
 tools/perf/util/parse-events.c             |    8 +-
 tools/perf/util/symbol.c                   |    2 +
 tools/perf/util/symbol.h                   |    4 +-
 10 files changed, 141 insertions(+), 70 deletions(-)


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

* Re: [GIT PULL 0/6] perf/core fixes and improvements
  2011-08-16 14:41 Arnaldo Carvalho de Melo
@ 2011-08-17 21:36 ` Ingo Molnar
  2011-08-18 12:43   ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 12+ messages in thread
From: Ingo Molnar @ 2011-08-17 21:36 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, linux-kernel, David Ahern, Frederic Weisbecker,
	Peter Zijlstra, Stephane Eranian, acme


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

> Hi Ingo,
> 
>         Please consider pulling from:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux perf/core
> 
> Regards,
> 
> - Arnaldo
> 
> Stephane Eranian (6):
>   perf list: Fix exit value
>   perf evlist: Fix missing event name init for default event (v2)
>   perf tools: Fix error handling of unknown events
>   perf annotate: Make output more readable
>   perf annotate: Add --symfs option
>   perf stat: Add -o and --append options
> 
>  tools/perf/Documentation/perf-annotate.txt |   11 ++
>  tools/perf/Documentation/perf-stat.txt     |    7 ++
>  tools/perf/builtin-annotate.c              |    6 +
>  tools/perf/builtin-stat.c                  |  155 ++++++++++++++++-----------
>  tools/perf/util/annotate.c                 |    5 +-
>  tools/perf/util/color.c                    |    2 +-
>  tools/perf/util/evlist.c                   |   11 ++-
>  tools/perf/util/parse-events.c             |    8 +-
>  tools/perf/util/symbol.c                   |    2 +
>  tools/perf/util/symbol.h                   |    4 +-
>  10 files changed, 141 insertions(+), 70 deletions(-)

hm, shouldnt these:

>   perf list: Fix exit value
>   perf evlist: Fix missing event name init for default event (v2)
>   perf tools: Fix error handling of unknown events

be in perf/urgent (based on -git) and go into v3.1, while the rest is 
for perf/core and goes into v3.2?

Thanks,

	Ingo

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

* Re: [GIT PULL 0/6] perf/core fixes and improvements
  2011-08-17 21:36 ` Ingo Molnar
@ 2011-08-18 12:43   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-08-18 12:43 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Ingo Molnar, linux-kernel, David Ahern, Frederic Weisbecker,
	Peter Zijlstra, Stephane Eranian

Em Wed, Aug 17, 2011 at 11:36:17PM +0200, Ingo Molnar escreveu:
> * Arnaldo Carvalho de Melo <acme@infradead.org> wrote:
> > Hi Ingo,
> > 
> >         Please consider pulling from:
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux perf/core
> 
> hm, shouldnt these:
> 
> >   perf list: Fix exit value
> >   perf evlist: Fix missing event name init for default event (v2)
> >   perf tools: Fix error handling of unknown events
> 
> be in perf/urgent (based on -git) and go into v3.1, while the rest is 
> for perf/core and goes into v3.2?

I'll rework the branches, adding these and other fixes (glibc build fix,
etc) in perf/urgent and the rest in perf/core, resubmit.

- Arnaldo

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

* [GIT PULL 0/6] perf/core fixes and improvements
@ 2011-10-13 15:08 Arnaldo Carvalho de Melo
  2011-10-13 15:08 ` [PATCH 1/6] perf hists browser: Recalculate browser pointers after resort/decay Arnaldo Carvalho de Melo
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-10-13 15:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, David Ahern,
	Frederic Weisbecker, Ingo Molnar, Mike Galbraith, Paul Mackerras,
	Peter Zijlstra, Stephane Eranian, arnaldo.melo

Hi Ingo,

        Please consider pulling from:

git://github.com/acmel/linux.git perf/core

Regards,

- Arnaldo

Arnaldo Carvalho de Melo (5):
  perf hists browser: Recalculate browser pointers after resort/decay
  perf hists: Don't free decayed entries if in the annotation browser
  perf ui browser: Handle SIGWINCH
  perf ui browser: Remove ui_browser__add_exit_keys
  perf top: Remove entries from entries_collapsed on decay

Stephane Eranian (1):
  perf hists: Fix compilation when NO_NEWT_SUPPORT is set

 tools/perf/builtin-top.c               |   23 ++++-
 tools/perf/perf.c                      |   24 +++++
 tools/perf/perf.h                      |    2 +
 tools/perf/util/hist.c                 |   24 ++++-
 tools/perf/util/hist.h                 |   11 ++-
 tools/perf/util/sort.h                 |    1 +
 tools/perf/util/ui/browser.c           |  176 ++++++++++++++++++++------------
 tools/perf/util/ui/browser.h           |    9 +-
 tools/perf/util/ui/browsers/annotate.c |   17 +--
 tools/perf/util/ui/browsers/hists.c    |   55 +++-------
 tools/perf/util/ui/browsers/map.c      |    6 +-
 tools/perf/util/ui/helpline.h          |    2 +
 12 files changed, 216 insertions(+), 134 deletions(-)


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

* [PATCH 1/6] perf hists browser: Recalculate browser pointers after resort/decay
  2011-10-13 15:08 [GIT PULL 0/6] perf/core fixes and improvements Arnaldo Carvalho de Melo
@ 2011-10-13 15:08 ` Arnaldo Carvalho de Melo
  2011-10-13 15:08 ` [PATCH 2/6] perf hists: Don't free decayed entries if in the annotation browser Arnaldo Carvalho de Melo
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-10-13 15:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, David Ahern,
	Frederic Weisbecker, Mike Galbraith, Paul Mackerras,
	Peter Zijlstra, Stephane Eranian

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

In browsers that access dynamic underlying data structures, like in the
hists browser and its hist_entry rb_tree, we need to revalidate any
reference to the underlying data structure, because they can have gone
away, decayed.

This fixes a problem where after a while the top entries get behind the
top of the screen, i.e. the top_idx stays at 0, which means it is at the
first entry in the rb_tree when in fact it wasn't because the
browser->top didn't got revalidated after the timer ran and the
underlying data structure got updated.

Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-mhje66qssdko24q67a2lhlho@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/ui/browser.c        |   23 +++++++++++++++++++++++
 tools/perf/util/ui/browser.h        |    1 +
 tools/perf/util/ui/browsers/hists.c |    9 ++-------
 3 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/ui/browser.c b/tools/perf/util/ui/browser.c
index 611219f..5911bba 100644
--- a/tools/perf/util/ui/browser.c
+++ b/tools/perf/util/ui/browser.c
@@ -230,6 +230,29 @@ int ui_browser__refresh(struct ui_browser *self)
 	return 0;
 }
 
+/*
+ * Here we're updating nr_entries _after_ we started browsing, i.e.  we have to
+ * forget about any reference to any entry in the underlying data structure,
+ * that is why we do a SEEK_SET. Think about 'perf top' in the hists browser
+ * after an output_resort and hist decay.
+ */
+void ui_browser__update_nr_entries(struct ui_browser *browser, u32 nr_entries)
+{
+	off_t offset = nr_entries - browser->nr_entries;
+
+	browser->nr_entries = nr_entries;
+
+	if (offset < 0) {
+		if (browser->top_idx < (u64)-offset)
+			offset = -browser->top_idx;
+
+		browser->index += offset;
+		browser->top_idx += offset;
+	}
+
+	browser->seek(browser, browser->top_idx, SEEK_SET);
+}
+
 int ui_browser__run(struct ui_browser *self)
 {
 	struct newtExitStruct es;
diff --git a/tools/perf/util/ui/browser.h b/tools/perf/util/ui/browser.h
index fc63dda..d42be43 100644
--- a/tools/perf/util/ui/browser.h
+++ b/tools/perf/util/ui/browser.h
@@ -41,6 +41,7 @@ int ui_browser__show(struct ui_browser *self, const char *title,
 void ui_browser__hide(struct ui_browser *self);
 int ui_browser__refresh(struct ui_browser *self);
 int ui_browser__run(struct ui_browser *self);
+void ui_browser__update_nr_entries(struct ui_browser *browser, u32 nr_entries);
 
 void ui_browser__rb_tree_seek(struct ui_browser *self, off_t offset, int whence);
 unsigned int ui_browser__rb_tree_refresh(struct ui_browser *self);
diff --git a/tools/perf/util/ui/browsers/hists.c b/tools/perf/util/ui/browsers/hists.c
index e64d952..9ece843 100644
--- a/tools/perf/util/ui/browsers/hists.c
+++ b/tools/perf/util/ui/browsers/hists.c
@@ -332,13 +332,7 @@ static int hist_browser__run(struct hist_browser *self, const char *ev_name,
 		case -1:
 			/* FIXME we need to check if it was es.reason == NEWT_EXIT_TIMER */
 			timer(arg);
-			/*
-			 * The timer may have changed the number of entries.
-			 * XXX: Find better way to keep this in synch, probably
-			 * removing this timer function altogether and just sync
-			 * using the hists->lock...
-			 */
-			self->b.nr_entries = self->hists->nr_entries;
+			ui_browser__update_nr_entries(&self->b, self->hists->nr_entries);
 			hists__browser_title(self->hists, title, sizeof(title),
 					     ev_name, self->dso_filter,
 					     self->thread_filter);
@@ -985,6 +979,7 @@ do_annotate:
 
 			hist_entry__tui_annotate(he, evsel->idx, nr_events,
 						 timer, arg, delay_secs);
+			ui_browser__update_nr_entries(&browser->b, browser->hists->nr_entries);
 		} else if (choice == browse_map)
 			map__browse(browser->selection->map);
 		else if (choice == zoom_dso) {
-- 
1.6.2.5


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

* [PATCH 2/6] perf hists: Don't free decayed entries if in the annotation browser
  2011-10-13 15:08 [GIT PULL 0/6] perf/core fixes and improvements Arnaldo Carvalho de Melo
  2011-10-13 15:08 ` [PATCH 1/6] perf hists browser: Recalculate browser pointers after resort/decay Arnaldo Carvalho de Melo
@ 2011-10-13 15:08 ` Arnaldo Carvalho de Melo
  2011-10-13 15:08 ` [PATCH 3/6] perf hists: Fix compilation when NO_NEWT_SUPPORT is set Arnaldo Carvalho de Melo
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-10-13 15:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, David Ahern,
	Frederic Weisbecker, Mike Galbraith, Paul Mackerras,
	Peter Zijlstra, Stephane Eranian

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

Just let it there till the user exits the annotation browser.

Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-nmaxuzreqhm5k10t2co5sk9a@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c              |   10 ++++++++--
 tools/perf/util/sort.h              |    1 +
 tools/perf/util/ui/browsers/hists.c |    6 +++++-
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 50c8fec..9b9d12b 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -100,6 +100,8 @@ static void hist_entry__decay(struct hist_entry *he)
 
 static bool hists__decay_entry(struct hists *hists, struct hist_entry *he)
 {
+	if (he->period == 0)
+		return true;
 	hists->stats.total_period -= he->period;
 	hist_entry__decay(he);
 	hists->stats.total_period += he->period;
@@ -114,8 +116,12 @@ void hists__decay_entries(struct hists *hists)
 	while (next) {
 		n = rb_entry(next, struct hist_entry, rb_node);
 		next = rb_next(&n->rb_node);
-
-		if (hists__decay_entry(hists, n)) {
+		/*
+		 * We may be annotating this, for instance, so keep it here in
+		 * case some it gets new samples, we'll eventually free it when
+		 * the user stops browsing and it agains gets fully decayed.
+		 */
+		if (hists__decay_entry(hists, n) && !n->used) {
 			rb_erase(&n->rb_node, &hists->entries);
 
 			if (sort__need_collapse)
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 03851e3..3f67ae3 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -64,6 +64,7 @@ struct hist_entry {
 
 	bool			init_have_children;
 	char			level;
+	bool			used;
 	u8			filtered;
 	struct symbol		*parent;
 	union {
diff --git a/tools/perf/util/ui/browsers/hists.c b/tools/perf/util/ui/browsers/hists.c
index 9ece843..fdc3c90 100644
--- a/tools/perf/util/ui/browsers/hists.c
+++ b/tools/perf/util/ui/browsers/hists.c
@@ -976,9 +976,13 @@ do_annotate:
 			he = hist_browser__selected_entry(browser);
 			if (he == NULL)
 				continue;
-
+			/*
+			 * Don't let this be freed, say, by hists__decay_entry.
+			 */
+			he->used = true;
 			hist_entry__tui_annotate(he, evsel->idx, nr_events,
 						 timer, arg, delay_secs);
+			he->used = false;
 			ui_browser__update_nr_entries(&browser->b, browser->hists->nr_entries);
 		} else if (choice == browse_map)
 			map__browse(browser->selection->map);
-- 
1.6.2.5


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

* [PATCH 3/6] perf hists: Fix compilation when NO_NEWT_SUPPORT is set
  2011-10-13 15:08 [GIT PULL 0/6] perf/core fixes and improvements Arnaldo Carvalho de Melo
  2011-10-13 15:08 ` [PATCH 1/6] perf hists browser: Recalculate browser pointers after resort/decay Arnaldo Carvalho de Melo
  2011-10-13 15:08 ` [PATCH 2/6] perf hists: Don't free decayed entries if in the annotation browser Arnaldo Carvalho de Melo
@ 2011-10-13 15:08 ` Arnaldo Carvalho de Melo
  2011-10-13 15:08 ` [PATCH 4/6] perf ui browser: Handle SIGWINCH Arnaldo Carvalho de Melo
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-10-13 15:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Stephane Eranian, Ingo Molnar, Peter Zijlstra,
	Arnaldo Carvalho de Melo

From: Stephane Eranian <eranian@google.com>

This patch, relative to tip/master, makes perf compile when
NO_NEWT_SUPPORT is set.  It also fixes the line formatting to fit 80
columns.

Please test with NO_NEWT.

Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20111012120328.GA1619@quad
Signed-off-by: Stephane Eranian <eranian@google.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.h |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 7ea1e56..7416521 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -103,16 +103,20 @@ struct perf_evlist;
 #ifdef NO_NEWT_SUPPORT
 static inline
 int perf_evlist__tui_browse_hists(struct perf_evlist *evlist __used,
-				  const char *help __used, void(*timer)(void *arg) __used, void *arg,
+				  const char *help __used,
+				  void(*timer)(void *arg) __used,
+				  void *arg __used,
 				  int refresh __used)
 {
 	return 0;
 }
 
 static inline int hist_entry__tui_annotate(struct hist_entry *self __used,
-					   int evidx __used, int nr_events __used,
+					   int evidx __used,
+					   int nr_events __used,
 					   void(*timer)(void *arg) __used,
-					   void *arg __used, int delay_secs __used);
+					   void *arg __used,
+					   int delay_secs __used)
 {
 	return 0;
 }
-- 
1.6.2.5


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

* [PATCH 4/6] perf ui browser: Handle SIGWINCH
  2011-10-13 15:08 [GIT PULL 0/6] perf/core fixes and improvements Arnaldo Carvalho de Melo
                   ` (2 preceding siblings ...)
  2011-10-13 15:08 ` [PATCH 3/6] perf hists: Fix compilation when NO_NEWT_SUPPORT is set Arnaldo Carvalho de Melo
@ 2011-10-13 15:08 ` Arnaldo Carvalho de Melo
  2011-10-13 15:08 ` [PATCH 5/6] perf ui browser: Remove ui_browser__add_exit_keys Arnaldo Carvalho de Melo
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-10-13 15:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, David Ahern,
	Frederic Weisbecker, Mike Galbraith, Paul Mackerras,
	Peter Zijlstra, Stephane Eranian

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

To do that we needed to stop using newtForm, as we don't want libnewt to
catch the xterm resize signal.

Remove some more newt calls and instead use the underlying libslang
directly. In time tools/perf will use just libslang.

Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-h1824yjiru5n2ivz4bseizwj@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-top.c               |   21 ++++-
 tools/perf/perf.c                      |   24 ++++++
 tools/perf/perf.h                      |    2 +
 tools/perf/util/ui/browser.c           |  137 +++++++++++++++++++++-----------
 tools/perf/util/ui/browser.h           |    6 +-
 tools/perf/util/ui/browsers/annotate.c |    5 +-
 tools/perf/util/ui/browsers/hists.c    |   12 +---
 tools/perf/util/ui/browsers/map.c      |    3 +-
 tools/perf/util/ui/helpline.h          |    2 +
 9 files changed, 144 insertions(+), 68 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index c5aebf6..de3cb1e 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -585,16 +585,31 @@ static void *display_thread(void *arg __used)
 	tc.c_cc[VMIN] = 0;
 	tc.c_cc[VTIME] = 0;
 
+	pthread__unblock_sigwinch();
 repeat:
 	delay_msecs = top.delay_secs * 1000;
 	tcsetattr(0, TCSANOW, &tc);
 	/* trash return*/
 	getc(stdin);
 
-	do {
+	while (1) {
 		print_sym_table();
-	} while (!poll(&stdin_poll, 1, delay_msecs) == 1);
-
+		/*
+		 * Either timeout expired or we got an EINTR due to SIGWINCH,
+		 * refresh screen in both cases.
+		 */
+		switch (poll(&stdin_poll, 1, delay_msecs)) {
+		case 0:
+			continue;
+		case -1:
+			if (errno == EINTR)
+				continue;
+			/* Fall trhu */
+		default:
+			goto process_hotkey;
+		}
+	}
+process_hotkey:
 	c = getc(stdin);
 	tcsetattr(0, TCSAFLUSH, &save);
 
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index ec635b7..73d0cac 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -427,6 +427,24 @@ static void get_debugfs_mntpt(void)
 		debugfs_mntpt[0] = '\0';
 }
 
+static void pthread__block_sigwinch(void)
+{
+	sigset_t set;
+
+	sigemptyset(&set);
+	sigaddset(&set, SIGWINCH);
+	pthread_sigmask(SIG_BLOCK, &set, NULL);
+}
+
+void pthread__unblock_sigwinch(void)
+{
+	sigset_t set;
+
+	sigemptyset(&set);
+	sigaddset(&set, SIGWINCH);
+	pthread_sigmask(SIG_UNBLOCK, &set, NULL);
+}
+
 int main(int argc, const char **argv)
 {
 	const char *cmd;
@@ -480,6 +498,12 @@ int main(int argc, const char **argv)
 	 * time.
 	 */
 	setup_path();
+	/*
+	 * Block SIGWINCH notifications so that the thread that wants it can
+	 * unblock and get syscalls like select interrupted instead of waiting
+	 * forever while the signal goes to some other non interested thread.
+	 */
+	pthread__block_sigwinch();
 
 	while (1) {
 		static int done_help;
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 08b0b5e..914c895 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -183,4 +183,6 @@ struct ip_callchain {
 extern bool perf_host, perf_guest;
 extern const char perf_version_string[];
 
+void pthread__unblock_sigwinch(void);
+
 #endif
diff --git a/tools/perf/util/ui/browser.c b/tools/perf/util/ui/browser.c
index 5911bba..05a0f61 100644
--- a/tools/perf/util/ui/browser.c
+++ b/tools/perf/util/ui/browser.c
@@ -1,4 +1,7 @@
+#include "../util.h"
+#include "../../perf.h"
 #include "libslang.h"
+#include <newt.h>
 #include "ui.h"
 #include <linux/compiler.h>
 #include <linux/list.h>
@@ -8,8 +11,8 @@
 #include "browser.h"
 #include "helpline.h"
 #include "../color.h"
-#include "../util.h"
-#include <stdio.h>
+
+int newtGetKey(void);
 
 static int ui_browser__percent_color(double percent, bool current)
 {
@@ -127,11 +130,8 @@ bool ui_browser__is_current_entry(struct ui_browser *self, unsigned row)
 
 void ui_browser__refresh_dimensions(struct ui_browser *self)
 {
-	int cols, rows;
-	newtGetScreenSize(&cols, &rows);
-
-	self->width = cols - 1;
-	self->height = rows - 2;
+	self->width = SLtt_Screen_Cols - 1;
+	self->height = SLtt_Screen_Rows - 2;
 	self->y = 1;
 	self->x = 0;
 }
@@ -142,9 +142,8 @@ void ui_browser__reset_index(struct ui_browser *self)
 	self->seek(self, 0, SEEK_SET);
 }
 
-void ui_browser__add_exit_key(struct ui_browser *self, int key)
+void ui_browser__add_exit_key(struct ui_browser *browser __used, int key __used)
 {
-	newtFormAddHotKey(self->form, key);
 }
 
 void ui_browser__add_exit_keys(struct ui_browser *self, int keys[])
@@ -161,7 +160,7 @@ void __ui_browser__show_title(struct ui_browser *browser, const char *title)
 {
 	SLsmg_gotorc(0, 0);
 	ui_browser__set_color(browser, NEWT_COLORSET_ROOT);
-	slsmg_write_nstring(title, browser->width);
+	slsmg_write_nstring(title, browser->width + 1);
 }
 
 void ui_browser__show_title(struct ui_browser *browser, const char *title)
@@ -174,57 +173,75 @@ void ui_browser__show_title(struct ui_browser *browser, const char *title)
 int ui_browser__show(struct ui_browser *self, const char *title,
 		     const char *helpline, ...)
 {
+	int err;
 	va_list ap;
 	int keys[] = { NEWT_KEY_UP, NEWT_KEY_DOWN, NEWT_KEY_PGUP,
 		       NEWT_KEY_PGDN, NEWT_KEY_HOME, NEWT_KEY_END, ' ',
 		       NEWT_KEY_LEFT, NEWT_KEY_ESCAPE, 'q', CTRL('c'), 0 };
 
-	if (self->form != NULL)
-		newtFormDestroy(self->form);
-
 	ui_browser__refresh_dimensions(self);
-	self->form = newtForm(NULL, NULL, 0);
-	if (self->form == NULL)
-		return -1;
-
-	self->sb = newtVerticalScrollbar(self->width, 1, self->height,
-					 HE_COLORSET_NORMAL,
-					 HE_COLORSET_SELECTED);
-	if (self->sb == NULL)
-		return -1;
 
 	pthread_mutex_lock(&ui__lock);
 	__ui_browser__show_title(self, title);
 
 	ui_browser__add_exit_keys(self, keys);
-	newtFormAddComponent(self->form, self->sb);
+	self->title = title;
+	free(self->helpline);
+	self->helpline = NULL;
 
 	va_start(ap, helpline);
-	ui_helpline__vpush(helpline, ap);
+	err = vasprintf(&self->helpline, helpline, ap);
 	va_end(ap);
+	if (err > 0)
+		ui_helpline__push(self->helpline);
 	pthread_mutex_unlock(&ui__lock);
-	return 0;
+	return err ? 0 : -1;
 }
 
-void ui_browser__hide(struct ui_browser *self)
+void ui_browser__hide(struct ui_browser *browser __used)
 {
 	pthread_mutex_lock(&ui__lock);
-	newtFormDestroy(self->form);
-	self->form = NULL;
 	ui_helpline__pop();
 	pthread_mutex_unlock(&ui__lock);
 }
 
-int ui_browser__refresh(struct ui_browser *self)
+static void ui_browser__scrollbar_set(struct ui_browser *browser)
+{
+	int height = browser->height, h = 0, pct = 0,
+	    col = browser->width,
+	    row = browser->y - 1;
+
+	if (browser->nr_entries > 1) {
+		pct = ((browser->index * (browser->height - 1)) /
+		       (browser->nr_entries - 1));
+	}
+
+	while (h < height) {
+	        ui_browser__gotorc(browser, row++, col);
+		SLsmg_set_char_set(1);
+		SLsmg_write_char(h == pct ? SLSMG_DIAMOND_CHAR : SLSMG_BOARD_CHAR);
+		SLsmg_set_char_set(0);
+		++h;
+	}
+}
+
+static int __ui_browser__refresh(struct ui_browser *browser)
 {
 	int row;
 
+	row = browser->refresh(browser);
+	ui_browser__set_color(browser, HE_COLORSET_NORMAL);
+	SLsmg_fill_region(browser->y + row, browser->x,
+			  browser->height - row, browser->width, ' ');
+	ui_browser__scrollbar_set(browser);
+
+	return 0;
+}
+
+int ui_browser__refresh(struct ui_browser *browser)
+{
 	pthread_mutex_lock(&ui__lock);
-	newtScrollbarSet(self->sb, self->index, self->nr_entries - 1);
-	row = self->refresh(self);
-	ui_browser__set_color(self, HE_COLORSET_NORMAL);
-	SLsmg_fill_region(self->y + row, self->x,
-			  self->height - row, self->width, ' ');
+	__ui_browser__refresh(browser);
 	pthread_mutex_unlock(&ui__lock);
 
 	return 0;
@@ -253,21 +270,49 @@ void ui_browser__update_nr_entries(struct ui_browser *browser, u32 nr_entries)
 	browser->seek(browser, browser->top_idx, SEEK_SET);
 }
 
-int ui_browser__run(struct ui_browser *self)
+int ui_browser__run(struct ui_browser *self, int delay_secs)
 {
-	struct newtExitStruct es;
+	int err, key;
+	struct timeval timeout, *ptimeout = delay_secs ? &timeout : NULL;
 
-	if (ui_browser__refresh(self) < 0)
-		return -1;
+	pthread__unblock_sigwinch();
 
 	while (1) {
 		off_t offset;
+		fd_set read_set;
 
-		newtFormRun(self->form, &es);
+		pthread_mutex_lock(&ui__lock);
+		err = __ui_browser__refresh(self);
+		SLsmg_refresh();
+		pthread_mutex_unlock(&ui__lock);
+		if (err < 0)
+			break;
+
+		FD_ZERO(&read_set);
+		FD_SET(0, &read_set);
+
+		if (delay_secs) {
+			timeout.tv_sec = delay_secs;
+			timeout.tv_usec = 0;
+		}
 
-		if (es.reason != NEWT_EXIT_HOTKEY)
+	        err = select(1, &read_set, NULL, NULL, ptimeout);
+		if (err > 0 && FD_ISSET(0, &read_set))
+			key = newtGetKey();
+		else if (err == 0)
 			break;
-		switch (es.u.key) {
+		else {
+			pthread_mutex_lock(&ui__lock);
+			SLtt_get_screen_size();
+			SLsmg_reinit_smg();
+			pthread_mutex_unlock(&ui__lock);
+			ui_browser__refresh_dimensions(self);
+			__ui_browser__show_title(self, self->title);
+			ui_helpline__puts(self->helpline);
+			continue;
+		}
+
+		switch (key) {
 		case NEWT_KEY_DOWN:
 			if (self->index == self->nr_entries - 1)
 				break;
@@ -324,10 +369,8 @@ int ui_browser__run(struct ui_browser *self)
 			self->seek(self, -offset, SEEK_END);
 			break;
 		default:
-			return es.u.key;
+			return key;
 		}
-		if (ui_browser__refresh(self) < 0)
-			return -1;
 	}
 	return -1;
 }
@@ -353,13 +396,13 @@ unsigned int ui_browser__list_head_refresh(struct ui_browser *self)
 	return row;
 }
 
-static struct newtPercentTreeColors {
+static struct ui_browser__colors {
 	const char *topColorFg, *topColorBg;
 	const char *mediumColorFg, *mediumColorBg;
 	const char *normalColorFg, *normalColorBg;
 	const char *selColorFg, *selColorBg;
 	const char *codeColorFg, *codeColorBg;
-} defaultPercentTreeColors = {
+} ui_browser__default_colors = {
 	"red",       "lightgray",
 	"green",     "lightgray",
 	"black",     "lightgray",
@@ -369,7 +412,7 @@ static struct newtPercentTreeColors {
 
 void ui_browser__init(void)
 {
-	struct newtPercentTreeColors *c = &defaultPercentTreeColors;
+	struct ui_browser__colors *c = &ui_browser__default_colors;
 
 	sltt_set_color(HE_COLORSET_TOP, NULL, c->topColorFg, c->topColorBg);
 	sltt_set_color(HE_COLORSET_MEDIUM, NULL, c->mediumColorFg, c->mediumColorBg);
diff --git a/tools/perf/util/ui/browser.h b/tools/perf/util/ui/browser.h
index d42be43..37d56bf 100644
--- a/tools/perf/util/ui/browser.h
+++ b/tools/perf/util/ui/browser.h
@@ -2,7 +2,6 @@
 #define _PERF_UI_BROWSER_H_ 1
 
 #include <stdbool.h>
-#include <newt.h>
 #include <sys/types.h>
 #include "../types.h"
 
@@ -13,11 +12,12 @@
 #define HE_COLORSET_CODE	54
 
 struct ui_browser {
-	newtComponent form, sb;
 	u64	      index, top_idx;
 	void	      *top, *entries;
 	u16	      y, x, width, height;
 	void	      *priv;
+	const char    *title;
+	char	      *helpline;
 	unsigned int  (*refresh)(struct ui_browser *self);
 	void	      (*write)(struct ui_browser *self, void *entry, int row);
 	void	      (*seek)(struct ui_browser *self, off_t offset, int whence);
@@ -40,7 +40,7 @@ int ui_browser__show(struct ui_browser *self, const char *title,
 		     const char *helpline, ...);
 void ui_browser__hide(struct ui_browser *self);
 int ui_browser__refresh(struct ui_browser *self);
-int ui_browser__run(struct ui_browser *self);
+int ui_browser__run(struct ui_browser *browser, int delay_secs);
 void ui_browser__update_nr_entries(struct ui_browser *browser, u32 nr_entries);
 
 void ui_browser__rb_tree_seek(struct ui_browser *self, off_t offset, int whence);
diff --git a/tools/perf/util/ui/browsers/annotate.c b/tools/perf/util/ui/browsers/annotate.c
index 674b55e..1967fbf 100644
--- a/tools/perf/util/ui/browsers/annotate.c
+++ b/tools/perf/util/ui/browsers/annotate.c
@@ -196,11 +196,8 @@ static int annotate_browser__run(struct annotate_browser *self, int evidx,
 
 	nd = self->curr_hot;
 
-	if (delay_secs != 0)
-		newtFormSetTimer(self->b.form, delay_secs * 1000);
-
 	while (1) {
-		key = ui_browser__run(&self->b);
+		key = ui_browser__run(&self->b, delay_secs);
 
 		if (delay_secs != 0) {
 			annotate_browser__calc_percent(self, evidx);
diff --git a/tools/perf/util/ui/browsers/hists.c b/tools/perf/util/ui/browsers/hists.c
index fdc3c90..603d6ee 100644
--- a/tools/perf/util/ui/browsers/hists.c
+++ b/tools/perf/util/ui/browsers/hists.c
@@ -301,7 +301,6 @@ static int hist_browser__run(struct hist_browser *self, const char *ev_name,
 			     void(*timer)(void *arg), void *arg, int delay_secs)
 {
 	int key;
-	int delay_msecs = delay_secs * 1000;
 	char title[160];
 	int sym_exit_keys[] = { 'a', 'h', 'C', 'd', 'E', 't', 0, };
 	int exit_keys[] = { '?', 'h', 'D', NEWT_KEY_LEFT, NEWT_KEY_RIGHT,
@@ -318,15 +317,12 @@ static int hist_browser__run(struct hist_browser *self, const char *ev_name,
 			     "Press '?' for help on key bindings") < 0)
 		return -1;
 
-	if (timer != NULL)
-		newtFormSetTimer(self->b.form, delay_msecs);
-
 	ui_browser__add_exit_keys(&self->b, exit_keys);
 	if (self->has_symbols)
 		ui_browser__add_exit_keys(&self->b, sym_exit_keys);
 
 	while (1) {
-		key = ui_browser__run(&self->b);
+		key = ui_browser__run(&self->b, delay_secs);
 
 		switch (key) {
 		case -1:
@@ -1061,7 +1057,6 @@ static int perf_evsel_menu__run(struct perf_evsel_menu *menu,
 				void(*timer)(void *arg), void *arg, int delay_secs)
 {
 	int exit_keys[] = { NEWT_KEY_ENTER, NEWT_KEY_RIGHT, 0, };
-	int delay_msecs = delay_secs * 1000;
 	struct perf_evlist *evlist = menu->b.priv;
 	struct perf_evsel *pos;
 	const char *ev_name, *title = "Available samples";
@@ -1071,13 +1066,10 @@ static int perf_evsel_menu__run(struct perf_evsel_menu *menu,
 			     "ESC: exit, ENTER|->: Browse histograms") < 0)
 		return -1;
 
-	if (timer != NULL)
-		newtFormSetTimer(menu->b.form, delay_msecs);
-
 	ui_browser__add_exit_keys(&menu->b, exit_keys);
 
 	while (1) {
-		key = ui_browser__run(&menu->b);
+		key = ui_browser__run(&menu->b, delay_secs);
 
 		switch (key) {
 		case -1:
diff --git a/tools/perf/util/ui/browsers/map.c b/tools/perf/util/ui/browsers/map.c
index 8462bff..499db76 100644
--- a/tools/perf/util/ui/browsers/map.c
+++ b/tools/perf/util/ui/browsers/map.c
@@ -1,5 +1,6 @@
 #include "../libslang.h"
 #include <elf.h>
+#include <newt.h>
 #include <inttypes.h>
 #include <sys/ttydefaults.h>
 #include <ctype.h>
@@ -112,7 +113,7 @@ static int map_browser__run(struct map_browser *self)
 		ui_browser__add_exit_key(&self->b, '/');
 
 	while (1) {
-		key = ui_browser__run(&self->b);
+		key = ui_browser__run(&self->b, 0);
 
 		if (verbose && key == '/')
 			map_browser__search(self);
diff --git a/tools/perf/util/ui/helpline.h b/tools/perf/util/ui/helpline.h
index ab6028d..8099757 100644
--- a/tools/perf/util/ui/helpline.h
+++ b/tools/perf/util/ui/helpline.h
@@ -1,6 +1,8 @@
 #ifndef _PERF_UI_HELPLINE_H_
 #define _PERF_UI_HELPLINE_H_ 1
 
+#include <stdio.h>
+
 void ui_helpline__init(void);
 void ui_helpline__pop(void);
 void ui_helpline__push(const char *msg);
-- 
1.6.2.5


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

* [PATCH 5/6] perf ui browser: Remove ui_browser__add_exit_keys
  2011-10-13 15:08 [GIT PULL 0/6] perf/core fixes and improvements Arnaldo Carvalho de Melo
                   ` (3 preceding siblings ...)
  2011-10-13 15:08 ` [PATCH 4/6] perf ui browser: Handle SIGWINCH Arnaldo Carvalho de Melo
@ 2011-10-13 15:08 ` Arnaldo Carvalho de Melo
  2011-10-13 15:08 ` [PATCH 6/6] perf top: Remove entries from entries_collapsed on decay Arnaldo Carvalho de Melo
  2011-10-14  7:16 ` [GIT PULL 0/6] perf/core fixes and improvements Ingo Molnar
  6 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-10-13 15:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, David Ahern,
	Frederic Weisbecker, Mike Galbraith, Paul Mackerras,
	Peter Zijlstra, Stephane Eranian

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

Users (hist_browser, etc) should just handle all keys, discarding the
ones they don't handle.

Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-fjouann12v2k58t6vdd2wawb@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/ui/browser.c           |   18 ------------------
 tools/perf/util/ui/browser.h           |    2 --
 tools/perf/util/ui/browsers/annotate.c |   12 ++++--------
 tools/perf/util/ui/browsers/hists.c    |   28 ++++++++--------------------
 tools/perf/util/ui/browsers/map.c      |    3 ---
 5 files changed, 12 insertions(+), 51 deletions(-)

diff --git a/tools/perf/util/ui/browser.c b/tools/perf/util/ui/browser.c
index 05a0f61..2923c49 100644
--- a/tools/perf/util/ui/browser.c
+++ b/tools/perf/util/ui/browser.c
@@ -142,20 +142,6 @@ void ui_browser__reset_index(struct ui_browser *self)
 	self->seek(self, 0, SEEK_SET);
 }
 
-void ui_browser__add_exit_key(struct ui_browser *browser __used, int key __used)
-{
-}
-
-void ui_browser__add_exit_keys(struct ui_browser *self, int keys[])
-{
-	int i = 0;
-
-	while (keys[i] && i < 64) {
-		ui_browser__add_exit_key(self, keys[i]);
-		++i;
-	}
-}
-
 void __ui_browser__show_title(struct ui_browser *browser, const char *title)
 {
 	SLsmg_gotorc(0, 0);
@@ -175,16 +161,12 @@ int ui_browser__show(struct ui_browser *self, const char *title,
 {
 	int err;
 	va_list ap;
-	int keys[] = { NEWT_KEY_UP, NEWT_KEY_DOWN, NEWT_KEY_PGUP,
-		       NEWT_KEY_PGDN, NEWT_KEY_HOME, NEWT_KEY_END, ' ',
-		       NEWT_KEY_LEFT, NEWT_KEY_ESCAPE, 'q', CTRL('c'), 0 };
 
 	ui_browser__refresh_dimensions(self);
 
 	pthread_mutex_lock(&ui__lock);
 	__ui_browser__show_title(self, title);
 
-	ui_browser__add_exit_keys(self, keys);
 	self->title = title;
 	free(self->helpline);
 	self->helpline = NULL;
diff --git a/tools/perf/util/ui/browser.h b/tools/perf/util/ui/browser.h
index 37d56bf..7dd9d61 100644
--- a/tools/perf/util/ui/browser.h
+++ b/tools/perf/util/ui/browser.h
@@ -32,8 +32,6 @@ void ui_browser__refresh_dimensions(struct ui_browser *self);
 void ui_browser__reset_index(struct ui_browser *self);
 
 void ui_browser__gotorc(struct ui_browser *self, int y, int x);
-void ui_browser__add_exit_key(struct ui_browser *self, int key);
-void ui_browser__add_exit_keys(struct ui_browser *self, int keys[]);
 void __ui_browser__show_title(struct ui_browser *browser, const char *title);
 void ui_browser__show_title(struct ui_browser *browser, const char *title);
 int ui_browser__show(struct ui_browser *self, const char *title,
diff --git a/tools/perf/util/ui/browsers/annotate.c b/tools/perf/util/ui/browsers/annotate.c
index 1967fbf..cbefb4f 100644
--- a/tools/perf/util/ui/browsers/annotate.c
+++ b/tools/perf/util/ui/browsers/annotate.c
@@ -175,12 +175,6 @@ static int annotate_browser__run(struct annotate_browser *self, int evidx,
 	struct rb_node *nd = NULL;
 	struct map_symbol *ms = self->b.priv;
 	struct symbol *sym = ms->sym;
-	/*
-	 * RIGHT To allow builtin-annotate to cycle thru multiple symbols by
-	 * examining the exit key for this function.
-	 */
-	int exit_keys[] = { 'H', NEWT_KEY_TAB, NEWT_KEY_UNTAB,
-			    NEWT_KEY_RIGHT, NEWT_KEY_ENTER, 0 };
 	int key;
 
 	if (ui_browser__show(&self->b, sym->name,
@@ -188,7 +182,6 @@ static int annotate_browser__run(struct annotate_browser *self, int evidx,
 			     "cycle hottest lines, H: Hottest, -> Line action") < 0)
 		return -1;
 
-	ui_browser__add_exit_keys(&self->b, exit_keys);
 	annotate_browser__calc_percent(self, evidx);
 
 	if (self->curr_hot)
@@ -292,8 +285,11 @@ static int annotate_browser__run(struct annotate_browser *self, int evidx,
 						     timer, arg, delay_secs);
 			}
 			break;
-		default:
+		case 'q':
+		case CTRL('c'):
 			goto out;
+		default:
+			continue;
 		}
 
 		if (nd != NULL)
diff --git a/tools/perf/util/ui/browsers/hists.c b/tools/perf/util/ui/browsers/hists.c
index 603d6ee..662b721 100644
--- a/tools/perf/util/ui/browsers/hists.c
+++ b/tools/perf/util/ui/browsers/hists.c
@@ -302,9 +302,6 @@ static int hist_browser__run(struct hist_browser *self, const char *ev_name,
 {
 	int key;
 	char title[160];
-	int sym_exit_keys[] = { 'a', 'h', 'C', 'd', 'E', 't', 0, };
-	int exit_keys[] = { '?', 'h', 'D', NEWT_KEY_LEFT, NEWT_KEY_RIGHT,
-			    NEWT_KEY_TAB, NEWT_KEY_UNTAB, NEWT_KEY_ENTER, 0, };
 
 	self->b.entries = &self->hists->entries;
 	self->b.nr_entries = self->hists->nr_entries;
@@ -317,10 +314,6 @@ static int hist_browser__run(struct hist_browser *self, const char *ev_name,
 			     "Press '?' for help on key bindings") < 0)
 		return -1;
 
-	ui_browser__add_exit_keys(&self->b, exit_keys);
-	if (self->has_symbols)
-		ui_browser__add_exit_keys(&self->b, sym_exit_keys);
-
 	while (1) {
 		key = ui_browser__run(&self->b, delay_secs);
 
@@ -921,8 +914,11 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 			    !ui__dialog_yesno("Do you really want to exit?"))
 				continue;
 			/* Fall thru */
-		default:
+		case 'q':
+		case CTRL('c'):
 			goto out_free_stack;
+		default:
+			continue;
 		}
 
 		if (!browser->has_symbols)
@@ -1056,7 +1052,6 @@ static int perf_evsel_menu__run(struct perf_evsel_menu *menu,
 				int nr_events, const char *help,
 				void(*timer)(void *arg), void *arg, int delay_secs)
 {
-	int exit_keys[] = { NEWT_KEY_ENTER, NEWT_KEY_RIGHT, 0, };
 	struct perf_evlist *evlist = menu->b.priv;
 	struct perf_evsel *pos;
 	const char *ev_name, *title = "Available samples";
@@ -1066,8 +1061,6 @@ static int perf_evsel_menu__run(struct perf_evsel_menu *menu,
 			     "ESC: exit, ENTER|->: Browse histograms") < 0)
 		return -1;
 
-	ui_browser__add_exit_keys(&menu->b, exit_keys);
-
 	while (1) {
 		key = ui_browser__run(&menu->b, delay_secs);
 
@@ -1091,15 +1084,6 @@ browse_hists:
 			break;
 		case NEWT_KEY_LEFT:
 			continue;
-		case NEWT_KEY_ESCAPE:
-			if (!ui__dialog_yesno("Do you really want to exit?"))
-				continue;
-			/* Fall thru */
-		default:
-			goto out;
-		}
-
-		switch (key) {
 		case NEWT_KEY_TAB:
 			if (pos->node.next == &evlist->entries)
 				pos = list_entry(evlist->entries.next, struct perf_evsel, node);
@@ -1114,6 +1098,10 @@ browse_hists:
 				pos = list_entry(pos->node.prev, struct perf_evsel, node);
 			perf_evlist__set_selected(evlist, pos);
 			goto browse_hists;
+		case NEWT_KEY_ESCAPE:
+			if (!ui__dialog_yesno("Do you really want to exit?"))
+				continue;
+			/* Fall thru */
 		case 'q':
 		case CTRL('c'):
 			goto out;
diff --git a/tools/perf/util/ui/browsers/map.c b/tools/perf/util/ui/browsers/map.c
index 499db76..6905bcc 100644
--- a/tools/perf/util/ui/browsers/map.c
+++ b/tools/perf/util/ui/browsers/map.c
@@ -109,9 +109,6 @@ static int map_browser__run(struct map_browser *self)
 			     verbose ? "" : "restart with -v to use") < 0)
 		return -1;
 
-	if (verbose)
-		ui_browser__add_exit_key(&self->b, '/');
-
 	while (1) {
 		key = ui_browser__run(&self->b, 0);
 
-- 
1.6.2.5


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

* [PATCH 6/6] perf top: Remove entries from entries_collapsed on decay
  2011-10-13 15:08 [GIT PULL 0/6] perf/core fixes and improvements Arnaldo Carvalho de Melo
                   ` (4 preceding siblings ...)
  2011-10-13 15:08 ` [PATCH 5/6] perf ui browser: Remove ui_browser__add_exit_keys Arnaldo Carvalho de Melo
@ 2011-10-13 15:08 ` Arnaldo Carvalho de Melo
  2011-10-14  7:16 ` [GIT PULL 0/6] perf/core fixes and improvements Ingo Molnar
  6 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-10-13 15:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, David Ahern,
	Frederic Weisbecker, Mike Galbraith, Paul Mackerras,
	Peter Zijlstra, Stephane Eranian

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

We were removing only when using a --sort order that needs collapsing,
while we also use it in the threaded case, causing memory corruption
because we were scribbling freed hist entries, oops.

Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-k16fb4jsulr7x0ixv43amb6d@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-top.c |    2 +-
 tools/perf/util/hist.c   |   14 ++++++++++++--
 tools/perf/util/hist.h   |    1 +
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index de3cb1e..3acf381 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -304,7 +304,7 @@ static void print_sym_table(void)
 
 	hists__collapse_resort_threaded(&top.sym_evsel->hists);
 	hists__output_resort_threaded(&top.sym_evsel->hists);
-	hists__decay_entries(&top.sym_evsel->hists);
+	hists__decay_entries_threaded(&top.sym_evsel->hists);
 	hists__output_recalc_col_len(&top.sym_evsel->hists, winsize.ws_row - 3);
 	putchar('\n');
 	hists__fprintf(&top.sym_evsel->hists, NULL, false, false,
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 9b9d12b..a7193c5 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -108,7 +108,7 @@ static bool hists__decay_entry(struct hists *hists, struct hist_entry *he)
 	return he->period == 0;
 }
 
-void hists__decay_entries(struct hists *hists)
+static void __hists__decay_entries(struct hists *hists, bool threaded)
 {
 	struct rb_node *next = rb_first(&hists->entries);
 	struct hist_entry *n;
@@ -124,7 +124,7 @@ void hists__decay_entries(struct hists *hists)
 		if (hists__decay_entry(hists, n) && !n->used) {
 			rb_erase(&n->rb_node, &hists->entries);
 
-			if (sort__need_collapse)
+			if (sort__need_collapse || threaded)
 				rb_erase(&n->rb_node_in, &hists->entries_collapsed);
 
 			hist_entry__free(n);
@@ -133,6 +133,16 @@ void hists__decay_entries(struct hists *hists)
 	}
 }
 
+void hists__decay_entries(struct hists *hists)
+{
+	return __hists__decay_entries(hists, false);
+}
+
+void hists__decay_entries_threaded(struct hists *hists)
+{
+	return __hists__decay_entries(hists, true);
+}
+
 /*
  * histogram, sorted on item, collects periods
  */
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 7416521..bcc8ab9 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -79,6 +79,7 @@ void hists__collapse_resort(struct hists *self);
 void hists__collapse_resort_threaded(struct hists *hists);
 
 void hists__decay_entries(struct hists *hists);
+void hists__decay_entries_threaded(struct hists *hists);
 void hists__output_recalc_col_len(struct hists *hists, int max_rows);
 
 void hists__inc_nr_events(struct hists *self, u32 type);
-- 
1.6.2.5


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

* Re: [GIT PULL 0/6] perf/core fixes and improvements
  2011-10-13 15:08 [GIT PULL 0/6] perf/core fixes and improvements Arnaldo Carvalho de Melo
                   ` (5 preceding siblings ...)
  2011-10-13 15:08 ` [PATCH 6/6] perf top: Remove entries from entries_collapsed on decay Arnaldo Carvalho de Melo
@ 2011-10-14  7:16 ` Ingo Molnar
  2011-10-14 15:07   ` Arnaldo Carvalho de Melo
  6 siblings, 1 reply; 12+ messages in thread
From: Ingo Molnar @ 2011-10-14  7:16 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, David Ahern, Frederic Weisbecker, Mike Galbraith,
	Paul Mackerras, Peter Zijlstra, Stephane Eranian, arnaldo.melo


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

> Hi Ingo,
> 
>         Please consider pulling from:
> 
> git://github.com/acmel/linux.git perf/core
> 
> Regards,
> 
> - Arnaldo
> 
> Arnaldo Carvalho de Melo (5):
>   perf hists browser: Recalculate browser pointers after resort/decay
>   perf hists: Don't free decayed entries if in the annotation browser
>   perf ui browser: Handle SIGWINCH
>   perf ui browser: Remove ui_browser__add_exit_keys
>   perf top: Remove entries from entries_collapsed on decay
> 
> Stephane Eranian (1):
>   perf hists: Fix compilation when NO_NEWT_SUPPORT is set
> 
>  tools/perf/builtin-top.c               |   23 ++++-
>  tools/perf/perf.c                      |   24 +++++
>  tools/perf/perf.h                      |    2 +
>  tools/perf/util/hist.c                 |   24 ++++-
>  tools/perf/util/hist.h                 |   11 ++-
>  tools/perf/util/sort.h                 |    1 +
>  tools/perf/util/ui/browser.c           |  176 ++++++++++++++++++++------------
>  tools/perf/util/ui/browser.h           |    9 +-
>  tools/perf/util/ui/browsers/annotate.c |   17 +--
>  tools/perf/util/ui/browsers/hists.c    |   55 +++-------
>  tools/perf/util/ui/browsers/map.c      |    6 +-
>  tools/perf/util/ui/helpline.h          |    2 +
>  12 files changed, 216 insertions(+), 134 deletions(-)

Hm, got a segfault with a plain 'perf top' on a 3.0-0.rc7.git3-ish 
box:

 ./perf top
 Segmentation fault (core dumped)

It took about 20 attempts to reproduce the segfault, and it always 
occured right after the first refresh (which, unlike the working 
cases showed zero samples):

Program received signal SIGSEGV, Segmentation fault.                                                    
[Switching to Thread 0x7ffff13a3700 (LWP 21657)]
ui_browser__hists_seek (whence=<optimized out>, offset=0, 
self=0x7fffec0008c0)
    at util/ui/browsers/hists.c:682
682		h->row_offset = 0;
(gdb) 

(gdb) bt
#0  ui_browser__hists_seek (whence=<optimized out>, offset=0, self=0x7fffec0008c0)
    at util/ui/browsers/hists.c:682
#1  ui_browser__hists_seek (self=0x7fffec0008c0, offset=0, whence=<optimized out>)
    at util/ui/browsers/hists.c:652
#2  0x0000000000472ab5 in hist_browser__run (delay_secs=2, arg=0x7602a0, 
    timer=0x420cc0 <perf_top__sort_new_samples>, ev_name=0x8f5dd0 "cycles", self=0x7fffec0008c0)
    at util/ui/browsers/hists.c:324
#3  perf_evsel__hists_browse (evsel=0x8f5b50, nr_events=1, helpline=<optimized out>, 
    ev_name=0x8f5dd0 "cycles", left_exits=false, timer=0x420cc0 <perf_top__sort_new_samples>, 
    arg=0x7602a0, delay_secs=2) at util/ui/browsers/hists.c:850
#4  0x0000000000473554 in perf_evlist__tui_browse_hists (evlist=0x8c4460, 
    help=0x50ab20 "For a higher level overview, try: perf top --sort comm,dso", 
    timer=0x420cc0 <perf_top__sort_new_samples>, arg=0x7602a0, delay_secs=2)
    at util/ui/browsers/hists.c:1178
#5  0x0000000000420ea3 in display_thread_tui (arg=<optimized out>) at builtin-top.c:567
#6  0x00000035f5807d31 in start_thread () from /lib64/libpthread.so.0
#7  0x00000035f54efdfd in clone () from /lib64/libc.so.6

(gdb) i r
rax            0x9118b8	9509048
rbx            0x7fffec0008c0	140737152813248
rcx            0x0	0
rdx            0x1	1
rsi            0x0	0
rdi            0x8f5bf0	9395184
rbp            0x7ffff13a2b90	0x7ffff13a2b90
rsp            0x7ffff13a2b70	0x7ffff13a2b70
r8             0x0	0
r9             0x0	0
r10            0xe6fe58	15138392
r11            0x0	0
r12            0x0	0
r13            0x2	2
r14            0x50ab20	5286688
r15            0x7602a0	7733920
rip            0x470f80	0x470f80 <ui_browser__hists_seek+128>
eflags         0x10246	[ PF ZF IF RF ]
cs             0x33	51
ss             0x2b	43
ds             0x0	0
es             0x0	0
fs             0x0	0
gs             0x0	0

The tree i pulled was 18eaf0b8e60a.

Btw., a side note: could we try to reset the console on segfaults and 
similar crashes? TUI crashes tend to leave the console in a messed up 
(blue, etc.) state.

Thanks,

	Ingo

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

* Re: [GIT PULL 0/6] perf/core fixes and improvements
  2011-10-14  7:16 ` [GIT PULL 0/6] perf/core fixes and improvements Ingo Molnar
@ 2011-10-14 15:07   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-10-14 15:07 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, David Ahern, Frederic Weisbecker, Mike Galbraith,
	Paul Mackerras, Peter Zijlstra, Stephane Eranian

Em Fri, Oct 14, 2011 at 09:16:10AM +0200, Ingo Molnar escreveu:
> Hm, got a segfault with a plain 'perf top' on a 3.0-0.rc7.git3-ish 
> box:
> 
>  ./perf top
>  Segmentation fault (core dumped)
> 
> It took about 20 attempts to reproduce the segfault, and it always 
> occured right after the first refresh (which, unlike the working 
> cases showed zero samples):
> 
> Program received signal SIGSEGV, Segmentation fault.                                                    
> [Switching to Thread 0x7ffff13a3700 (LWP 21657)]
> ui_browser__hists_seek (whence=<optimized out>, offset=0, 
> self=0x7fffec0008c0)
>     at util/ui/browsers/hists.c:682
> 682		h->row_offset = 0;
> (gdb) 
> 
> (gdb) bt
> #0  ui_browser__hists_seek (whence=<optimized out>, offset=0, self=0x7fffec0008c0)

browser->top was NULL at that point, it was only being initialized in
the refresh routine, fixed in my tree, please re-pull.

One can easily trigger it by specifying a seldom ocurring event, which
will start the browser without samples, then generating such event, I
used net:netif_rx + ping on my workstation.

> Btw., a side note: could we try to reset the console on segfaults and 
> similar crashes? TUI crashes tend to leave the console in a messed up 
> (blue, etc.) state.

I'll work on that, and also on allowing changing the browser colors,
say, to look like the stdio one.

- Arnaldo

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

end of thread, other threads:[~2011-10-14 15:08 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-13 15:08 [GIT PULL 0/6] perf/core fixes and improvements Arnaldo Carvalho de Melo
2011-10-13 15:08 ` [PATCH 1/6] perf hists browser: Recalculate browser pointers after resort/decay Arnaldo Carvalho de Melo
2011-10-13 15:08 ` [PATCH 2/6] perf hists: Don't free decayed entries if in the annotation browser Arnaldo Carvalho de Melo
2011-10-13 15:08 ` [PATCH 3/6] perf hists: Fix compilation when NO_NEWT_SUPPORT is set Arnaldo Carvalho de Melo
2011-10-13 15:08 ` [PATCH 4/6] perf ui browser: Handle SIGWINCH Arnaldo Carvalho de Melo
2011-10-13 15:08 ` [PATCH 5/6] perf ui browser: Remove ui_browser__add_exit_keys Arnaldo Carvalho de Melo
2011-10-13 15:08 ` [PATCH 6/6] perf top: Remove entries from entries_collapsed on decay Arnaldo Carvalho de Melo
2011-10-14  7:16 ` [GIT PULL 0/6] perf/core fixes and improvements Ingo Molnar
2011-10-14 15:07   ` Arnaldo Carvalho de Melo
  -- strict thread matches above, loose matches on Subject: below --
2011-08-16 14:41 Arnaldo Carvalho de Melo
2011-08-17 21:36 ` Ingo Molnar
2011-08-18 12:43   ` Arnaldo Carvalho de Melo

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