Skip to contentSkip to conversation

perf: Finish sampling commands when events are closed

RFC In review Jiri Olsa 5 participants 33 messages Aug 11, 2014 – Aug 24, 2014 perf events

lore
Share & tools
lore

Jiri Olsa

author reviewer
#0 lore raw

cover perf: Finish sampling commands when events are closed

hi,
adding support to quit sampling commands:
record,top,trace,kvm stat live

when all the monitored events are closed, like following perf command will now exit when monitored process (pid 1234) exits:

  $ perf record -p 1234

I added independent poller object to handle basic polling tasks. I had to factor some parts, so sending this as RFC, because I'm pretty sure I broke something else ;-)

The patch#1 is kernel change, which is needed for the patchset to work properly. The current behavior stays without this kernel change.

The code is based on latest Arnaldo's perf/core and following kernel change:
http://marc.info/?l=linux-kernel&m=140715910417130&w=2

The patchset is also reachable in here:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/core_poll

thanks for comments,
jirka

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>

---
Jiri Olsa (20):
perf: Add PERF_EVENT_STATE_EXIT state for events with exited task perf tools: Add poller object to handle polling globaly perf tests: Add poller object test
perf tools: Add support to traverse xyarrays
perf tools: Introduce perf_evsel__fd function
perf tools: Add evlist/evsel poller object support perf record: Add support to see event's ERR and HUP poll errors perf tools: Add set_term_quiet_input helper function perf tui browser: Add interface to terminate browser from uotside perf top: Add support to see event's ERR and HUP poll errors perf top: Join the display thread on exit
perf top: Use set_term_quiet_input for terminal input perf top: Setup signals for terminal output
perf top: Use poller object for display thread stdin perf kvm: Fix stdin handling for 'kvm stat live' command perf kvm: Add support to see event's ERR and HUP poll errors perf trace: Add support to see event's ERR and HUP poll errors perf python: Use poller object for polling
perf tests: Use poller object for polling
perf tools: Remove pollfd stuff out of evlist object

 include/linux/perf_event.h                |   1 +
 kernel/events/core.c                      |  12 ++++-
 tools/perf/Makefile.perf                  |   4 ++
 tools/perf/builtin-kvm.c                  | 123 +++++++++++++++++++++++------------------------
 tools/perf/builtin-record.c               |  29 ++++++++++-
 tools/perf/builtin-report.c               |   3 +-
 tools/perf/builtin-top.c                  | 104 ++++++++++++++++++++++++++--------------
 tools/perf/builtin-trace.c                |  25 +++++++++-
 tools/perf/tests/builtin-test.c           |   8 ++++
 tools/perf/tests/open-syscall-tp-fields.c |   9 +++-
 tools/perf/tests/perf-record.c            |   4 --
 tools/perf/tests/poller.c                 | 126 ++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/task-exit.c              |   9 +++-
 tools/perf/tests/tests.h                  |   2 +
 tools/perf/tests/xyarray.c                |  33 +++++++++++++
 tools/perf/ui/browser.c                   |   4 +-
 tools/perf/ui/browser.h                   |   8 +++-
 tools/perf/ui/browsers/annotate.c         |   2 +-
 tools/perf/ui/browsers/header.c           |   2 +-
 tools/perf/ui/browsers/hists.c            |  39 ++++++++-------
 tools/perf/ui/browsers/map.c              |   2 +-
 tools/perf/ui/browsers/scripts.c          |   2 +-
 tools/perf/ui/keysyms.h                   |   1 +
 tools/perf/ui/tui/util.c                  |   2 +-
 tools/perf/util/evlist.c                  |  38 ++++++---------
 tools/perf/util/evlist.h                  |   7 ++-
 tools/perf/util/evsel.c                   |  21 +++++++-
 tools/perf/util/evsel.h                   |  10 ++++
 tools/perf/util/hist.h                    |   6 ++-
 tools/perf/util/kvm-stat.h                |   1 -
 tools/perf/util/poller.c                  | 233 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/poller.h                  |  88 ++++++++++++++++++++++++++++++++++
 tools/perf/util/python-ext-sources        |   2 +
 tools/perf/util/python.c                  |  19 ++++++--
 tools/perf/util/util.c                    |  13 +++++
 tools/perf/util/util.h                    |   2 +
 tools/perf/util/xyarray.c                 |   4 +-
 tools/perf/util/xyarray.h                 |   6 +++
 38 files changed, 832 insertions(+), 172 deletions(-)
 create mode 100644 tools/perf/tests/poller.c
 create mode 100644 tools/perf/tests/xyarray.c
 create mode 100644 tools/perf/util/poller.c
 create mode 100644 tools/perf/util/poller.h
author reviewer
#1 lore raw

PATCH 1/20 perf: Add PERF_EVENT_STATE_EXIT state for events with exited task

Adding new perf event state to indicate that the monitored task has exited. In this case the event stays alive until the owner task exits or close the event fd while providing the last data through the read syscall and ring buffer.

Instead it needs to propagate the error info (monitored task has died) via poll and read syscalls by returning POLLHUP and 0 respectively.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 include/linux/perf_event.h |  1 +
 kernel/events/core.c       | 12 ++++++++++--
 2 files changed, 11 insertions(+), 2 deletions(-)
include/linux/perf_event.h +1 -0
index 707617a8c0f6..54f3a6241386 100644
@@ -268,6 +268,7 @@ struct pmu {
  * enum perf_event_active_state - the states of a event
  */
 enum perf_event_active_state {
+	PERF_EVENT_STATE_EXIT		= -3,
 	PERF_EVENT_STATE_ERROR		= -2,
 	PERF_EVENT_STATE_OFF		= -1,
 	PERF_EVENT_STATE_INACTIVE	=  0,
kernel/events/core.c +10 -2
index 14086e45c5c4..dde0eefa410a 100644
@@ -3499,7 +3499,8 @@ perf_read_hw(struct perf_event *event, char __user *buf, size_t count)
 	 * error state (i.e. because it was pinned but it couldn't be
 	 * scheduled on to the CPU at some point).
 	 */
-	if (event->state == PERF_EVENT_STATE_ERROR)
+	if ((event->state == PERF_EVENT_STATE_ERROR) ||
+	    (event->state == PERF_EVENT_STATE_EXIT))
 		return 0;
 
 	if (count < event->read_size)
@@ -3526,9 +3527,13 @@ static unsigned int perf_poll(struct file *file, poll_table *wait)
 {
 	struct perf_event *event = file->private_data;
 	struct ring_buffer *rb;
-	unsigned int events = POLL_HUP;
+	unsigned int events = POLLHUP;
 
 	poll_wait(file, &event->waitq, wait);
+
+	if (event->state == PERF_EVENT_STATE_EXIT)
+		return POLLHUP;
+
 	/*
 	 * Pin the event->rb by taking event->mmap_mutex; otherwise
 	 * perf_event_set_output() can swizzle our rb and make us miss wakeups.
@@ -7484,6 +7489,9 @@ __perf_event_exit_task(struct perf_event *child_event,
 	if (child_event->parent) {
 		sync_child_event(child_event, child);
 		free_event(child_event);
+	} else {
+		child_event->state = PERF_EVENT_STATE_EXIT;
+		perf_event_wakeup(child_event);
 	}
 }
 
Signature · 1 line
1.8.3.1
maintainer
#2 lore raw

Re: [PATCH 01/20] perf: Add PERF_EVENT_STATE_EXIT state for events with exited task

Quoted 32 lines · Jiri Olsa
---
 include/linux/perf_event.h |  1 +
 kernel/events/core.c       | 12 ++++++++++--
 2 files changed, 11 insertions(+), 2 deletions(-)
include/linux/perf_event.h +1 -0
index 707617a8c0f6..54f3a6241386 100644
@@ -268,6 +268,7 @@ struct pmu {
  * enum perf_event_active_state - the states of a event
  */
 enum perf_event_active_state {
+	PERF_EVENT_STATE_EXIT		= -3,
 	PERF_EVENT_STATE_ERROR		= -2,
 	PERF_EVENT_STATE_OFF		= -1,
 	PERF_EVENT_STATE_INACTIVE	=  0,
kernel/events/core.c +2 -1
index 14086e45c5c4..dde0eefa410a 100644
@@ -3499,7 +3499,8 @@ perf_read_hw(struct perf_event *event, char __user *buf, size_t count)
 	 * error state (i.e. because it was pinned but it couldn't be
 	 * scheduled on to the CPU at some point).
 	 */
-	if (event->state == PERF_EVENT_STATE_ERROR)
+	if ((event->state == PERF_EVENT_STATE_ERROR) ||
+	    (event->state == PERF_EVENT_STATE_EXIT))
 		return 0;
 
 	if (count < event->read_size)

kernel/events/core.c static unsigned int perf_poll(struct file *file, poll_table *wait) · line 3530

Quoted 5 lines
kernel/events/core.c +1 -1
@@ -3526,9 +3527,13 @@ static unsigned int perf_poll(struct file *file, poll_table *wait)
 {
 	struct perf_event *event = file->private_data;
 	struct ring_buffer *rb;
-	unsigned int events = POLL_HUP;
+	unsigned int events = POLLHUP;

Should not that be an independent bugfix? It is a silly little thing indeed, but it does change behaviour.

kernel/events/core.c static unsigned int perf_poll(struct file *file, poll_table *wait) · line 3536

Quoted 5 lines
kernel/events/core.c +4 -0
 	poll_wait(file, &event->waitq, wait);
+
+	if (event->state == PERF_EVENT_STATE_EXIT)
+		return POLLHUP;
+

So, seeing how events is already POLLHUP here, why not return that?

kernel/events/core.c __perf_event_exit_task(struct perf_event *child_event, · line 7496

Quoted 12 lines
kernel/events/core.c +3 -0
 	/*
 	 * Pin the event->rb by taking event->mmap_mutex; otherwise
 	 * perf_event_set_output() can swizzle our rb and make us miss wakeups.
@@ -7484,6 +7489,9 @@ __perf_event_exit_task(struct perf_event *child_event,
 	if (child_event->parent) {
 		sync_child_event(child_event, child);
 		free_event(child_event);
+	} else {
+		child_event->state = PERF_EVENT_STATE_EXIT;
+		perf_event_wakeup(child_event);
 	}
 }

In any case, ACK on this patch, I'll assume you want to take the lot through acme seeing how its mostly tools bits.

#3 lore raw

Re: [PATCH 01/20] perf: Add PERF_EVENT_STATE_EXIT state for events with exited task

On Mon, Aug 11, 2014 at 02:01:02PM +0200, Peter Zijlstra wrote:

SNIP

Quoted 7 lines

kernel/events/core.c static unsigned int perf_poll(struct file *file, poll_table *wait) · line 3530

Quoted 4 lines
kernel/events/core.c +1 -1
 	struct perf_event *event = file->private_data;
 	struct ring_buffer *rb;
-	unsigned int events = POLL_HUP;
+	unsigned int events = POLLHUP;

Should not that be an independent bugfix? It is a silly little thing indeed, but it does change behaviour.

ok, I'll send it in a separate patch plus your suggestion below in v2

Quoted 9 lines

kernel/events/core.c static unsigned int perf_poll(struct file *file, poll_table *wait) · line 3536

Quoted 6 lines
kernel/events/core.c +4 -0
 	poll_wait(file, &event->waitq, wait);
+
+	if (event->state == PERF_EVENT_STATE_EXIT)
+		return POLLHUP;
+

So, seeing how events is already POLLHUP here, why not return that?

Quoted 15 lines

kernel/events/core.c __perf_event_exit_task(struct perf_event *child_event, · line 7496

Quoted 12 lines
kernel/events/core.c +3 -0
 	/*
 	 * Pin the event->rb by taking event->mmap_mutex; otherwise
 	 * perf_event_set_output() can swizzle our rb and make us miss wakeups.
@@ -7484,6 +7489,9 @@ __perf_event_exit_task(struct perf_event *child_event,
 	if (child_event->parent) {
 		sync_child_event(child_event, child);
 		free_event(child_event);
+	} else {
+		child_event->state = PERF_EVENT_STATE_EXIT;
+		perf_event_wakeup(child_event);
 	}
 }

In any case, ACK on this patch, I'll assume you want to take the lot through acme seeing how its mostly tools bits.

yep

thanks,
jirka

#4 lore raw

[tip:perf/core] perf: Add PERF_EVENT_STATE_EXIT state for events with exited task

In reply to Peter Zijlstra
perf: Add PERF_EVENT_STATE_EXIT state for events with exited task
Commit-ID:  179033b3e064d2cd3f5f9945e76b0a0f0fbf4883
Gitweb:     http://git.kernel.org/tip/179033b3e064d2cd3f5f9945e76b0a0f0fbf4883
Author:     Jiri Olsa <[email protected]>
AuthorDate: Thu, 7 Aug 2014 11:48:26 -0400
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Sun, 24 Aug 2014 08:11:09 -0300

perf: Add PERF_EVENT_STATE_EXIT state for events with exited task

Adding new perf event state to indicate that the monitored task has exited. In this case the event stays alive until the owner task exits or close the event fd while providing the last data through the read syscall and ring buffer.

Instead it needs to propagate the error info (monitored task has died) via poll and read syscalls by returning POLLHUP and 0 respectively.

---
 include/linux/perf_event.h |  1 +
 kernel/events/core.c       | 10 +++++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)
include/linux/perf_event.h +1 -0
index f0a1036..893a0d0 100644
@@ -269,6 +269,7 @@ struct pmu {
  * enum perf_event_active_state - the states of a event
  */
 enum perf_event_active_state {
+	PERF_EVENT_STATE_EXIT		= -3,
 	PERF_EVENT_STATE_ERROR		= -2,
 	PERF_EVENT_STATE_OFF		= -1,
 	PERF_EVENT_STATE_INACTIVE	=  0,
kernel/events/core.c +9 -1
index 4575dd6..d8cb4d2 100644
@@ -3600,7 +3600,8 @@ perf_read_hw(struct perf_event *event, char __user *buf, size_t count)
 	 * error state (i.e. because it was pinned but it couldn't be
 	 * scheduled on to the CPU at some point).
 	 */
-	if (event->state == PERF_EVENT_STATE_ERROR)
+	if ((event->state == PERF_EVENT_STATE_ERROR) ||
+	    (event->state == PERF_EVENT_STATE_EXIT))
 		return 0;
 
 	if (count < event->read_size)
@@ -3630,6 +3631,10 @@ static unsigned int perf_poll(struct file *file, poll_table *wait)
 	unsigned int events = POLLHUP;
 
 	poll_wait(file, &event->waitq, wait);
+
+	if (event->state == PERF_EVENT_STATE_EXIT)
+		return events;
+
 	/*
 	 * Pin the event->rb by taking event->mmap_mutex; otherwise
 	 * perf_event_set_output() can swizzle our rb and make us miss wakeups.
@@ -7588,6 +7593,9 @@ __perf_event_exit_task(struct perf_event *child_event,
 	if (child_event->parent) {
 		sync_child_event(child_event, child);
 		free_event(child_event);
+	} else {
+		child_event->state = PERF_EVENT_STATE_EXIT;
+		perf_event_wakeup(child_event);
 	}
 }
 
#5 lore raw

[tip:perf/core] perf: Fix perf_poll to return proper POLLHUP value

In reply to Peter Zijlstra
perf: Fix perf_poll to return proper POLLHUP value
Commit-ID:  61b67684c4a4d04b30d9ed67aa2eadfa0089c590
Gitweb:     http://git.kernel.org/tip/61b67684c4a4d04b30d9ed67aa2eadfa0089c590
Author:     Jiri Olsa <[email protected]>
AuthorDate: Wed, 13 Aug 2014 19:39:56 +0200
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Sun, 24 Aug 2014 08:10:55 -0300

perf: Fix perf_poll to return proper POLLHUP value

Currently perf_poll returns POLL_HUP in case of error, which is wrong, because poll syscall expects POLLHUP. The POLL_HUP is meant to be used for SIGIO state.

---
 kernel/events/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/events/core.c +1 -1
index 2d7363a..4575dd6 100644
@@ -3627,7 +3627,7 @@ static unsigned int perf_poll(struct file *file, poll_table *wait)
 {
 	struct perf_event *event = file->private_data;
 	struct ring_buffer *rb;
-	unsigned int events = POLL_HUP;
+	unsigned int events = POLLHUP;
 
 	poll_wait(file, &event->waitq, wait);
 	/*
author reviewer
#6 lore raw

PATCH 2/20 perf tools: Add poller object to handle polling globaly

In reply to Jiri Olsa

Adding poller object to centralize polling processing and to have independent polling object, that could gather and process multiple file descriptors from different sources. (like perf events, timers or standard input..)

The interface functions are documented in the poller.c and usage is explained in the poller.h.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/Makefile.perf |   2 +
 tools/perf/util/poller.c | 233 +++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/poller.h |  88 ++++++++++++++++++
 3 files changed, 323 insertions(+)
 create mode 100644 tools/perf/util/poller.c
 create mode 100644 tools/perf/util/poller.h
tools/perf/Makefile.perf +2 -0
index 1ea31e275b4d..3ef50d35f8bf 100644
@@ -303,6 +303,7 @@ LIB_H += ui/util.h
 LIB_H += ui/ui.h
 LIB_H += util/data.h
 LIB_H += util/kvm-stat.h
+LIB_H += util/poller.h
 
 LIB_OBJS += $(OUTPUT)util/abspath.o
 LIB_OBJS += $(OUTPUT)util/alias.o
@@ -379,6 +380,7 @@ LIB_OBJS += $(OUTPUT)util/srcline.o
 LIB_OBJS += $(OUTPUT)util/data.o
 LIB_OBJS += $(OUTPUT)util/tsc.o
 LIB_OBJS += $(OUTPUT)util/cloexec.o
+LIB_OBJS += $(OUTPUT)util/poller.o
 
 LIB_OBJS += $(OUTPUT)ui/setup.o
 LIB_OBJS += $(OUTPUT)ui/helpline.o
tools/perf/util/poller.c new file +233 -0

233 lines not on this page · raw message

tools/perf/util/poller.h new file +88 -0

88 lines not on this page · raw message

Signature · 1 line
1.8.3.1
author reviewer
#7 lore raw

PATCH 3/20 perf tests: Add poller object test

In reply to Jiri Olsa

Adding some basic automated tests for poller object.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/Makefile.perf        |   1 +
 tools/perf/tests/builtin-test.c |   4 ++
 tools/perf/tests/poller.c       | 126 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/tests.h        |   1 +
 4 files changed, 132 insertions(+)
 create mode 100644 tools/perf/tests/poller.c
tools/perf/Makefile.perf +1 -0
index 3ef50d35f8bf..69d90285a994 100644
@@ -427,6 +427,7 @@ endif
 endif
 LIB_OBJS += $(OUTPUT)tests/mmap-thread-lookup.o
 LIB_OBJS += $(OUTPUT)tests/thread-mg-share.o
+LIB_OBJS += $(OUTPUT)tests/poller.o
 
 BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
 BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
tools/perf/tests/builtin-test.c +4 -0
index 6f8b01bc6033..e2e3827ebd17 100644
@@ -154,6 +154,10 @@ static struct test {
 		.func = test__hists_cumulate,
 	},
 	{
+		.desc = "Test poller",
+		.func = test__poller,
+	},
+	{
 		.func = NULL,
 	},
 };
tools/perf/tests/poller.c new file +126 -0

126 lines not on this page · raw message

tools/perf/tests/tests.h +1 -0
index ed64790a395f..e2a76d509644 100644
@@ -48,6 +48,7 @@ int test__mmap_thread_lookup(void);
 int test__thread_mg_share(void);
 int test__hists_output(void);
 int test__hists_cumulate(void);
+int test__poller(void);
 
 #if defined(__x86_64__) || defined(__i386__) || defined(__arm__)
 #ifdef HAVE_DWARF_UNWIND_SUPPORT
Signature · 1 line
1.8.3.1
author reviewer
#8 lore raw

PATCH 4/20 perf tools: Add support to traverse xyarrays

In reply to Jiri Olsa

Adding xyarray__for_each define to allow sequentially traverse xyarrays. It will be handy in following patch.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/Makefile.perf        |  1 +
 tools/perf/tests/builtin-test.c |  4 ++++
 tools/perf/tests/tests.h        |  1 +
 tools/perf/tests/xyarray.c      | 33 +++++++++++++++++++++++++++++++++
 tools/perf/util/xyarray.c       |  4 +++-
 tools/perf/util/xyarray.h       |  6 ++++++
 6 files changed, 48 insertions(+), 1 deletion(-)
 create mode 100644 tools/perf/tests/xyarray.c
tools/perf/Makefile.perf +1 -0
index 69d90285a994..0b478a372494 100644
@@ -428,6 +428,7 @@ endif
 LIB_OBJS += $(OUTPUT)tests/mmap-thread-lookup.o
 LIB_OBJS += $(OUTPUT)tests/thread-mg-share.o
 LIB_OBJS += $(OUTPUT)tests/poller.o
+LIB_OBJS += $(OUTPUT)tests/xyarray.o
 
 BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
 BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
tools/perf/tests/builtin-test.c +4 -0
index e2e3827ebd17..2da8c86599cd 100644
@@ -158,6 +158,10 @@ static struct test {
 		.func = test__poller,
 	},
 	{
+		.desc = "Test xyarray",
+		.func = test__xyarray,
+	},
+	{
 		.func = NULL,
 	},
 };
tools/perf/tests/tests.h +1 -0
index e2a76d509644..7b726578ff0d 100644
@@ -49,6 +49,7 @@ int test__thread_mg_share(void);
 int test__hists_output(void);
 int test__hists_cumulate(void);
 int test__poller(void);
+int test__xyarray(void);
 
 #if defined(__x86_64__) || defined(__i386__) || defined(__arm__)
 #ifdef HAVE_DWARF_UNWIND_SUPPORT
tools/perf/tests/xyarray.c new file +33 -0
new file mode 100644
index 000000000000..e1a1d6a45106
@@ -0,0 +1,33 @@
+#include "tests.h"
+#include "xyarray.h"
+#include "debug.h"
+
+struct krava {
+	int a;
+};
+
+#define X 100
+#define Y 100
+
+int test__xyarray(void)
+{
+	struct xyarray *a;
+	struct krava *k;
+	int x, y;
+
+	a = xyarray__new(X, Y, sizeof(struct krava));
+	TEST_ASSERT_VAL("failed to allocate xyarray", a);
+
+	for (x = 0; x < X; x++) {
+		for (y = 0; y < Y; y++) {
+			k = xyarray__entry(a, x, y);
+			k->a = x * X + y;
+		}
+	}
+
+	y = 0;
+	xyarray__for_each(a, k)
+		TEST_ASSERT_VAL("wrong array value", k->a == y++);
+
+	return 0;
+}
tools/perf/util/xyarray.c +3 -1
index 22afbf6c536a..077e8240fe98 100644
@@ -4,11 +4,13 @@
 struct xyarray *xyarray__new(int xlen, int ylen, size_t entry_size)
 {
 	size_t row_size = ylen * entry_size;
-	struct xyarray *xy = zalloc(sizeof(*xy) + xlen * row_size);
+	size_t size = xlen * row_size;
+	struct xyarray *xy = zalloc(sizeof(*xy) + size);
 
 	if (xy != NULL) {
 		xy->entry_size = entry_size;
 		xy->row_size   = row_size;
+		xy->size       = size;
 	}
 
 	return xy;
tools/perf/util/xyarray.h +6 -0
index c488a07275dd..e4efa075fd76 100644
@@ -6,6 +6,7 @@
 struct xyarray {
 	size_t row_size;
 	size_t entry_size;
+	size_t size;
 	char contents[];
 };
 
@@ -17,4 +18,9 @@ static inline void *xyarray__entry(struct xyarray *xy, int x, int y)
 	return &xy->contents[x * xy->row_size + y * xy->entry_size];
 }
 
+#define xyarray__for_each(array, entry)					\
+	for (entry = (void *) &array->contents[0];			\
+	     (void *) entry < ((void *) array->contents + array->size);	\
+	     entry++)
+
 #endif /* _PERF_XYARRAY_H_ */
Signature · 1 line
1.8.3.1
author reviewer
#9 lore raw

PATCH 5/20 perf tools: Introduce perf_evsel__fd function

In reply to Jiri Olsa

I'm about to make the fd entry more complex, so adding perf_evsel__fd helper to keep the FD macro sane.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/util/evlist.c | 2 +-
 tools/perf/util/evsel.c  | 2 +-
 tools/perf/util/evsel.h  | 5 +++++
 3 files changed, 7 insertions(+), 2 deletions(-)
tools/perf/util/evlist.c +1 -1
index 3b366c085021..4e0f0670b655 100644
@@ -25,7 +25,7 @@
 #include <linux/bitops.h>
 #include <linux/hash.h>
 
-#define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
+#define FD(e, x, y) (*perf_evsel__fd(e, x, y))
 #define SID(e, x, y) xyarray__entry(e->sample_id, x, y)
 
 void perf_evlist__init(struct perf_evlist *evlist, struct cpu_map *cpus,
tools/perf/util/evsel.c +1 -1
index 0c8919decac8..1ec1a7ec1cfb 100644
@@ -32,7 +32,7 @@ static struct {
 	bool cloexec;
 } perf_missing_features;
 
-#define FD(e, x, y) (*(int *)xyarray__entry(e->fd, x, y))
+#define FD(e, x, y) (*perf_evsel__fd(e, x, y))
 
 int __perf_evsel__sample_size(u64 sample_type)
 {
tools/perf/util/evsel.h +5 -0
index d7f93ce0ebc1..fe8794fa73ed 100644
@@ -364,4 +364,9 @@ for ((_evsel) = list_entry((_leader)->node.next, struct perf_evsel, node); 	\
      (_evsel) && (_evsel)->leader == (_leader);					\
      (_evsel) = list_entry((_evsel)->node.next, struct perf_evsel, node))
 
+static inline int *perf_evsel__fd(struct perf_evsel *evsel, int cpu, int thread)
+{
+	return (int *) xyarray__entry(evsel->fd, cpu, thread);
+}
+
 #endif /* __PERF_EVSEL_H */
Signature · 1 line
1.8.3.1
author reviewer
#10 lore raw

PATCH 6/20 perf tools: Add evlist/evsel poller object support

In reply to Jiri Olsa

Changing perf_evsel fd (xyarray) entry to carry poller_item object. The reason for this is to source polling data with file descriptor, because we need them both.

Adding following helper functions to go through the event's descriptors and initialize poller object:

perf_evlist__set_poller
perf_evsel__set_poller

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/util/evlist.c | 14 ++++++++++++++
 tools/perf/util/evlist.h |  3 +++
 tools/perf/util/evsel.c  | 19 ++++++++++++++++++-
 tools/perf/util/evsel.h  |  7 ++++++-
 4 files changed, 41 insertions(+), 2 deletions(-)
tools/perf/util/evlist.c +14 -0
index 4e0f0670b655..a4c381c10c72 100644
@@ -1266,3 +1266,17 @@ void perf_evlist__to_front(struct perf_evlist *evlist,
 
 	list_splice(&move, &evlist->entries);
 }
+
+int perf_evlist__set_poller(struct perf_evlist *evlist)
+{
+	struct perf_evsel *evsel;
+	int err = -1;
+
+	evlist__for_each(evlist, evsel) {
+		err = perf_evsel__set_poller(evsel, &evlist->poller);
+		if (err)
+			break;
+	}
+
+	return err;
+}
tools/perf/util/evlist.h +3 -0
index f5173cd63693..7d03af1ce4f7 100644
@@ -45,6 +45,7 @@ struct perf_evlist {
 	struct thread_map *threads;
 	struct cpu_map	  *cpus;
 	struct perf_evsel *selected;
+	struct poller	 poller;
 };
 
 struct perf_evsel_str_handler {
@@ -196,6 +197,8 @@ bool perf_evlist__can_select_event(struct perf_evlist *evlist, const char *str);
 void perf_evlist__to_front(struct perf_evlist *evlist,
 			   struct perf_evsel *move_evsel);
 
+int perf_evlist__set_poller(struct perf_evlist *evlist);
+
 /**
  * __evlist__for_each - iterate thru all the evsels
  * @list: list_head instance to iterate
tools/perf/util/evsel.c +18 -1
index 1ec1a7ec1cfb..580061d16e4a 100644
@@ -695,7 +695,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
 int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
 {
 	int cpu, thread;
-	evsel->fd = xyarray__new(ncpus, nthreads, sizeof(int));
+	evsel->fd = xyarray__new(ncpus, nthreads, sizeof(struct poller_item));
 
 	if (evsel->fd) {
 		for (cpu = 0; cpu < ncpus; cpu++) {
@@ -2055,3 +2055,20 @@ int perf_evsel__open_strerror(struct perf_evsel *evsel, struct target *target,
 	"No CONFIG_PERF_EVENTS=y kernel support configured?\n",
 			 err, strerror(err), perf_evsel__name(evsel));
 }
+
+int perf_evsel__set_poller(struct perf_evsel *evsel, struct poller *poller)
+{
+	struct poller_item *p;
+	int err = -1;
+
+	xyarray__for_each(evsel->fd, p) {
+		if (p->fd == -1)
+			continue;
+
+		err = poller__add(poller, p);
+		if (err)
+			break;
+	}
+
+	return err;
+}
tools/perf/util/evsel.h +6 -1
index fe8794fa73ed..4c6cd9185710 100644
@@ -10,6 +10,7 @@
 #include "cgroup.h"
 #include "hist.h"
 #include "symbol.h"
+#include "poller.h"
 
 struct perf_counts_values {
 	union {
@@ -366,7 +367,11 @@ for ((_evsel) = list_entry((_leader)->node.next, struct perf_evsel, node); 	\
 
 static inline int *perf_evsel__fd(struct perf_evsel *evsel, int cpu, int thread)
 {
-	return (int *) xyarray__entry(evsel->fd, cpu, thread);
+	struct poller_item *p;
+
+	p = xyarray__entry(evsel->fd, cpu, thread);
+	return &p->fd;
 }
 
+int perf_evsel__set_poller(struct perf_evsel *evsel, struct poller *poller);
 #endif /* __PERF_EVSEL_H */
Signature · 1 line
1.8.3.1
author reviewer
#11 lore raw

PATCH 7/20 perf record: Add support to see event's ERR and HUP poll errors

In reply to Jiri Olsa

Adding record command support to see when all events are down and forcing it to stop in that case.

Like following record session will stop if the monitored pid (1234) exits:

  $ perf record -p 1234
Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/builtin-record.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)
tools/perf/builtin-record.c +28 -1
index ca0251e8470d..dfcc470a9b4a 100644
@@ -25,6 +25,7 @@
 #include "util/cpumap.h"
 #include "util/thread_map.h"
 #include "util/data.h"
+#include "util/poller.h"
 
 #include <unistd.h>
 #include <sched.h>
@@ -157,6 +158,13 @@ try_again:
 			ui__error("%s\n", msg);
 			goto out;
 		}
+
+		if (perf_evsel__set_poller(pos, &evlist->poller)) {
+			error("failed to set poller with %d (%s)\n", errno,
+				strerror(errno));
+			rc = -1;
+			goto out;
+		}
 	}
 
 	if (perf_evlist__apply_filters(evlist)) {
@@ -296,6 +304,18 @@ static void workload_exec_failed_signal(int signo __maybe_unused,
 	child_finished = 1;
 }
 
+static int data_error(struct poller *p, struct poller_item *item)
+{
+	pr_debug("got HUP/ERROR on fd %d\n", item->fd);
+
+	poller__del(p, item);
+	if (poller__empty(p)) {
+		pr_debug("All events closed, shutting down.\n");
+		done = 1;
+	}
+	return 0;
+}
+
 static int __cmd_record(struct record *rec, int argc, const char **argv)
 {
 	int err;
@@ -308,6 +328,11 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	struct perf_data_file *file = &rec->file;
 	struct perf_session *session;
 	bool disabled = false;
+	struct poller *poller = &rec->evlist->poller;
+	struct poller_ops poller_ops = {
+		.error	= data_error,
+		.hup	= data_error,
+	};
 
 	rec->progname = argv[0];
 
@@ -447,6 +472,8 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 		perf_evlist__enable(rec->evlist);
 	}
 
+	poller__set_ops(poller, &poller_ops);
+
 	for (;;) {
 		int hits = rec->samples;
 
@@ -458,7 +485,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 		if (hits == rec->samples) {
 			if (done)
 				break;
-			err = poll(rec->evlist->pollfd, rec->evlist->nr_fds, -1);
+			err = poller__poll(poller, -1);
 			/*
 			 * Propagate error, only if there's any. Ignore positive
 			 * number of returned events and interrupt error.
Signature · 1 line
1.8.3.1
author reviewer
#12 lore raw

PATCH 8/20 perf tools: Add set_term_quiet_input helper function

In reply to Jiri Olsa

Adding set_term_quiet_input helper to set the terminal quiet. It will be used in following patches.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/util/util.c | 13 +++++++++++++
 tools/perf/util/util.h |  2 ++
 2 files changed, 15 insertions(+)
tools/perf/util/util.c +13 -0
index b82a93cb1694..25822bdf7bbf 100644
@@ -13,6 +13,7 @@
 #include <limits.h>
 #include <byteswap.h>
 #include <linux/kernel.h>
+#include <unistd.h>
 
 /*
  * XXX We need to find a better place for these things...
@@ -282,6 +283,18 @@ void get_term_dimensions(struct winsize *ws)
 	ws->ws_col = 80;
 }
 
+void set_term_quiet_input(struct termios *old)
+{
+	struct termios tc;
+
+	tcgetattr(0, old);
+	tc = *old;
+	tc.c_lflag &= ~(ICANON | ECHO);
+	tc.c_cc[VMIN] = 0;
+	tc.c_cc[VTIME] = 0;
+	tcsetattr(0, TCSANOW, &tc);
+}
+
 static void set_tracing_events_path(const char *mountpoint)
 {
 	snprintf(tracing_events_path, sizeof(tracing_events_path), "%s/%s",
tools/perf/util/util.h +2 -0
index 03a1ea2266b8..d6a79b1fb28c 100644
@@ -75,6 +75,7 @@
 #include <api/fs/debugfs.h>
 #include <termios.h>
 #include <linux/bitops.h>
+#include <termios.h>
 
 extern const char *graph_line;
 extern const char *graph_dotted_line;
@@ -308,6 +309,7 @@ extern unsigned int page_size;
 extern int cacheline_size;
 
 void get_term_dimensions(struct winsize *ws);
+void set_term_quiet_input(struct termios *old);
 
 struct parse_tag {
 	char tag;
Signature · 1 line
1.8.3.1
#13 lore raw

[tip:perf/core] perf tools: Introduce set_term_quiet_input helper function

perf tools: Introduce set_term_quiet_input helper function
Commit-ID:  9398c484f8abc8d287cb90f5a33dd43ac26f24ef
Gitweb:     http://git.kernel.org/tip/9398c484f8abc8d287cb90f5a33dd43ac26f24ef
Author:     Jiri Olsa <[email protected]>
AuthorDate: Mon, 11 Aug 2014 10:50:02 +0200
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 13 Aug 2014 11:14:22 -0300

perf tools: Introduce set_term_quiet_input helper function

Adding set_term_quiet_input helper to set the terminal quiet, out from 'perf top', used in following patches in 'perf kvm'.

Cc Adrian Hunter <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/builtin-top.c | 10 ++--------
 tools/perf/util/util.c   | 13 +++++++++++++
 tools/perf/util/util.h   |  2 ++
 3 files changed, 17 insertions(+), 8 deletions(-)
tools/perf/builtin-top.c +2 -8
index 10fc7e8..e8459e2 100644
@@ -580,20 +580,14 @@ static void *display_thread_tui(void *arg)
 static void *display_thread(void *arg)
 {
 	struct pollfd stdin_poll = { .fd = 0, .events = POLLIN };
-	struct termios tc, save;
+	struct termios save;
 	struct perf_top *top = arg;
 	int delay_msecs, c;
 
-	tcgetattr(0, &save);
-	tc = save;
-	tc.c_lflag &= ~(ICANON | ECHO);
-	tc.c_cc[VMIN] = 0;
-	tc.c_cc[VTIME] = 0;
-
 	pthread__unblock_sigwinch();
 repeat:
 	delay_msecs = top->delay_secs * 1000;
-	tcsetattr(0, TCSANOW, &tc);
+	set_term_quiet_input(&save);
 	/* trash return*/
 	getc(stdin);
 
tools/perf/util/util.c +13 -0
index b82a93c..25822bd 100644
@@ -13,6 +13,7 @@
 #include <limits.h>
 #include <byteswap.h>
 #include <linux/kernel.h>
+#include <unistd.h>
 
 /*
  * XXX We need to find a better place for these things...
@@ -282,6 +283,18 @@ void get_term_dimensions(struct winsize *ws)
 	ws->ws_col = 80;
 }
 
+void set_term_quiet_input(struct termios *old)
+{
+	struct termios tc;
+
+	tcgetattr(0, old);
+	tc = *old;
+	tc.c_lflag &= ~(ICANON | ECHO);
+	tc.c_cc[VMIN] = 0;
+	tc.c_cc[VTIME] = 0;
+	tcsetattr(0, TCSANOW, &tc);
+}
+
 static void set_tracing_events_path(const char *mountpoint)
 {
 	snprintf(tracing_events_path, sizeof(tracing_events_path), "%s/%s",
tools/perf/util/util.h +2 -0
index 03a1ea2..d6a79b1 100644
@@ -75,6 +75,7 @@
 #include <api/fs/debugfs.h>
 #include <termios.h>
 #include <linux/bitops.h>
+#include <termios.h>
 
 extern const char *graph_line;
 extern const char *graph_dotted_line;
@@ -308,6 +309,7 @@ extern unsigned int page_size;
 extern int cacheline_size;
 
 void get_term_dimensions(struct winsize *ws);
+void set_term_quiet_input(struct termios *old);
 
 struct parse_tag {
 	char tag;
author reviewer
#14 lore raw

PATCH 9/20 perf tui browser: Add interface to terminate browser from uotside

In reply to Jiri Olsa

Currently the TUI browser could be terminated only via keypress 'q' and interrupt. But there's another situation possible where we want to terminate the browser from outside like when all monitored events are closed.

Adding 'done' pointer parameter to TUI browser functions to allow a browser loops break when '*done != 0'.

Changing top TUI browser to use top's 'done' in here.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/builtin-report.c       |  3 ++-
 tools/perf/builtin-top.c          |  3 ++-
 tools/perf/ui/browser.c           |  4 ++--
 tools/perf/ui/browser.h           |  8 +++++++-
 tools/perf/ui/browsers/annotate.c |  2 +-
 tools/perf/ui/browsers/header.c   |  2 +-
 tools/perf/ui/browsers/hists.c    | 39 ++++++++++++++++++++++-----------------
 tools/perf/ui/browsers/map.c      |  2 +-
 tools/perf/ui/browsers/scripts.c  |  2 +-
 tools/perf/ui/keysyms.h           |  1 +
 tools/perf/ui/tui/util.c          |  2 +-
 tools/perf/util/hist.h            |  6 ++++--
 12 files changed, 45 insertions(+), 29 deletions(-)
tools/perf/builtin-report.c +2 -1
index 041e93da13e4..a5d8c9140eba 100644
@@ -400,7 +400,8 @@ static int report__browse_hists(struct report *rep)
 	case 1:
 		ret = perf_evlist__tui_browse_hists(evlist, help, NULL,
 						    rep->min_percent,
-						    &session->header.env);
+						    &session->header.env,
+						    NULL);
 		/*
 		 * Usually "ret" is the last pressed key, and we only
 		 * care if the key notifies us to switch data file.
tools/perf/builtin-top.c +2 -1
index 4fb6f726271c..4cd56c9ddedc 100644
@@ -571,7 +571,8 @@ static void *display_thread_tui(void *arg)
 		pos->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);
+				      &top->session->header.env,
+				      (int *) &done);
 
 	done = 1;
 	return NULL;
tools/perf/ui/browser.c +2 -2
index 6680fa5cb9dd..82d70c71f522 100644
@@ -352,11 +352,11 @@ 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 *browser, int delay_secs)
+int ui_browser__run(struct ui_browser *browser, int delay_secs, int *done)
 {
 	int err, key;
 
-	while (1) {
+	while (!browser_is_done(done)) {
 		off_t offset;
 
 		pthread_mutex_lock(&ui__lock);
tools/perf/ui/browser.h +7 -1
index 92ae72113965..9e9a4bf12f2a 100644
@@ -46,7 +46,7 @@ int ui_browser__show(struct ui_browser *browser, const char *title,
 		     const char *helpline, ...);
 void ui_browser__hide(struct ui_browser *browser);
 int ui_browser__refresh(struct ui_browser *browser);
-int ui_browser__run(struct ui_browser *browser, int delay_secs);
+int ui_browser__run(struct ui_browser *browser, int delay_secs, int *done);
 void ui_browser__update_nr_entries(struct ui_browser *browser, u32 nr_entries);
 void ui_browser__handle_resize(struct ui_browser *browser);
 void __ui_browser__vline(struct ui_browser *browser, unsigned int column,
@@ -72,4 +72,10 @@ unsigned int ui_browser__list_head_refresh(struct ui_browser *browser);
 
 void ui_browser__init(void);
 void annotate_browser__init(void);
+
+static inline bool browser_is_done(int *done)
+{
+	return done ? *done != 0 : 0;
+}
+
 #endif /* _PERF_UI_BROWSER_H_ */
tools/perf/ui/browsers/annotate.c +1 -1
index f0697a3aede0..e3271d76480d 100644
@@ -681,7 +681,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
 	nd = browser->curr_hot;
 
 	while (1) {
-		key = ui_browser__run(&browser->b, delay_secs);
+		key = ui_browser__run(&browser->b, delay_secs, NULL);
 
 		if (delay_secs != 0) {
 			annotate_browser__calc_percent(browser, evsel);
tools/perf/ui/browsers/header.c +1 -1
index 89c16b988618..931be7c5e91b 100644
@@ -42,7 +42,7 @@ static int list_menu__run(struct ui_browser *menu)
 		return -1;
 
 	while (1) {
-		key = ui_browser__run(menu, 0);
+		key = ui_browser__run(menu, 0, NULL);
 
 		switch (key) {
 		case K_RIGHT:
tools/perf/ui/browsers/hists.c +22 -17
index 045c1e16ac59..bb8beb8c3ad8 100644
@@ -378,9 +378,10 @@ static void ui_browser__warn_lost_events(struct ui_browser *browser)
 }
 
 static int hist_browser__run(struct hist_browser *browser,
-			     struct hist_browser_timer *hbt)
+			     struct hist_browser_timer *hbt,
+			     int *done)
 {
-	int key;
+	int key = K_INIT;
 	char title[160];
 	int delay_secs = hbt ? hbt->refresh : 0;
 
@@ -393,8 +394,8 @@ static int hist_browser__run(struct hist_browser *browser,
 			     "Press '?' for help on key bindings") < 0)
 		return -1;
 
-	while (1) {
-		key = ui_browser__run(&browser->b, delay_secs);
+	while (!browser_is_done(done)) {
+		key = ui_browser__run(&browser->b, delay_secs, done);
 
 		switch (key) {
 		case K_TIMER: {
@@ -1486,7 +1487,8 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 				    bool left_exits,
 				    struct hist_browser_timer *hbt,
 				    float min_pcnt,
-				    struct perf_session_env *env)
+				    struct perf_session_env *env,
+				    int *done)
 {
 	struct hists *hists = &evsel->hists;
 	struct hist_browser *browser = hist_browser__new(hists);
@@ -1554,7 +1556,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 	if (symbol_conf.col_width_list_str)
 		perf_hpp__set_user_width(symbol_conf.col_width_list_str);
 
-	while (1) {
+	while (!browser_is_done(done)) {
 		const struct thread *thread = NULL;
 		const struct dso *dso = NULL;
 		int choice = 0,
@@ -1565,7 +1567,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 
 		nr_options = 0;
 
-		key = hist_browser__run(browser, hbt);
+		key = hist_browser__run(browser, hbt, done);
 
 		if (browser->he_selection != NULL) {
 			thread = hist_browser__selected_thread(browser);
@@ -1939,20 +1941,21 @@ static void perf_evsel_menu__write(struct ui_browser *browser,
 
 static int perf_evsel_menu__run(struct perf_evsel_menu *menu,
 				int nr_events, const char *help,
-				struct hist_browser_timer *hbt)
+				struct hist_browser_timer *hbt,
+				int *done)
 {
 	struct perf_evlist *evlist = menu->b.priv;
 	struct perf_evsel *pos;
 	const char *title = "Available samples";
 	int delay_secs = hbt ? hbt->refresh : 0;
-	int key;
+	int key = K_INIT;
 
 	if (ui_browser__show(&menu->b, title,
 			     "ESC: exit, ENTER|->: Browse histograms") < 0)
 		return -1;
 
-	while (1) {
-		key = ui_browser__run(&menu->b, delay_secs);
+	while (!browser_is_done(done)) {
+		key = ui_browser__run(&menu->b, delay_secs, done);
 
 		switch (key) {
 		case K_TIMER:
@@ -1979,7 +1982,7 @@ browse_hists:
 			key = perf_evsel__hists_browse(pos, nr_events, help,
 						       true, hbt,
 						       menu->min_pcnt,
-						       menu->env);
+						       menu->env, done);
 			ui_browser__show_title(&menu->b, title);
 			switch (key) {
 			case K_TAB:
@@ -2041,7 +2044,8 @@ static int __perf_evlist__tui_browse_hists(struct perf_evlist *evlist,
 					   int nr_entries, const char *help,
 					   struct hist_browser_timer *hbt,
 					   float min_pcnt,
-					   struct perf_session_env *env)
+					   struct perf_session_env *env,
+					   int *done)
 {
 	struct perf_evsel *pos;
 	struct perf_evsel_menu menu = {
@@ -2068,13 +2072,14 @@ static int __perf_evlist__tui_browse_hists(struct perf_evlist *evlist,
 			menu.b.width = line_len;
 	}
 
-	return perf_evsel_menu__run(&menu, nr_entries, help, hbt);
+	return perf_evsel_menu__run(&menu, nr_entries, help, hbt, done);
 }
 
 int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
 				  struct hist_browser_timer *hbt,
 				  float min_pcnt,
-				  struct perf_session_env *env)
+				  struct perf_session_env *env,
+				  int *done)
 {
 	int nr_entries = evlist->nr_entries;
 
@@ -2084,7 +2089,7 @@ single_entry:
 
 		return perf_evsel__hists_browse(first, nr_entries, help,
 						false, hbt, min_pcnt,
-						env);
+						env, done);
 	}
 
 	if (symbol_conf.event_group) {
@@ -2101,5 +2106,5 @@ single_entry:
 	}
 
 	return __perf_evlist__tui_browse_hists(evlist, nr_entries, help,
-					       hbt, min_pcnt, env);
+					       hbt, min_pcnt, env, done);
 }
tools/perf/ui/browsers/map.c +1 -1
index b11639f33682..f87fa61c5e91 100644
@@ -78,7 +78,7 @@ static int map_browser__run(struct map_browser *browser)
 		return -1;
 
 	while (1) {
-		key = ui_browser__run(&browser->b, 0);
+		key = ui_browser__run(&browser->b, 0, NULL);
 
 		switch (key) {
 		case '/':
tools/perf/ui/browsers/scripts.c +1 -1
index 402d2bd30b09..6c23b197d6fb 100644
@@ -93,7 +93,7 @@ static int script_browser__run(struct perf_script_browser *browser)
 		return -1;
 
 	while (1) {
-		key = ui_browser__run(&browser->b, 0);
+		key = ui_browser__run(&browser->b, 0, NULL);
 
 		/* We can add some special key handling here if needed */
 		break;
tools/perf/ui/keysyms.h +1 -0
index 65092d576b4e..63a0c1b56761 100644
@@ -24,5 +24,6 @@
 #define K_ERROR	 -2
 #define K_RESIZE -3
 #define K_SWITCH_INPUT_DATA -4
+#define K_INIT	 -5
 
 #endif /* _PERF_KEYSYMS_H_ */
tools/perf/ui/tui/util.c +1 -1
index bf890f72fe80..a97fd4c43079 100644
@@ -32,7 +32,7 @@ static int popup_menu__run(struct ui_browser *menu)
 		return -1;
 
 	while (1) {
-		key = ui_browser__run(menu, 0);
+		key = ui_browser__run(menu, 0, NULL);
 
 		switch (key) {
 		case K_RIGHT:
tools/perf/util/hist.h +4 -2
index 95405a8fbd95..c452af22ea89 100644
@@ -312,7 +312,8 @@ int hist_entry__tui_annotate(struct hist_entry *he, struct perf_evsel *evsel,
 int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
 				  struct hist_browser_timer *hbt,
 				  float min_pcnt,
-				  struct perf_session_env *env);
+				  struct perf_session_env *env,
+				  int *done);
 int script_browse(const char *script_opt);
 #else
 static inline
@@ -320,7 +321,8 @@ int perf_evlist__tui_browse_hists(struct perf_evlist *evlist __maybe_unused,
 				  const char *help __maybe_unused,
 				  struct hist_browser_timer *hbt __maybe_unused,
 				  float min_pcnt __maybe_unused,
-				  struct perf_session_env *env __maybe_unused)
+				  struct perf_session_env *env __maybe_unused,
+				  int *done __maybe_unused)
 {
 	return 0;
 }
Signature · 1 line
1.8.3.1
author reviewer
#15 lore raw

PATCH 10/20 perf top: Add support to see event's ERR and HUP poll errors

In reply to Jiri Olsa

Adding top command support to see when all events are down and forcing it to stop in that case.

Like following top session will stop if the monitored pid (1234) exits:

  $ perf top -p 1234
Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/builtin-top.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)
tools/perf/builtin-top.c +28 -2
index 4cd56c9ddedc..d0eeeb7ca5ff 100644
@@ -40,6 +40,7 @@
 #include "util/xyarray.h"
 #include "util/sort.h"
 #include "util/intlist.h"
+#include "util/poller.h"
 #include "arch/common.h"
 
 #include "util/debug.h"
@@ -875,6 +876,12 @@ try_again:
 			ui__error("%s\n", msg);
 			goto out_err;
 		}
+
+		if (perf_evsel__set_poller(counter, &evlist->poller)) {
+			error("failed to set poller with %d (%s)\n", errno,
+			      strerror(errno));
+			goto out_err;
+		}
 	}
 
 	if (perf_evlist__mmap(evlist, opts->mmap_pages, false) < 0) {
@@ -906,11 +913,28 @@ static int perf_top__setup_sample_type(struct perf_top *top __maybe_unused)
 	return 0;
 }
 
+static int data_error(struct poller *p, struct poller_item *item)
+{
+	pr_debug("got HUP/ERROR on fd %d\n", item->fd);
+
+	poller__del(p, item);
+	if (poller__empty(p)) {
+		pr_debug("All events closed shutting down.\n");
+		done = 1;
+	}
+	return 0;
+}
+
 static int __cmd_top(struct perf_top *top)
 {
 	struct record_opts *opts = &top->record_opts;
 	pthread_t thread;
 	int ret;
+	struct poller *poller = &top->evlist->poller;
+	struct poller_ops poller_ops = {
+		.error  = data_error,
+		.hup    = data_error,
+	};
 
 	top->session = perf_session__new(NULL, false, NULL);
 	if (top->session == NULL)
@@ -948,8 +972,10 @@ static int __cmd_top(struct perf_top *top)
         if (!target__none(&opts->target))
                 perf_evlist__enable(top->evlist);
 
+	poller__set_ops(poller, &poller_ops);
+
 	/* Wait for a minimal set of events before starting the snapshot */
-	poll(top->evlist->pollfd, top->evlist->nr_fds, 100);
+	poller__poll(poller, 100);
 
 	perf_top__mmap_read(top);
 
@@ -976,7 +1002,7 @@ static int __cmd_top(struct perf_top *top)
 		perf_top__mmap_read(top);
 
 		if (hits == top->samples)
-			ret = poll(top->evlist->pollfd, top->evlist->nr_fds, 100);
+			ret = poller__poll(poller, 100);
 	}
 
 	ret = 0;
Signature · 1 line
1.8.3.1
author reviewer
#16 lore raw

PATCH 11/20 perf top: Join the display thread on exit

In reply to Jiri Olsa

We create the display thread, but never join it. It gives the display thread a chance to quit and cleanup properly.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/builtin-top.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
tools/perf/builtin-top.c +3 -1
index d0eeeb7ca5ff..eca2318b68a3 100644
@@ -992,7 +992,7 @@ static int __cmd_top(struct perf_top *top)
 		param.sched_priority = top->realtime_prio;
 		if (sched_setscheduler(0, SCHED_FIFO, &param)) {
 			ui__error("Could not set realtime priority.\n");
-			goto out_delete;
+			goto out_join;
 		}
 	}
 
@@ -1006,6 +1006,8 @@ static int __cmd_top(struct perf_top *top)
 	}
 
 	ret = 0;
+out_join:
+	pthread_join(thread, NULL);
 out_delete:
 	perf_session__delete(top->session);
 	top->session = NULL;
Signature · 1 line
1.8.3.1
#17 lore raw

[tip:perf/core] perf top: Join the display thread on exit

perf top: Join the display thread on exit
Commit-ID:  ae256fa27118e54b0f093f0cfe2ada28f1704ef3
Gitweb:     http://git.kernel.org/tip/ae256fa27118e54b0f093f0cfe2ada28f1704ef3
Author:     Jiri Olsa <[email protected]>
AuthorDate: Mon, 11 Aug 2014 10:50:05 +0200
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 13 Aug 2014 11:14:21 -0300

perf top: Join the display thread on exit

We create the display thread, but never join it. It gives the display thread a chance to quit and cleanup properly.

Cc Adrian Hunter <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/builtin-top.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
tools/perf/builtin-top.c +3 -1
index 4fb6f72..10fc7e8 100644
@@ -965,7 +965,7 @@ static int __cmd_top(struct perf_top *top)
 		param.sched_priority = top->realtime_prio;
 		if (sched_setscheduler(0, SCHED_FIFO, &param)) {
 			ui__error("Could not set realtime priority.\n");
-			goto out_delete;
+			goto out_join;
 		}
 	}
 
@@ -979,6 +979,8 @@ static int __cmd_top(struct perf_top *top)
 	}
 
 	ret = 0;
+out_join:
+	pthread_join(thread, NULL);
 out_delete:
 	perf_session__delete(top->session);
 	top->session = NULL;
author reviewer
#18 lore raw

PATCH 12/20 perf top: Use set_term_quiet_input for terminal input

In reply to Jiri Olsa

Using helper function instead of repeating the code.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/builtin-top.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)
tools/perf/builtin-top.c +2 -8
index eca2318b68a3..f19321282436 100644
@@ -582,20 +582,14 @@ static void *display_thread_tui(void *arg)
 static void *display_thread(void *arg)
 {
 	struct pollfd stdin_poll = { .fd = 0, .events = POLLIN };
-	struct termios tc, save;
+	struct termios save;
 	struct perf_top *top = arg;
 	int delay_msecs, c;
 
-	tcgetattr(0, &save);
-	tc = save;
-	tc.c_lflag &= ~(ICANON | ECHO);
-	tc.c_cc[VMIN] = 0;
-	tc.c_cc[VTIME] = 0;
-
 	pthread__unblock_sigwinch();
 repeat:
 	delay_msecs = top->delay_secs * 1000;
-	tcsetattr(0, TCSANOW, &tc);
+	set_term_quiet_input(&save);
 	/* trash return*/
 	getc(stdin);
 
Signature · 1 line
1.8.3.1
author reviewer
#19 lore raw

PATCH 13/20 perf top: Setup signals for terminal output

In reply to Jiri Olsa

The TUI code setup standard signals handling, while the stdio display code does not. This leads to premature termination of display thread when signal is received and leaving terminal in wrong state.

Also adding terminal cleanup at the end of display thread, to ensure we get the old terminal state in case of signal interruption.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/builtin-top.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
tools/perf/builtin-top.c +16 -0
index f19321282436..9b4edf0bd49f 100644
@@ -579,6 +579,20 @@ static void *display_thread_tui(void *arg)
 	return NULL;
 }
 
+static void display_sig(int sig __maybe_unused)
+{
+	done = 1;
+}
+
+static void display_setup_sig(void)
+{
+	signal(SIGSEGV, display_sig);
+	signal(SIGFPE,  display_sig);
+	signal(SIGINT,  display_sig);
+	signal(SIGQUIT, display_sig);
+	signal(SIGTERM, display_sig);
+}
+
 static void *display_thread(void *arg)
 {
 	struct pollfd stdin_poll = { .fd = 0, .events = POLLIN };
@@ -586,6 +600,7 @@ static void *display_thread(void *arg)
 	struct perf_top *top = arg;
 	int delay_msecs, c;
 
+	display_setup_sig();
 	pthread__unblock_sigwinch();
 repeat:
 	delay_msecs = top->delay_secs * 1000;
@@ -616,6 +631,7 @@ repeat:
 		}
 	}
 
+	tcsetattr(0, TCSAFLUSH, &save);
 	return NULL;
 }
 
Signature · 1 line
1.8.3.1
#20 lore raw

[tip:perf/core] perf top: Setup signals for terminal output

perf top: Setup signals for terminal output
Commit-ID:  4a1a99712a8a13d97e9de818869bf1b88583d6bc
Gitweb:     http://git.kernel.org/tip/4a1a99712a8a13d97e9de818869bf1b88583d6bc
Author:     Jiri Olsa <[email protected]>
AuthorDate: Mon, 11 Aug 2014 10:50:07 +0200
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 13 Aug 2014 12:42:24 -0300

perf top: Setup signals for terminal output

The TUI code setup standard signals handling, while the stdio display code does not. This leads to premature termination of display thread when signal is received and leaving terminal in wrong state.

Also adding terminal cleanup at the end of display thread, to ensure we get the old terminal state in case of signal interruption.

Cc Adrian Hunter <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/builtin-top.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
tools/perf/builtin-top.c +16 -0
index e8459e2..0ab3ea7 100644
@@ -577,6 +577,20 @@ static void *display_thread_tui(void *arg)
 	return NULL;
 }
 
+static void display_sig(int sig __maybe_unused)
+{
+	done = 1;
+}
+
+static void display_setup_sig(void)
+{
+	signal(SIGSEGV, display_sig);
+	signal(SIGFPE,  display_sig);
+	signal(SIGINT,  display_sig);
+	signal(SIGQUIT, display_sig);
+	signal(SIGTERM, display_sig);
+}
+
 static void *display_thread(void *arg)
 {
 	struct pollfd stdin_poll = { .fd = 0, .events = POLLIN };
@@ -584,6 +598,7 @@ static void *display_thread(void *arg)
 	struct perf_top *top = arg;
 	int delay_msecs, c;
 
+	display_setup_sig();
 	pthread__unblock_sigwinch();
 repeat:
 	delay_msecs = top->delay_secs * 1000;
@@ -614,6 +629,7 @@ repeat:
 		}
 	}
 
+	tcsetattr(0, TCSAFLUSH, &save);
 	return NULL;
 }
 
author reviewer
#21 lore raw

PATCH 14/20 perf top: Use poller object for display thread stdin

In reply to Jiri Olsa

Using poller object to handle user input for stdio top.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/builtin-top.c | 43 +++++++++++++++++++------------------------
 1 file changed, 19 insertions(+), 24 deletions(-)
tools/perf/builtin-top.c +19 -24
index 9b4edf0bd49f..8d0ed211fd7b 100644
@@ -422,26 +422,22 @@ static int perf_top__key_mapped(struct perf_top *top, int c)
 	return 0;
 }
 
-static bool perf_top__handle_keypress(struct perf_top *top, int c)
+static bool perf_top__handle_keypress(struct perf_top *top, int c,
+				      struct poller *poller)
 {
 	bool ret = true;
 
 	if (!perf_top__key_mapped(top, c)) {
-		struct pollfd stdin_poll = { .fd = 0, .events = POLLIN };
-		struct termios tc, save;
+		struct termios save;
 
 		perf_top__print_mapped_keys(top);
 		fprintf(stdout, "\nEnter selection, or unmapped key to continue: ");
 		fflush(stdout);
 
-		tcgetattr(0, &save);
-		tc = save;
-		tc.c_lflag &= ~(ICANON | ECHO);
-		tc.c_cc[VMIN] = 0;
-		tc.c_cc[VTIME] = 0;
-		tcsetattr(0, TCSANOW, &tc);
+		set_term_quiet_input(&save);
+		if (poller__poll(poller, -1) < 0)
+			return 0;
 
-		poll(&stdin_poll, 1, -1);
 		c = getc(stdin);
 
 		tcsetattr(0, TCSAFLUSH, &save);
@@ -595,11 +591,19 @@ static void display_setup_sig(void)
 
 static void *display_thread(void *arg)
 {
-	struct pollfd stdin_poll = { .fd = 0, .events = POLLIN };
+	struct poller poller;
+	struct poller_item in = {
+		.fd = fileno(stdin),
+	};
 	struct termios save;
 	struct perf_top *top = arg;
 	int delay_msecs, c;
 
+	if (poller__add(&poller, &in)) {
+		done = 1;
+		return NULL;
+	}
+
 	display_setup_sig();
 	pthread__unblock_sigwinch();
 repeat:
@@ -610,23 +614,14 @@ repeat:
 
 	while (!done) {
 		perf_top__print_sym_table(top);
-		/*
-		 * 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:
+
+		if (poller__poll(&poller, delay_msecs) > 0) {
 			c = getc(stdin);
 			tcsetattr(0, TCSAFLUSH, &save);
 
-			if (perf_top__handle_keypress(top, c))
+			if (perf_top__handle_keypress(top, c, &poller))
 				goto repeat;
+
 			done = 1;
 		}
 	}
Signature · 1 line
1.8.3.1
author reviewer
#22 lore raw

PATCH 15/20 perf kvm: Fix stdin handling for 'kvm stat live' command

In reply to Jiri Olsa

Currently the initial ~(ICANON | ECHO) terminal mode is not set, so we dont get stdin data until we press ENTER.

Fixing this by early setting of the ~(ICANON | ECHO) mode and leaving this mode for whole life of the command, because canonical mode is not needed.

This fix is preparation for the poller object installation into 'kvm stat live' command.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/builtin-kvm.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)
tools/perf/builtin-kvm.c +5 -13
index 7ccceadcd9f8..ff404750d744 100644
@@ -885,15 +885,11 @@ static int fd_set_nonblock(int fd)
 	return 0;
 }
 
-static
-int perf_kvm__handle_stdin(struct termios *tc_now, struct termios *tc_save)
+static int perf_kvm__handle_stdin(void)
 {
 	int c;
 
-	tcsetattr(0, TCSANOW, tc_now);
 	c = getc(stdin);
-	tcsetattr(0, TCSAFLUSH, tc_save);
-
 	if (c == 'q')
 		return 1;
 
@@ -904,7 +900,7 @@ static int kvm_events_live_report(struct perf_kvm_stat *kvm)
 {
 	struct pollfd *pollfds = NULL;
 	int nr_fds, nr_stdin, ret, err = -EINVAL;
-	struct termios tc, save;
+	struct termios save;
 
 	/* live flag must be set first */
 	kvm->live = true;
@@ -919,14 +915,9 @@ static int kvm_events_live_report(struct perf_kvm_stat *kvm)
 		goto out;
 	}
 
+	set_term_quiet_input(&save);
 	init_kvm_event_record(kvm);
 
-	tcgetattr(0, &save);
-	tc = save;
-	tc.c_lflag &= ~(ICANON | ECHO);
-	tc.c_cc[VMIN] = 0;
-	tc.c_cc[VTIME] = 0;
-
 	signal(SIGINT, sig_handler);
 	signal(SIGTERM, sig_handler);
 
@@ -972,7 +963,7 @@ static int kvm_events_live_report(struct perf_kvm_stat *kvm)
 			goto out;
 
 		if (pollfds[nr_stdin].revents & POLLIN)
-			done = perf_kvm__handle_stdin(&tc, &save);
+			done = perf_kvm__handle_stdin();
 
 		if (!rc && !done)
 			err = poll(pollfds, nr_fds, 100);
@@ -989,6 +980,7 @@ out:
 	if (kvm->timerfd >= 0)
 		close(kvm->timerfd);
 
+	tcsetattr(0, TCSAFLUSH, &save);
 	free(pollfds);
 	return err;
 }
Signature · 1 line
1.8.3.1
#23 lore raw

[tip:perf/core] perf kvm: Fix stdin handling for 'kvm stat live' command

perf kvm: Fix stdin handling for 'kvm stat live' command
Commit-ID:  d5b4130ae636e1fd0eaaea7152170198c34fbf62
Gitweb:     http://git.kernel.org/tip/d5b4130ae636e1fd0eaaea7152170198c34fbf62
Author:     Jiri Olsa <[email protected]>
AuthorDate: Mon, 11 Aug 2014 10:50:09 +0200
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 13 Aug 2014 12:42:24 -0300

perf kvm: Fix stdin handling for 'kvm stat live' command

Currently the initial ~(ICANON | ECHO) terminal mode is not set, so we dont get stdin data until we press ENTER.

Fixing this by early setting of the ~(ICANON | ECHO) mode and leaving this mode for whole life of the command, because canonical mode is not needed.

Cc Adrian Hunter <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/builtin-kvm.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)
tools/perf/builtin-kvm.c +5 -13
index 7cccead..ff40475 100644
@@ -885,15 +885,11 @@ static int fd_set_nonblock(int fd)
 	return 0;
 }
 
-static
-int perf_kvm__handle_stdin(struct termios *tc_now, struct termios *tc_save)
+static int perf_kvm__handle_stdin(void)
 {
 	int c;
 
-	tcsetattr(0, TCSANOW, tc_now);
 	c = getc(stdin);
-	tcsetattr(0, TCSAFLUSH, tc_save);
-
 	if (c == 'q')
 		return 1;
 
@@ -904,7 +900,7 @@ static int kvm_events_live_report(struct perf_kvm_stat *kvm)
 {
 	struct pollfd *pollfds = NULL;
 	int nr_fds, nr_stdin, ret, err = -EINVAL;
-	struct termios tc, save;
+	struct termios save;
 
 	/* live flag must be set first */
 	kvm->live = true;
@@ -919,14 +915,9 @@ static int kvm_events_live_report(struct perf_kvm_stat *kvm)
 		goto out;
 	}
 
+	set_term_quiet_input(&save);
 	init_kvm_event_record(kvm);
 
-	tcgetattr(0, &save);
-	tc = save;
-	tc.c_lflag &= ~(ICANON | ECHO);
-	tc.c_cc[VMIN] = 0;
-	tc.c_cc[VTIME] = 0;
-
 	signal(SIGINT, sig_handler);
 	signal(SIGTERM, sig_handler);
 
@@ -972,7 +963,7 @@ static int kvm_events_live_report(struct perf_kvm_stat *kvm)
 			goto out;
 
 		if (pollfds[nr_stdin].revents & POLLIN)
-			done = perf_kvm__handle_stdin(&tc, &save);
+			done = perf_kvm__handle_stdin();
 
 		if (!rc && !done)
 			err = poll(pollfds, nr_fds, 100);
@@ -989,6 +980,7 @@ out:
 	if (kvm->timerfd >= 0)
 		close(kvm->timerfd);
 
+	tcsetattr(0, TCSAFLUSH, &save);
 	free(pollfds);
 	return err;
 }
author reviewer
#24 lore raw

PATCH 16/20 perf kvm: Add support to see event's ERR and HUP poll errors

In reply to Jiri Olsa

Adding kvm command support to see when all events are down and forcing it to stop in that case.

Like following kvm stat live session will stop if the monitored pid (1234) exits:

      $ sudo perf kvm stat live -p 1234
Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/builtin-kvm.c   | 111 +++++++++++++++++++++++----------------------
 tools/perf/util/kvm-stat.h |   1 -
 2 files changed, 57 insertions(+), 55 deletions(-)
tools/perf/builtin-kvm.c +57 -54
index ff404750d744..30057b6fb44b 100644
@@ -808,10 +808,10 @@ static void sig_handler(int sig __maybe_unused)
 static int perf_kvm__timerfd_create(struct perf_kvm_stat *kvm)
 {
 	struct itimerspec new_value;
-	int rc = -1;
+	int fd;
 
-	kvm->timerfd = timerfd_create(CLOCK_MONOTONIC, TFD_NONBLOCK);
-	if (kvm->timerfd < 0) {
+	fd = timerfd_create(CLOCK_MONOTONIC, TFD_NONBLOCK);
+	if (fd < 0) {
 		pr_err("timerfd_create failed\n");
 		goto out;
 	}
@@ -821,23 +821,24 @@ static int perf_kvm__timerfd_create(struct perf_kvm_stat *kvm)
 	new_value.it_interval.tv_sec = kvm->display_time;
 	new_value.it_interval.tv_nsec = 0;
 
-	if (timerfd_settime(kvm->timerfd, 0, &new_value, NULL) != 0) {
+	if (timerfd_settime(fd, 0, &new_value, NULL) != 0) {
 		pr_err("timerfd_settime failed: %d\n", errno);
-		close(kvm->timerfd);
-		goto out;
+		close(fd);
+		return -1;
 	}
 
-	rc = 0;
 out:
-	return rc;
+	return fd;
 }
 
-static int perf_kvm__handle_timerfd(struct perf_kvm_stat *kvm)
+static int perf_kvm__handle_timerfd(struct poller *p __maybe_unused,
+				    struct poller_item *item)
 {
+	struct perf_kvm_stat *kvm = item->data;
 	uint64_t c;
 	int rc;
 
-	rc = read(kvm->timerfd, &c, sizeof(uint64_t));
+	rc = read(item->fd, &c, sizeof(uint64_t));
 	if (rc < 0) {
 		if (errno == EAGAIN)
 			return 0;
@@ -867,40 +868,59 @@ static int perf_kvm__handle_timerfd(struct perf_kvm_stat *kvm)
 	return 0;
 }
 
-static int fd_set_nonblock(int fd)
+static int data_error(struct poller *p, struct poller_item *item)
 {
-	long arg = 0;
+	pr_debug("got HUP/ERROR on fd %d\n", item->fd);
 
-	arg = fcntl(fd, F_GETFL);
-	if (arg < 0) {
-		pr_err("Failed to get current flags for fd %d\n", fd);
-		return -1;
-	}
+	poller__del(p, item);
 
-	if (fcntl(fd, F_SETFL, arg | O_NONBLOCK) < 0) {
-		pr_err("Failed to set non-block option on fd %d\n", fd);
-		return -1;
+	/*
+	 * Only 2 remains (stdin and timerfd), that mean we are
+	 * out of events, time to leave.
+	 */
+	if (p->n == 2) {
+		pr_debug("All events closed, shutting down.\n");
+		done = 1;
 	}
-
 	return 0;
 }
 
-static int perf_kvm__handle_stdin(void)
+static
+int perf_kvm__handle_stdin(struct poller *p __maybe_unused,
+			   struct poller_item *item __maybe_unused)
 {
 	int c;
 
 	c = getc(stdin);
 	if (c == 'q')
-		return 1;
+		done = 1;
 
 	return 0;
 }
 
 static int kvm_events_live_report(struct perf_kvm_stat *kvm)
 {
-	struct pollfd *pollfds = NULL;
-	int nr_fds, nr_stdin, ret, err = -EINVAL;
+	int ret, err = -EINVAL;
 	struct termios save;
+	struct poller *poller = &kvm->evlist->poller;
+	struct poller_item input = {
+		.fd  = fileno(stdin),
+		.ops = {
+			.data = perf_kvm__handle_stdin,
+		}
+	};
+	struct poller_item timer = {
+		.data = kvm,
+		.ops  = {
+			.data = perf_kvm__handle_timerfd,
+		},
+	};
+	struct poller_ops poller_ops = {
+		.error  = data_error,
+		.hup    = data_error,
+	};
+
+	poller__set_ops(poller, &poller_ops);
 
 	/* live flag must be set first */
 	kvm->live = true;
@@ -921,31 +941,22 @@ static int kvm_events_live_report(struct perf_kvm_stat *kvm)
 	signal(SIGINT, sig_handler);
 	signal(SIGTERM, sig_handler);
 
-	/* copy pollfds -- need to add timerfd and stdin */
-	nr_fds = kvm->evlist->nr_fds;
-	pollfds = zalloc(sizeof(struct pollfd) * (nr_fds + 2));
-	if (!pollfds) {
-		err = -ENOMEM;
+	err = perf_evlist__set_poller(kvm->evlist);
+	if (err < 0)
 		goto out;
-	}
-	memcpy(pollfds, kvm->evlist->pollfd,
-		sizeof(struct pollfd) * kvm->evlist->nr_fds);
 
-	/* add timer fd */
-	if (perf_kvm__timerfd_create(kvm) < 0) {
+	/* create timer fd */
+	timer.fd = perf_kvm__timerfd_create(kvm);
+	if (timer.fd < 0) {
 		err = -1;
 		goto out;
 	}
 
-	pollfds[nr_fds].fd = kvm->timerfd;
-	pollfds[nr_fds].events = POLLIN;
-	nr_fds++;
+	err = -1;
 
-	pollfds[nr_fds].fd = fileno(stdin);
-	pollfds[nr_fds].events = POLLIN;
-	nr_stdin = nr_fds;
-	nr_fds++;
-	if (fd_set_nonblock(fileno(stdin)) != 0)
+	/* add stdin and timer into poller */
+	if (poller__add(poller, &timer) ||
+	    poller__add(poller, &input))
 		goto out;
 
 	/* everything is good - enable the events and process */
@@ -958,15 +969,8 @@ static int kvm_events_live_report(struct perf_kvm_stat *kvm)
 		if (rc < 0)
 			break;
 
-		err = perf_kvm__handle_timerfd(kvm);
-		if (err)
-			goto out;
-
-		if (pollfds[nr_stdin].revents & POLLIN)
-			done = perf_kvm__handle_stdin();
-
 		if (!rc && !done)
-			err = poll(pollfds, nr_fds, 100);
+			err = poller__poll(poller, 100);
 	}
 
 	perf_evlist__disable(kvm->evlist);
@@ -977,11 +981,10 @@ static int kvm_events_live_report(struct perf_kvm_stat *kvm)
 	}
 
 out:
-	if (kvm->timerfd >= 0)
-		close(kvm->timerfd);
+	if (timer.fd >= 0)
+		close(timer.fd);
 
 	tcsetattr(0, TCSAFLUSH, &save);
-	free(pollfds);
 	return err;
 }
 
tools/perf/util/kvm-stat.h +0 -1
index 0b5a8cd2ee79..eb34bee83615 100644
@@ -97,7 +97,6 @@ struct perf_kvm_stat {
 
 	struct rb_root result;
 
-	int timerfd;
 	unsigned int display_time;
 	bool live;
 };
Signature · 1 line
1.8.3.1
author reviewer
#25 lore raw

PATCH 17/20 perf trace: Add support to see event's ERR and HUP poll errors

In reply to Jiri Olsa

Adding trace command support to see when all events are down and forcing it to stop in that case.

Like following trace session will stop if the monitored pid (1234) exits:

  $ perf trace -p 1234
Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/builtin-trace.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)
tools/perf/builtin-trace.c +24 -1
index 36ae51d61be2..d465f4519a66 100644
@@ -2011,6 +2011,18 @@ static int perf_evlist__add_pgfault(struct perf_evlist *evlist,
 	return 0;
 }
 
+static int data_error(struct poller *p, struct poller_item *item)
+{
+	pr_debug("got HUP/ERROR on fd %d\n", item->fd);
+
+	poller__del(p, item);
+	if (poller__empty(p)) {
+		pr_debug("All events closed, shutting down.\n");
+		done = true;
+	}
+	return 0;
+}
+
 static int trace__run(struct trace *trace, int argc, const char **argv)
 {
 	struct perf_evlist *evlist = perf_evlist__new();
@@ -2018,6 +2030,11 @@ static int trace__run(struct trace *trace, int argc, const char **argv)
 	int err = -1, i;
 	unsigned long before;
 	const bool forks = argc > 0;
+	struct poller *poller = &evlist->poller;
+	struct poller_ops poller_ops = {
+		.error  = data_error,
+		.hup    = data_error,
+	};
 
 	trace->live = true;
 
@@ -2077,6 +2094,10 @@ static int trace__run(struct trace *trace, int argc, const char **argv)
 	if (err < 0)
 		goto out_error_open;
 
+	err = perf_evlist__set_poller(evlist);
+	if (err < 0)
+		goto out_delete_evlist;
+
 	err = perf_evlist__mmap(evlist, trace->opts.mmap_pages, false);
 	if (err < 0) {
 		fprintf(trace->output, "Couldn't mmap the events: %s\n", strerror(errno));
@@ -2088,6 +2109,8 @@ static int trace__run(struct trace *trace, int argc, const char **argv)
 	if (forks)
 		perf_evlist__start_workload(evlist);
 
+	poller__set_ops(poller, &poller_ops);
+
 	trace->multiple_threads = evlist->threads->map[0] == -1 || evlist->threads->nr > 1;
 again:
 	before = trace->nr_events;
@@ -2143,7 +2166,7 @@ next_event:
 	if (trace->nr_events == before) {
 		int timeout = done ? 100 : -1;
 
-		if (poll(evlist->pollfd, evlist->nr_fds, timeout) > 0)
+		if (poller__poll(poller, timeout) > 0)
 			goto again;
 	} else {
 		goto again;
Signature · 1 line
1.8.3.1
author reviewer
#26 lore raw

PATCH 18/20 perf python: Use poller object for polling

In reply to Jiri Olsa

Change python polling guts using evlist->pollfd for poller object.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/util/python-ext-sources |  2 ++
 tools/perf/util/python.c           | 19 +++++++++++++++----
 2 files changed, 17 insertions(+), 4 deletions(-)
tools/perf/util/python-ext-sources +2 -0
index 16a475a7d492..a2d584126eb1 100644
@@ -20,3 +20,5 @@ util/strlist.c
 ../lib/api/fs/fs.c
 util/trace-event.c
 ../../lib/rbtree.c
+util/poller.c
+util/string.c
tools/perf/util/python.c +15 -4
index 12aa9b0d0ba1..5903b6c30c16 100644
@@ -7,6 +7,7 @@
 #include "event.h"
 #include "cpumap.h"
 #include "thread_map.h"
+#include "poller.h"
 
 /*
  * Support debug printing even though util/debug.c is not linked.  That means
@@ -730,13 +731,22 @@ static PyObject *pyrf_evlist__poll(struct pyrf_evlist *pevlist,
 				   PyObject *args, PyObject *kwargs)
 {
 	struct perf_evlist *evlist = &pevlist->evlist;
+	struct poller *poller = &evlist->poller;
 	static char *kwlist[] = { "timeout", NULL };
-	int timeout = -1, n;
+	int timeout = -1, n, err;
 
 	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|i", kwlist, &timeout))
 		return NULL;
 
-	n = poll(evlist->pollfd, evlist->nr_fds, timeout);
+	if (poller__empty(poller)) {
+		err = perf_evlist__set_poller(evlist);
+		if (err) {
+			PyErr_SetFromErrno(PyExc_OSError);
+			return NULL;
+		}
+	}
+
+	n = poller__poll(poller, timeout);
 	if (n < 0) {
 		PyErr_SetFromErrno(PyExc_OSError);
 		return NULL;
@@ -750,12 +760,13 @@ static PyObject *pyrf_evlist__get_pollfd(struct pyrf_evlist *pevlist,
 					 PyObject *kwargs __maybe_unused)
 {
 	struct perf_evlist *evlist = &pevlist->evlist;
+	struct poller *poller = &evlist->poller;
         PyObject *list = PyList_New(0);
 	int i;
 
-	for (i = 0; i < evlist->nr_fds; ++i) {
+	for (i = 0; i < poller->n; ++i) {
 		PyObject *file;
-		FILE *fp = fdopen(evlist->pollfd[i].fd, "r");
+		FILE *fp = fdopen(poller->ptr[i].fd, "r");
 
 		if (fp == NULL)
 			goto free_list;
Signature · 1 line
1.8.3.1
author reviewer
#27 lore raw

PATCH 19/20 perf tests: Use poller object for polling

In reply to Jiri Olsa

Several tests are using evlist->pollfd means for polling events. Changing them to use poller object.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/tests/open-syscall-tp-fields.c | 9 ++++++++-
 tools/perf/tests/perf-record.c            | 4 ----
 tools/perf/tests/task-exit.c              | 9 ++++++++-
 3 files changed, 16 insertions(+), 6 deletions(-)
tools/perf/tests/open-syscall-tp-fields.c +8 -1
index 0785b64ffd6c..b0fc35d23244 100644
@@ -4,6 +4,7 @@
 #include "thread_map.h"
 #include "tests.h"
 #include "debug.h"
+#include "poller.h"
 
 int test__syscall_open_tp_fields(void)
 {
@@ -58,6 +59,12 @@ int test__syscall_open_tp_fields(void)
 		goto out_delete_evlist;
 	}
 
+	err = perf_evlist__set_poller(evlist);
+	if (err < 0) {
+		pr_debug("perf_evlist__set_poller: %s\n", strerror(errno));
+		goto out_delete_evlist;
+	};
+
 	perf_evlist__enable(evlist);
 
 	/*
@@ -102,7 +109,7 @@ int test__syscall_open_tp_fields(void)
 		}
 
 		if (nr_events == before)
-			poll(evlist->pollfd, evlist->nr_fds, 10);
+			poller__poll(&evlist->poller, 10);
 
 		if (++nr_polls > 5) {
 			pr_debug("%s: no events!\n", __func__);
tools/perf/tests/perf-record.c +0 -4
index aca1a83dd13a..ee5d2881e3fe 100644
@@ -147,8 +147,6 @@ int test__PERF_RECORD(void)
 	perf_evlist__start_workload(evlist);
 
 	while (1) {
-		int before = total_events;
-
 		for (i = 0; i < evlist->nr_mmaps; i++) {
 			union perf_event *event;
 
@@ -262,8 +260,6 @@ int test__PERF_RECORD(void)
 		 * PERF_RECORD_{!SAMPLE} events don't honour
 		 * perf_event_attr.wakeup_events, just PERF_EVENT_SAMPLE does.
 		 */
-		if (total_events == before && false)
-			poll(evlist->pollfd, evlist->nr_fds, -1);
 
 		sleep(1);
 		if (++wakeups > 5) {
tools/perf/tests/task-exit.c +8 -1
index 5ff3db318f12..bc858c799ff2 100644
@@ -3,6 +3,7 @@
 #include "thread_map.h"
 #include "cpumap.h"
 #include "tests.h"
+#include "poller.h"
 
 #include <signal.h>
 
@@ -92,6 +93,12 @@ int test__task_exit(void)
 		goto out_delete_evlist;
 	}
 
+	if (perf_evlist__set_poller(evlist)) {
+		pr_debug("failed to set poller: %d (%s)\n", errno,
+			 strerror(errno));
+		goto out_delete_evlist;
+	}
+
 	perf_evlist__start_workload(evlist);
 
 retry:
@@ -103,7 +110,7 @@ retry:
 	}
 
 	if (!exited || !nr_exit) {
-		poll(evlist->pollfd, evlist->nr_fds, -1);
+		poller__poll(&evlist->poller, -1);
 		goto retry;
 	}
 
Signature · 1 line
1.8.3.1
author reviewer
#28 lore raw

PATCH 20/20 perf tools: Remove pollfd stuff out of evlist object

In reply to Jiri Olsa

There's no need for evlist->pollfd data anymore. All the perf events polling job is now done poller object.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 tools/perf/util/evlist.c | 22 ----------------------
 tools/perf/util/evlist.h |  4 ----
 2 files changed, 26 deletions(-)
tools/perf/util/evlist.c +0 -22
index a4c381c10c72..3cd5b711b762 100644
@@ -102,7 +102,6 @@ static void perf_evlist__purge(struct perf_evlist *evlist)
 void perf_evlist__exit(struct perf_evlist *evlist)
 {
 	zfree(&evlist->mmap);
-	zfree(&evlist->pollfd);
 }
 
 void perf_evlist__delete(struct perf_evlist *evlist)
@@ -339,23 +338,6 @@ int perf_evlist__enable_event(struct perf_evlist *evlist,
 	return 0;
 }
 
-static int perf_evlist__alloc_pollfd(struct perf_evlist *evlist)
-{
-	int nr_cpus = cpu_map__nr(evlist->cpus);
-	int nr_threads = thread_map__nr(evlist->threads);
-	int nfds = nr_cpus * nr_threads * evlist->nr_entries;
-	evlist->pollfd = malloc(sizeof(struct pollfd) * nfds);
-	return evlist->pollfd != NULL ? 0 : -ENOMEM;
-}
-
-void perf_evlist__add_pollfd(struct perf_evlist *evlist, int fd)
-{
-	fcntl(fd, F_SETFL, O_NONBLOCK);
-	evlist->pollfd[evlist->nr_fds].fd = fd;
-	evlist->pollfd[evlist->nr_fds].events = POLLIN;
-	evlist->nr_fds++;
-}
-
 static void perf_evlist__id_hash(struct perf_evlist *evlist,
 				 struct perf_evsel *evsel,
 				 int cpu, int thread, u64 id)
@@ -625,7 +607,6 @@ static int __perf_evlist__mmap(struct perf_evlist *evlist, int idx,
 		return -1;
 	}
 
-	perf_evlist__add_pollfd(evlist, fd);
 	return 0;
 }
 
@@ -804,9 +785,6 @@ int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages,
 	if (evlist->mmap == NULL && perf_evlist__alloc_mmap(evlist) < 0)
 		return -ENOMEM;
 
-	if (evlist->pollfd == NULL && perf_evlist__alloc_pollfd(evlist) < 0)
-		return -ENOMEM;
-
 	evlist->overwrite = overwrite;
 	evlist->mmap_len = perf_evlist__mmap_size(pages);
 	pr_debug("mmap size %zuB\n", evlist->mmap_len);
tools/perf/util/evlist.h +0 -4
index 7d03af1ce4f7..d9a55e79f8bb 100644
@@ -9,7 +9,6 @@
 #include "util.h"
 #include <unistd.h>
 
-struct pollfd;
 struct thread_map;
 struct cpu_map;
 struct record_opts;
@@ -41,7 +40,6 @@ struct perf_evlist {
 	} workload;
 	bool		 overwrite;
 	struct perf_mmap *mmap;
-	struct pollfd	 *pollfd;
 	struct thread_map *threads;
 	struct cpu_map	  *cpus;
 	struct perf_evsel *selected;
@@ -83,8 +81,6 @@ perf_evlist__find_tracepoint_by_name(struct perf_evlist *evlist,
 void perf_evlist__id_add(struct perf_evlist *evlist, struct perf_evsel *evsel,
 			 int cpu, int thread, u64 id);
 
-void perf_evlist__add_pollfd(struct perf_evlist *evlist, int fd);
-
 struct perf_evsel *perf_evlist__id2evsel(struct perf_evlist *evlist, u64 id);
 
 struct perf_sample_id *perf_evlist__id2sid(struct perf_evlist *evlist, u64 id);
Signature · 1 line
1.8.3.1
#29 lore raw
In reply to Jiri Olsa

Em Mon, Aug 11, 2014 at 10:49:54AM +0200, Jiri Olsa escreveu:

Quoted 11 lines

hi,
adding support to quit sampling commands:
record,top,trace,kvm stat live

when all the monitored events are closed, like following perf command will now exit when monitored process (pid 1234) exits:

  $ perf record -p 1234

I added independent poller object to handle basic polling tasks. I had to factor some parts, so sending this as RFC,

Why? I'm trying to figure out why this poller class is needed, just from reading the changelog entries, no luck so far.

Quoted 95 lines

because I'm pretty sure I broke something else ;-)

The patch#1 is kernel change, which is needed for the patchset to work properly. The current behavior stays without this kernel change.

The code is based on latest Arnaldo's perf/core and following kernel change:
http://marc.info/?l=linux-kernel&m=140715910417130&w=2

The patchset is also reachable in here:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/core_poll

thanks for comments,
jirka

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>

---
Jiri Olsa (20):
perf: Add PERF_EVENT_STATE_EXIT state for events with exited task perf tools: Add poller object to handle polling globaly perf tests: Add poller object test
perf tools: Add support to traverse xyarrays
perf tools: Introduce perf_evsel__fd function
perf tools: Add evlist/evsel poller object support perf record: Add support to see event's ERR and HUP poll errors perf tools: Add set_term_quiet_input helper function perf tui browser: Add interface to terminate browser from uotside perf top: Add support to see event's ERR and HUP poll errors perf top: Join the display thread on exit
perf top: Use set_term_quiet_input for terminal input perf top: Setup signals for terminal output
perf top: Use poller object for display thread stdin perf kvm: Fix stdin handling for 'kvm stat live' command perf kvm: Add support to see event's ERR and HUP poll errors perf trace: Add support to see event's ERR and HUP poll errors perf python: Use poller object for polling
perf tests: Use poller object for polling
perf tools: Remove pollfd stuff out of evlist object

 include/linux/perf_event.h                |   1 +
 kernel/events/core.c                      |  12 ++++-
 tools/perf/Makefile.perf                  |   4 ++
 tools/perf/builtin-kvm.c                  | 123 +++++++++++++++++++++++------------------------
 tools/perf/builtin-record.c               |  29 ++++++++++-
 tools/perf/builtin-report.c               |   3 +-
 tools/perf/builtin-top.c                  | 104 ++++++++++++++++++++++++++--------------
 tools/perf/builtin-trace.c                |  25 +++++++++-
 tools/perf/tests/builtin-test.c           |   8 ++++
 tools/perf/tests/open-syscall-tp-fields.c |   9 +++-
 tools/perf/tests/perf-record.c            |   4 --
 tools/perf/tests/poller.c                 | 126 ++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/task-exit.c              |   9 +++-
 tools/perf/tests/tests.h                  |   2 +
 tools/perf/tests/xyarray.c                |  33 +++++++++++++
 tools/perf/ui/browser.c                   |   4 +-
 tools/perf/ui/browser.h                   |   8 +++-
 tools/perf/ui/browsers/annotate.c         |   2 +-
 tools/perf/ui/browsers/header.c           |   2 +-
 tools/perf/ui/browsers/hists.c            |  39 ++++++++-------
 tools/perf/ui/browsers/map.c              |   2 +-
 tools/perf/ui/browsers/scripts.c          |   2 +-
 tools/perf/ui/keysyms.h                   |   1 +
 tools/perf/ui/tui/util.c                  |   2 +-
 tools/perf/util/evlist.c                  |  38 ++++++---------
 tools/perf/util/evlist.h                  |   7 ++-
 tools/perf/util/evsel.c                   |  21 +++++++-
 tools/perf/util/evsel.h                   |  10 ++++
 tools/perf/util/hist.h                    |   6 ++-
 tools/perf/util/kvm-stat.h                |   1 -
 tools/perf/util/poller.c                  | 233 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/poller.h                  |  88 ++++++++++++++++++++++++++++++++++
 tools/perf/util/python-ext-sources        |   2 +
 tools/perf/util/python.c                  |  19 ++++++--
 tools/perf/util/util.c                    |  13 +++++
 tools/perf/util/util.h                    |   2 +
 tools/perf/util/xyarray.c                 |   4 +-
 tools/perf/util/xyarray.h                 |   6 +++
 38 files changed, 832 insertions(+), 172 deletions(-)
 create mode 100644 tools/perf/tests/poller.c
 create mode 100644 tools/perf/tests/xyarray.c
 create mode 100644 tools/perf/util/poller.c
 create mode 100644 tools/perf/util/poller.h
#30 lore raw

Em Mon, Aug 11, 2014 at 05:12:49PM -0300, Arnaldo Carvalho de Melo escreveu:

Quoted 8 lines

Em Mon, Aug 11, 2014 at 10:49:54AM +0200, Jiri Olsa escreveu:

Quoted 7 lines

hi,
adding support to quit sampling commands:
record,top,trace,kvm stat live

when all the monitored events are closed, like following perf command will now exit when monitored process (pid 1234) exits:

Quoted 7 lines
Quoted 4 lines
  $ perf record -p 1234

I added independent poller object to handle basic polling tasks. I had to factor some parts, so sending this as RFC,

Why? I'm trying to figure out why this poller class is needed, just from reading the changelog entries, no luck so far.

So this is all about accounting for when all events are closed? I.e. as we receive the ERR and HUP we go on closing the entries in the pollfd? Then, when none are closed, then we exit, is that the case?

Why not just mark them -1 and decrement evlist->nr_open_fds (new field)?

struct pollfds {
	struct pollfd *pollfds;
	int    nr_fds;
	int    nr_open_fds;
};

Don't use callbacks and then that poller_item doesn't need to exist :-\

I'll try to cook a patch with those ideas later today, if for anything, to validate your patches :-)

- Arnaldo

Quoted 95 lines
Quoted 95 lines

because I'm pretty sure I broke something else ;-)

The patch#1 is kernel change, which is needed for the patchset to work properly. The current behavior stays without this kernel change.

The code is based on latest Arnaldo's perf/core and following kernel change:
http://marc.info/?l=linux-kernel&m=140715910417130&w=2

The patchset is also reachable in here:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/core_poll

thanks for comments,
jirka

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Jean Pihet <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>

---
Jiri Olsa (20):
perf: Add PERF_EVENT_STATE_EXIT state for events with exited task perf tools: Add poller object to handle polling globaly perf tests: Add poller object test
perf tools: Add support to traverse xyarrays
perf tools: Introduce perf_evsel__fd function
perf tools: Add evlist/evsel poller object support perf record: Add support to see event's ERR and HUP poll errors perf tools: Add set_term_quiet_input helper function perf tui browser: Add interface to terminate browser from uotside perf top: Add support to see event's ERR and HUP poll errors perf top: Join the display thread on exit
perf top: Use set_term_quiet_input for terminal input perf top: Setup signals for terminal output
perf top: Use poller object for display thread stdin perf kvm: Fix stdin handling for 'kvm stat live' command perf kvm: Add support to see event's ERR and HUP poll errors perf trace: Add support to see event's ERR and HUP poll errors perf python: Use poller object for polling
perf tests: Use poller object for polling
perf tools: Remove pollfd stuff out of evlist object

 include/linux/perf_event.h                |   1 +
 kernel/events/core.c                      |  12 ++++-
 tools/perf/Makefile.perf                  |   4 ++
 tools/perf/builtin-kvm.c                  | 123 +++++++++++++++++++++++------------------------
 tools/perf/builtin-record.c               |  29 ++++++++++-
 tools/perf/builtin-report.c               |   3 +-
 tools/perf/builtin-top.c                  | 104 ++++++++++++++++++++++++++--------------
 tools/perf/builtin-trace.c                |  25 +++++++++-
 tools/perf/tests/builtin-test.c           |   8 ++++
 tools/perf/tests/open-syscall-tp-fields.c |   9 +++-
 tools/perf/tests/perf-record.c            |   4 --
 tools/perf/tests/poller.c                 | 126 ++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/task-exit.c              |   9 +++-
 tools/perf/tests/tests.h                  |   2 +
 tools/perf/tests/xyarray.c                |  33 +++++++++++++
 tools/perf/ui/browser.c                   |   4 +-
 tools/perf/ui/browser.h                   |   8 +++-
 tools/perf/ui/browsers/annotate.c         |   2 +-
 tools/perf/ui/browsers/header.c           |   2 +-
 tools/perf/ui/browsers/hists.c            |  39 ++++++++-------
 tools/perf/ui/browsers/map.c              |   2 +-
 tools/perf/ui/browsers/scripts.c          |   2 +-
 tools/perf/ui/keysyms.h                   |   1 +
 tools/perf/ui/tui/util.c                  |   2 +-
 tools/perf/util/evlist.c                  |  38 ++++++---------
 tools/perf/util/evlist.h                  |   7 ++-
 tools/perf/util/evsel.c                   |  21 +++++++-
 tools/perf/util/evsel.h                   |  10 ++++
 tools/perf/util/hist.h                    |   6 ++-
 tools/perf/util/kvm-stat.h                |   1 -
 tools/perf/util/poller.c                  | 233 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/poller.h                  |  88 ++++++++++++++++++++++++++++++++++
 tools/perf/util/python-ext-sources        |   2 +
 tools/perf/util/python.c                  |  19 ++++++--
 tools/perf/util/util.c                    |  13 +++++
 tools/perf/util/util.h                    |   2 +
 tools/perf/util/xyarray.c                 |   4 +-
 tools/perf/util/xyarray.h                 |   6 +++
 38 files changed, 832 insertions(+), 172 deletions(-)
 create mode 100644 tools/perf/tests/poller.c
 create mode 100644 tools/perf/tests/xyarray.c
 create mode 100644 tools/perf/util/poller.c
 create mode 100644 tools/perf/util/poller.h
#31 lore raw
Quoted 23 lines · Arnaldo Carvalho de Melo

Em Mon, Aug 11, 2014 at 05:12:49PM -0300, Arnaldo Carvalho de Melo escreveu:

Quoted 8 lines

Em Mon, Aug 11, 2014 at 10:49:54AM +0200, Jiri Olsa escreveu:

Quoted 7 lines

hi,
adding support to quit sampling commands:
record,top,trace,kvm stat live

when all the monitored events are closed, like following perf command will now exit when monitored process (pid 1234) exits:

Quoted 7 lines
Quoted 4 lines
  $ perf record -p 1234

I added independent poller object to handle basic polling tasks. I had to factor some parts, so sending this as RFC,

Why? I'm trying to figure out why this poller class is needed, just from reading the changelog entries, no luck so far.

So this is all about accounting for when all events are closed? I.e. as we receive the ERR and HUP we go on closing the entries in the pollfd? Then, when none are closed, then we exit, is that the case?

Why not just mark them -1 and decrement evlist->nr_open_fds (new field)?

Quoted 7 lines
struct pollfds {
	struct pollfd *pollfds;
	int    nr_fds;
	int    nr_open_fds;
};

Don't use callbacks and then that poller_item doesn't need to exist :-\

I think callbacks are convenient when having different kind of events combined in polling, as described in my reply to your first email

Quoted 3 lines

I'll try to cook a patch with those ideas later today, if for anything, to validate your patches :-)

ok ;-) thanks

jirka

#32 lore raw
In reply to Arnaldo Carvalho de Melo
Quoted 8 lines · Arnaldo Carvalho de Melo

Em Mon, Aug 11, 2014 at 10:49:54AM +0200, Jiri Olsa escreveu:

Quoted 7 lines

hi,
adding support to quit sampling commands:
record,top,trace,kvm stat live

when all the monitored events are closed, like following perf command will now exit when monitored process (pid 1234) exits:

Quoted 7 lines
Quoted 4 lines
  $ perf record -p 1234

I added independent poller object to handle basic polling tasks. I had to factor some parts, so sending this as RFC,

Why? I'm trying to figure out why this poller class is needed, just from reading the changelog entries, no luck so far.

multiple reasons.. at the moment:
- have a way to get FD state notification -> callback triggers for FD and we can figure when we are out of monitored events or notify there was error/hup on event

- to be able to gather/register more kinds of FDs under one poll instance, so we could poll together on standard input or other kind of notify event with perf events within a single poll call

for future:
- there's another feature for record to watch hotplug CPUs and open/close perf event for it when CPU is added/removed we need to watch/poll inotify events for that

- when having multiple threads for record command we'll need to split evlist events into more polling instances, so each thread could poll independently

also I think the poller class just encapsulates the polling processing and reduces the evlist complexity

jirka