public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [patch] perf_events: fix perf buffer watermark setting
@ 2011-06-01 19:15 Vince Weaver
  2011-07-01 15:18 ` [tip:perf/core] perf_events: Fix " tip-bot for Vince Weaver
  0 siblings, 1 reply; 2+ messages in thread
From: Vince Weaver @ 2011-06-01 19:15 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: linux-kernel, mingo, paulus, acme


Since 2.6.36 (specifically commit d57e34fdd60be7ffd0b1d86bfa1a553df86b7172)
the perf_buffer_init_code() has been mis-setting the buffer watermark if
perf_event_attr.wakeup_events has a non-zero value.

This is because perf_event_attr.wakeup_events is a union with 
perf_event_attr.wakeup_watermark.

This commit re-enables the check for perf_event_attr.watermark being
set before continuing with setting a non-default watermark.

This bug is most noticable when you are trying to use PERF_IOC_REFRESH
with a value larger than one and perf_event_attr.wakeup_events is set to 
one.  In this case the buffer watermark will be set to 1 and you will
get extraneous POLL_IN overflows rather than POLL_HUP as expected.

Signed-off-by: Vince Weaver <vweaver1@eecs.utk.edu>


diff --git a/kernel/events/core.c b/kernel/events/core.c
index d863b3c..b949661 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3787,7 +3787,8 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 	if (vma->vm_flags & VM_WRITE)
 		flags |= PERF_BUFFER_WRITABLE;
 
-	buffer = perf_buffer_alloc(nr_pages, event->attr.wakeup_watermark,
+	buffer = perf_buffer_alloc(nr_pages, 
+				   event->attr.watermark?event->attr.wakeup_watermark:0,
 				   event->cpu, flags);
 	if (!buffer) {
 		ret = -ENOMEM;

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [tip:perf/core] perf_events: Fix perf buffer watermark setting
  2011-06-01 19:15 [patch] perf_events: fix perf buffer watermark setting Vince Weaver
@ 2011-07-01 15:18 ` tip-bot for Vince Weaver
  0 siblings, 0 replies; 2+ messages in thread
From: tip-bot for Vince Weaver @ 2011-07-01 15:18 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, stable, tglx, vweaver1,
	mingo

Commit-ID:  4ec8363dfc1451f8c8f86825731fe712798ada02
Gitweb:     http://git.kernel.org/tip/4ec8363dfc1451f8c8f86825731fe712798ada02
Author:     Vince Weaver <vweaver1@eecs.utk.edu>
AuthorDate: Wed, 1 Jun 2011 15:15:36 -0400
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Fri, 1 Jul 2011 11:06:32 +0200

perf_events: Fix perf buffer watermark setting

Since 2.6.36 (specifically commit d57e34fdd60b ("perf: Simplify the
ring-buffer logic: make perf_buffer_alloc() do everything needed"),
the perf_buffer_init_code() has been mis-setting the buffer watermark
if perf_event_attr.wakeup_events has a non-zero value.

This is because perf_event_attr.wakeup_events is a union with
perf_event_attr.wakeup_watermark.

This commit re-enables the check for perf_event_attr.watermark being
set before continuing with setting a non-default watermark.

This bug is most noticable when you are trying to use PERF_IOC_REFRESH
with a value larger than one and perf_event_attr.wakeup_events is set to
one.  In this case the buffer watermark will be set to 1 and you will
get extraneous POLL_IN overflows rather than POLL_HUP as expected.

[ avoid using attr.wakeup_events when attr.watermark is set ]

Signed-off-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/r/alpine.DEB.2.00.1106011506390.5384@cl320.eecs.utk.edu
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/events/core.c        |    6 ++++--
 kernel/events/ring_buffer.c |   16 +++++++++-------
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 5e70f62..e4aee51 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3569,8 +3569,10 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 	if (vma->vm_flags & VM_WRITE)
 		flags |= RING_BUFFER_WRITABLE;
 
-	rb = rb_alloc(nr_pages, event->attr.wakeup_watermark,
-				   event->cpu, flags);
+	rb = rb_alloc(nr_pages, 
+		event->attr.watermark ? event->attr.wakeup_watermark : 0,
+		event->cpu, flags);
+
 	if (!rb) {
 		ret = -ENOMEM;
 		goto unlock;
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index fde5259..fc2701c 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -199,13 +199,15 @@ void perf_output_end(struct perf_output_handle *handle)
 	struct perf_event *event = handle->event;
 	struct ring_buffer *rb = handle->rb;
 
-	int wakeup_events = event->attr.wakeup_events;
-
-	if (handle->sample && wakeup_events) {
-		int events = local_inc_return(&rb->events);
-		if (events >= wakeup_events) {
-			local_sub(wakeup_events, &rb->events);
-			local_inc(&rb->wakeup);
+	if (handle->sample && !event->attr.watermark) {
+		int wakeup_events = event->attr.wakeup_events;
+
+		if (wakeup_events) {
+			int events = local_inc_return(&rb->events);
+			if (events >= wakeup_events) {
+				local_sub(wakeup_events, &rb->events);
+				local_inc(&rb->wakeup);
+			}
 		}
 	}
 

^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2011-07-01 15:19 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-01 19:15 [patch] perf_events: fix perf buffer watermark setting Vince Weaver
2011-07-01 15:18 ` [tip:perf/core] perf_events: Fix " tip-bot for Vince Weaver

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox