All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.