On Mon, Aug 11, 2014 at 10:49:55AM +0200, Jiri Olsa wrote:
> ---
> include/linux/perf_event.h | 1 +
> kernel/events/core.c | 12 ++++++++++--
> 2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 707617a8c0f6..54f3a6241386 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -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,
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 14086e45c5c4..dde0eefa410a 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -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;
Should not that be an independent bugfix? It is a silly little thing
indeed, but it does change behaviour.
> 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?
> /*
> * 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.
pgp0bVBhcbTKR.pgp
Description: PGP signature
