Re: perf: [patch] regression with PERF_EVENT_IOC_REFRESH

[Vince Weaver]

On Sun, 29 May 2011, Vince Weaver wrote:

> On Sat, 28 May 2011, Peter Zijlstra wrote:
> 
> > On Fri, 2011-05-27 at 23:38 -0400, Vince Weaver wrote:
> > > on that note (and while trying to document exactly what the ioctls do) it 
> > > seems that a PERF_EVENT_IOC_REFRESH with an argument of anything higher 
> > > than one does not work on kernels 2.6.36 and newer.  The behavior acts
> > > as if 1 was passed, even if you pass in, say, 3.
> > 
> > Urgh, no that should definitely work. Thanks for the test-case, I'll
> > work on that (probably not until Monday though, but who knows).
> > 
> 
> after a painfully long bisection, it turns out that this problem was in
> theory introduced by the following commit:
> 
>   d57e34fdd60be7ffd0b1d86bfa1a553df86b7172
> 
>   perf: Simplify the ring-buffer logic: make perf_buffer_alloc() do everything needed
> 
> I'll see if I can come up with a patch, but it's a bit non-obvious why
> this commit is affecting the REFRESH value at all.

the problem was the mentioned commit tried to optimize the use of 
watermark and wakeup_watermark without taking into account that 
wakeup_watermark is a union with wakeup_events.

The patch below *should* fix it, but something unrelated has broken 
overflow support between 2.6.39 and 3.0-rc1 which I haven't had time to 
investigate.  The overflow count is suddenly about 10x what it should be 
though.  So the below is semi-untested and I possibly need to do another 
bisect.  *sigh*

Vince

Signed-off-by: Vince Weaver <vweaver1@xxxxxxxxxxxxxxxxxx>


diff --git a/kernel/events/core.c b/kernel/events/core.c
index d863b3c..e3ff283 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3403,12 +3403,14 @@ unlock:
 static unsigned long perf_data_size(struct perf_buffer *buffer);
 
 static void
-perf_buffer_init(struct perf_buffer *buffer, long watermark, int flags)
+perf_buffer_init(struct perf_buffer *buffer, 
+		 long watermark, 
+		 long wakeup_watermark, int flags)
 {
 	long max_size = perf_data_size(buffer);
 
 	if (watermark)
-		buffer->watermark = min(max_size, watermark);
+		buffer->watermark = min(max_size, wakeup_watermark);
 
 	if (!buffer->watermark)
 		buffer->watermark = max_size / 2;
@@ -3451,7 +3453,8 @@ static void *perf_mmap_alloc_page(int cpu)
 }
 
 static struct perf_buffer *
-perf_buffer_alloc(int nr_pages, long watermark, int cpu, int flags)
+perf_buffer_alloc(int nr_pages, long watermark, 
+		  long wakeup_watermark, int cpu, int flags)
 {
 	struct perf_buffer *buffer;
 	unsigned long size;
@@ -3476,7 +3479,7 @@ perf_buffer_alloc(int nr_pages, long watermark, int cpu, int flags)
 
 	buffer->nr_pages = nr_pages;
 
-	perf_buffer_init(buffer, watermark, flags);
+	perf_buffer_init(buffer, watermark, wakeup_watermark, flags);
 
 	return buffer;
 
@@ -3568,7 +3571,8 @@ static void perf_buffer_free(struct perf_buffer *buffer)
 }
 
 static struct perf_buffer *
-perf_buffer_alloc(int nr_pages, long watermark, int cpu, int flags)
+perf_buffer_alloc(int nr_pages, long watermark, 
+		  long wakeup_watermark, int cpu, int flags)
 {
 	struct perf_buffer *buffer;
 	unsigned long size;
@@ -3592,7 +3596,7 @@ perf_buffer_alloc(int nr_pages, long watermark, int cpu, int flags)
 	buffer->page_order = ilog2(nr_pages);
 	buffer->nr_pages = 1;
 
-	perf_buffer_init(buffer, watermark, flags);
+	perf_buffer_init(buffer, watermark, wakeup_watermark, flags);
 
 	return buffer;
 
@@ -3787,7 +3791,9 @@ 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,
 				   event->cpu, flags);
 	if (!buffer) {
 		ret = -ENOMEM;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/