Re: [PATCH v2 1/1] ftrace: do not update max buffer with no users

[Ingo Molnar <mingo@elte.hu>]

* Steven Rostedt <rostedt@goodmis.org> wrote:

> 
> On Thu, 13 Nov 2008, Ingo Molnar wrote:
> > 
> > the obvious solution is to add this to ring_buffer_resize():
> > 
> > 	if (!buffer)
> > 		return size;
> 
> Having a NULL buffer return a successful resize is a bit worrisome to me.
> 
> And looking at the code I was trying to make sure could never be called
> if there are no max_tr users:
> 
> in update_max_tr
> 
> 	buf = tr->buffer;
> 	tr->buffer = max_tr.buffer;
> 	max_tr.buffer = buf;
> 
> Should all the ring buffer API return success on NULL pointers?

well, update_max_tr() is only used from tracers which also actually 
allocate a max trace buffer: irqs/preemptoff, sched-wakeup.

So, can you tell me any way how the patch below could be broken?

	Ingo

------------->
>From ee51a1de7e3837577412be269e0100038068e691 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@elte.hu>
Date: Thu, 13 Nov 2008 14:58:31 +0100
Subject: [PATCH] tracing: fix mmiotrace resizing crash

Pekka reported a crash when resizing the mmiotrace tracer (if only
mmiotrace is enabled).

This happens because in that case we do not allocate the max buffer,
but we try to use it.

Make ring_buffer_resize() idempotent against NULL buffers.

Reported-by: Pekka Paalanen <pq@iki.fi>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/trace/ring_buffer.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 231db20..036456c 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -538,6 +538,12 @@ int ring_buffer_resize(struct ring_buffer *buffer, unsigned long size)
 	LIST_HEAD(pages);
 	int i, cpu;
 
+	/*
+	 * Always succeed at resizing a non-existent buffer:
+	 */
+	if (!buffer)
+		return size;
+
 	size = DIV_ROUND_UP(size, BUF_PAGE_SIZE);
 	size *= BUF_PAGE_SIZE;
 	buffer_size = buffer->pages * BUF_PAGE_SIZE;