* [tip:perf/urgent] perf: Fix SIGIO handling @ 2011-05-28 15:42 tip-bot for Peter Zijlstra 2011-06-01 19:41 ` Vince Weaver 0 siblings, 1 reply; 4+ messages in thread From: tip-bot for Peter Zijlstra @ 2011-05-28 15:42 UTC (permalink / raw) To: linux-tip-commits Cc: linux-kernel, hpa, mingo, a.p.zijlstra, stable, tglx, vweaver1, mingo Commit-ID: f506b3dc0ec454a16d40cab9ee5d75435b39dc50 Gitweb: http://git.kernel.org/tip/f506b3dc0ec454a16d40cab9ee5d75435b39dc50 Author: Peter Zijlstra <a.p.zijlstra@chello.nl> AuthorDate: Thu, 26 May 2011 17:02:53 +0200 Committer: Ingo Molnar <mingo@elte.hu> 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 <vweaver1@eecs.utk.edu> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl> Cc: <stable@kernel.org> Link: http://lkml.kernel.org/n/tip-2euus3f3x3dyvdk52cjxw8zu@git.kernel.org Signed-off-by: Ingo Molnar <mingo@elte.hu> --- 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; } ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [tip:perf/urgent] perf: Fix SIGIO handling 2011-05-28 15:42 [tip:perf/urgent] perf: Fix SIGIO handling tip-bot for Peter Zijlstra @ 2011-06-01 19:41 ` Vince Weaver 2011-06-07 10:15 ` Peter Zijlstra 0 siblings, 1 reply; 4+ messages in thread From: Vince Weaver @ 2011-06-01 19:41 UTC (permalink / raw) To: mingo, hpa, linux-kernel, a.p.zijlstra, tglx, mingo On Sat, 28 May 2011, tip-bot for Peter Zijlstra wrote: > Commit-ID: f506b3dc0ec454a16d40cab9ee5d75435b39dc50 > Gitweb: http://git.kernel.org/tip/f506b3dc0ec454a16d40cab9ee5d75435b39dc50 > Author: Peter Zijlstra <a.p.zijlstra@chello.nl> > AuthorDate: Thu, 26 May 2011 17:02:53 +0200 > Committer: Ingo Molnar <mingo@elte.hu> > 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 <vweaver1@eecs.utk.edu> > Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl> > Cc: <stable@kernel.org> > Link: http://lkml.kernel.org/n/tip-2euus3f3x3dyvdk52cjxw8zu@git.kernel.org > Signed-off-by: Ingo Molnar <mingo@elte.hu> > --- > 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [tip:perf/urgent] perf: Fix SIGIO handling 2011-06-01 19:41 ` Vince Weaver @ 2011-06-07 10:15 ` Peter Zijlstra 2011-06-27 13:09 ` Peter Zijlstra 0 siblings, 1 reply; 4+ messages in thread From: Peter Zijlstra @ 2011-06-07 10:15 UTC (permalink / raw) To: Vince Weaver; +Cc: mingo, hpa, linux-kernel, tglx, mingo 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 <a.p.zijlstra@chello.nl> > > AuthorDate: Thu, 26 May 2011 17:02:53 +0200 > > Committer: Ingo Molnar <mingo@elte.hu> > > 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 <vweaver1@eecs.utk.edu> > > Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl> > > Cc: <stable@kernel.org> > > Link: http://lkml.kernel.org/n/tip-2euus3f3x3dyvdk52cjxw8zu@git.kernel.org > > Signed-off-by: Ingo Molnar <mingo@elte.hu> > > --- > > 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. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [tip:perf/urgent] perf: Fix SIGIO handling 2011-06-07 10:15 ` Peter Zijlstra @ 2011-06-27 13:09 ` Peter Zijlstra 0 siblings, 0 replies; 4+ messages in thread From: Peter Zijlstra @ 2011-06-27 13:09 UTC (permalink / raw) To: Vince Weaver; +Cc: mingo, hpa, linux-kernel, tglx, mingo 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 <a.p.zijlstra@chello.nl> > > > AuthorDate: Thu, 26 May 2011 17:02:53 +0200 > > > Committer: Ingo Molnar <mingo@elte.hu> > > > 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 <vweaver1@eecs.utk.edu> > > > Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl> > > > Cc: <stable@kernel.org> > > > Link: http://lkml.kernel.org/n/tip-2euus3f3x3dyvdk52cjxw8zu@git.kernel.org > > > Signed-off-by: Ingo Molnar <mingo@elte.hu> > > > --- > > > 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? ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-06-27 13:11 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-05-28 15:42 [tip:perf/urgent] perf: Fix SIGIO handling tip-bot for Peter Zijlstra 2011-06-01 19:41 ` Vince Weaver 2011-06-07 10:15 ` Peter Zijlstra 2011-06-27 13:09 ` Peter Zijlstra
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).