On 2017/07/12 01:46PM, Peter Zijlstra wrote:
> On Mon, Jun 19, 2017 at 08:01:07PM +0530, Naveen N. Rao wrote:
>
> > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > index 6c4e523dc1e2..812fcfc767f4 100644
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -2679,7 +2679,8 @@ static int _perf_event_refresh(struct perf_event
> > *event, int refresh)
> > /*
> > * not supported on inherited events
>
> That comment wants updating to explain the signal crud..
Ok sure. Will do.
>
> > */
> > - if (event->attr.inherit || !is_sampling_event(event))
> > + if (event->attr.inherit || event->attr.signal_on_wakeup ||
> > + !is_sampling_event(event))
> > return -EINVAL;
> >
> > atomic_add(refresh, &event->event_limit);
>
> > @@ -7362,12 +7362,15 @@ static int __perf_event_overflow(struct perf_event
> > *event,
> > * events
> > */
> >
> > + if (!event->attr.signal_on_wakeup) {
> > + int events = atomic_read(&event->event_limit);
> > + event->pending_kill = POLL_IN;
> > + if (events && atomic_dec_and_test(&event->event_limit)) {
> > + ret = 1;
> > + event->pending_kill = POLL_HUP;
> >
> > + perf_event_disable_inatomic(event);
> > + }
> > }
>
> So even without event_limit (IOC_REFRESH) this would've generated
> SIGIO:POLL_IN.
Yes, this still does if signal_on_wakeup isn't set. However, if
signal_on_wakeup is set, we follow the ring buffer notification which
will generate a POLL_IN controlled by {wakeup_events, wakeup_watermark}.
>
> >
> > READ_ONCE(event->overflow_handler)(event, data, regs);
> > @@ -10408,6 +10411,7 @@ perf_event_exit_event(struct perf_event
> > *child_event,
> > perf_group_detach(child_event);
> > list_del_event(child_event, child_ctx);
> > child_event->state = PERF_EVENT_STATE_EXIT; /* is_event_hup() */
> > + child_event->pending_kill = POLL_HUP;
>
> This looks like an undocumented change..
Yes, sorry - I should have added a note.
This comes from Jiri's feedback that if user chooses to have a signal
delivered on ring buffer wakeup, we should also send a HUP on exit.
>
> > raw_spin_unlock_irq(&child_ctx->lock);
> >
> > /*
> > diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
> > index 2831480c63a2..4e7c728569a8 100644
> > --- a/kernel/events/ring_buffer.c
> > +++ b/kernel/events/ring_buffer.c
> > @@ -21,6 +21,9 @@ static void perf_output_wakeup(struct perf_output_handle
> > *handle)
> > {
> > atomic_set(&handle->rb->poll, POLLIN);
> >
> > + if (handle->event->attr.signal_on_wakeup)
> > + handle->event->pending_kill = POLL_IN;
> > +
>
> And this is the bit that generates SIGIO:POLL_IN on wakeup.
Yes.
Thanks,
Naveen
>
> > handle->event->pending_wakeup = 1;
> > irq_work_queue(&handle->event->pending);
> > }
> > --
> > 2.13.1
> >
>
