Commit-ID: f506b3dc0ec454a16d40cab9ee5d75435b39dc50
Gitweb: http://git.kernel.org/tip/f506b3dc0ec454a16d40cab9ee5d75435b39dc50
Author: Peter Zijlstra <[email protected]>
AuthorDate: Thu, 26 May 2011 17:02:53 +0200
Committer: Ingo Molnar <[email protected]>
CommitDate: Sat, 28 May 2011 17:04:59 +0200
perf: Fix SIGIO handling
Vince noticed that unless we mmap() a buffer, SIGIO gets lost. So
explicitly push the wakeup (including signals) when requested.
Reported-by: Vince Weaver <[email protected]>
Signed-off-by: Peter Zijlstra <[email protected]>
Cc: <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/events/core.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index c09767f..d863b3c 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5028,6 +5028,14 @@ static int __perf_event_overflow(struct perf_event *event, int nmi,
else
perf_event_output(event, nmi, data, regs);
+ if (event->fasync && event->pending_kill) {
+ if (nmi) {
+ event->pending_wakeup = 1;
+ irq_work_queue(&event->pending);
+ } else
+ perf_event_wakeup(event);
+ }
+
return ret;
}
On Sat, 28 May 2011, tip-bot for Peter Zijlstra wrote:
> Commit-ID: f506b3dc0ec454a16d40cab9ee5d75435b39dc50
> Gitweb: http://git.kernel.org/tip/f506b3dc0ec454a16d40cab9ee5d75435b39dc50
> Author: Peter Zijlstra <[email protected]>
> AuthorDate: Thu, 26 May 2011 17:02:53 +0200
> Committer: Ingo Molnar <[email protected]>
> CommitDate: Sat, 28 May 2011 17:04:59 +0200
>
> perf: Fix SIGIO handling
>
> Vince noticed that unless we mmap() a buffer, SIGIO gets lost. So
> explicitly push the wakeup (including signals) when requested.
>
> Reported-by: Vince Weaver <[email protected]>
> Signed-off-by: Peter Zijlstra <[email protected]>
> Cc: <[email protected]>
> Link: http://lkml.kernel.org/n/[email protected]
> Signed-off-by: Ingo Molnar <[email protected]>
> ---
> kernel/events/core.c | 8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index c09767f..d863b3c 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -5028,6 +5028,14 @@ static int __perf_event_overflow(struct perf_event *event, int nmi,
> else
> perf_event_output(event, nmi, data, regs);
>
> + if (event->fasync && event->pending_kill) {
> + if (nmi) {
> + event->pending_wakeup = 1;
> + irq_work_queue(&event->pending);
> + } else
> + perf_event_wakeup(event);
> + }
> +
> return ret;
> }
there is something strange about this commit.
With it I get many more overflows than expected (on the order of 40x more)
when I am using PERF_IOC_REFRESH,1 to restart the sampled counter
from the end of the signal handler.
Also if you use PERF_IOC_REFRESH with a value other than 1, the value
seems to be ignored and all of the counts are POLL_IN rather than
POLL_HUP as I'd expect.
Vince
On Wed, 2011-06-01 at 15:41 -0400, Vince Weaver wrote:
> On Sat, 28 May 2011, tip-bot for Peter Zijlstra wrote:
>
> > Commit-ID: f506b3dc0ec454a16d40cab9ee5d75435b39dc50
> > Gitweb: http://git.kernel.org/tip/f506b3dc0ec454a16d40cab9ee5d75435b39dc50
> > Author: Peter Zijlstra <[email protected]>
> > AuthorDate: Thu, 26 May 2011 17:02:53 +0200
> > Committer: Ingo Molnar <[email protected]>
> > CommitDate: Sat, 28 May 2011 17:04:59 +0200
> >
> > perf: Fix SIGIO handling
> >
> > Vince noticed that unless we mmap() a buffer, SIGIO gets lost. So
> > explicitly push the wakeup (including signals) when requested.
> >
> > Reported-by: Vince Weaver <[email protected]>
> > Signed-off-by: Peter Zijlstra <[email protected]>
> > Cc: <[email protected]>
> > Link: http://lkml.kernel.org/n/[email protected]
> > Signed-off-by: Ingo Molnar <[email protected]>
> > ---
> > kernel/events/core.c | 8 ++++++++
> > 1 files changed, 8 insertions(+), 0 deletions(-)
> >
> > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > index c09767f..d863b3c 100644
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -5028,6 +5028,14 @@ static int __perf_event_overflow(struct perf_event *event, int nmi,
> > else
> > perf_event_output(event, nmi, data, regs);
> >
> > + if (event->fasync && event->pending_kill) {
> > + if (nmi) {
> > + event->pending_wakeup = 1;
> > + irq_work_queue(&event->pending);
> > + } else
> > + perf_event_wakeup(event);
> > + }
> > +
> > return ret;
> > }
>
> there is something strange about this commit.
>
> With it I get many more overflows than expected (on the order of 40x more)
> when I am using PERF_IOC_REFRESH,1 to restart the sampled counter
> from the end of the signal handler.
>
> Also if you use PERF_IOC_REFRESH with a value other than 1, the value
> seems to be ignored and all of the counts are POLL_IN rather than
> POLL_HUP as I'd expect.
OK, so what semantics do we expect?
Currently (with this patch) when a SIGIO is registered, every event
overflow (sample) generates a POLL_IN SIGIO, except when event_limit
disables the counter, in which case its a POLL_HUP.
Without the patch, we used to send POLL_IN on every wakeup that would
wake poll() and POLL_HUP every time event_limit was reached.
event_limit is incremented using IOC_REFRESH, when its non-zero its
decremented on every overflow, and when it reaches 0 again it fires
SIGIO-POLL_HUP and disables the event.
On Tue, 2011-06-07 at 12:15 +0200, Peter Zijlstra wrote:
> On Wed, 2011-06-01 at 15:41 -0400, Vince Weaver wrote:
> > On Sat, 28 May 2011, tip-bot for Peter Zijlstra wrote:
> >
> > > Commit-ID: f506b3dc0ec454a16d40cab9ee5d75435b39dc50
> > > Gitweb: http://git.kernel.org/tip/f506b3dc0ec454a16d40cab9ee5d75435b39dc50
> > > Author: Peter Zijlstra <[email protected]>
> > > AuthorDate: Thu, 26 May 2011 17:02:53 +0200
> > > Committer: Ingo Molnar <[email protected]>
> > > CommitDate: Sat, 28 May 2011 17:04:59 +0200
> > >
> > > perf: Fix SIGIO handling
> > >
> > > Vince noticed that unless we mmap() a buffer, SIGIO gets lost. So
> > > explicitly push the wakeup (including signals) when requested.
> > >
> > > Reported-by: Vince Weaver <[email protected]>
> > > Signed-off-by: Peter Zijlstra <[email protected]>
> > > Cc: <[email protected]>
> > > Link: http://lkml.kernel.org/n/[email protected]
> > > Signed-off-by: Ingo Molnar <[email protected]>
> > > ---
> > > kernel/events/core.c | 8 ++++++++
> > > 1 files changed, 8 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > > index c09767f..d863b3c 100644
> > > --- a/kernel/events/core.c
> > > +++ b/kernel/events/core.c
> > > @@ -5028,6 +5028,14 @@ static int __perf_event_overflow(struct perf_event *event, int nmi,
> > > else
> > > perf_event_output(event, nmi, data, regs);
> > >
> > > + if (event->fasync && event->pending_kill) {
> > > + if (nmi) {
> > > + event->pending_wakeup = 1;
> > > + irq_work_queue(&event->pending);
> > > + } else
> > > + perf_event_wakeup(event);
> > > + }
> > > +
> > > return ret;
> > > }
> >
> > there is something strange about this commit.
> >
> > With it I get many more overflows than expected (on the order of 40x more)
> > when I am using PERF_IOC_REFRESH,1 to restart the sampled counter
> > from the end of the signal handler.
> >
> > Also if you use PERF_IOC_REFRESH with a value other than 1, the value
> > seems to be ignored and all of the counts are POLL_IN rather than
> > POLL_HUP as I'd expect.
>
> OK, so what semantics do we expect?
>
> Currently (with this patch) when a SIGIO is registered, every event
> overflow (sample) generates a POLL_IN SIGIO, except when event_limit
> disables the counter, in which case its a POLL_HUP.
>
> Without the patch, we used to send POLL_IN on every wakeup that would
> wake poll() and POLL_HUP every time event_limit was reached.
>
> event_limit is incremented using IOC_REFRESH, when its non-zero its
> decremented on every overflow, and when it reaches 0 again it fires
> SIGIO-POLL_HUP and disables the event.
Also, I realized we could get the current behaviour by setting
attr.wakeup_events = 1.
So what should we do, revert the above patch?