[PATCH] tracing: shrink max latency ringbuffer if unnecessary

[PATCH] tracing: shrink max latency ringbuffer if unnecessary

[KOSAKI Motohiro]

Documentation/trace/ftrace.txt says

  buffer_size_kb:

        This sets or displays the number of kilobytes each CPU
        buffer can hold. The tracer buffers are the same size
        for each CPU. The displayed number is the size of the
        CPU buffer and not total size of all buffers. The
        trace buffers are allocated in pages (blocks of memory
        that the kernel uses for allocation, usually 4 KB in size).
        If the last page allocated has room for more bytes
        than requested, the rest of the page will be used,
        making the actual allocation bigger than requested.
        ( Note, the size may not be a multiple of the page size
          due to buffer management overhead. )

        This can only be updated when the current_tracer
        is set to "nop".

But it's incorrect. currently total memory consumption is
'buffer_size_kb x CPUs x 2'.

Why two times difference is there? because ftrace implicitly allocate
the buffer for max latency too.

That makes sad result when admin want to use large buffer. (If admin
want full logging and makes detail analysis). example, If admin
have 24 CPUs machine and write 200MB to buffer_size_kb, the system
consume ~10GB memory (200MB x 24 x 2). umm.. 5GB memory waste is
usually unacceptable.

Fortunatelly, almost all users don't use max latency feature.
The max latency buffer can be disabled easily.

This patch shrink buffer size of the max latency buffer if
unnecessary.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
 kernel/trace/trace.c              |   22 +++++++++++++---------
 kernel/trace/trace.h              |    1 +
 kernel/trace/trace_irqsoff.c      |    9 +++++++++
 kernel/trace/trace_sched_wakeup.c |    9 +++++++++
 4 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 8683dec..ab2c061 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2782,7 +2782,7 @@ int tracer_init(struct tracer *t, struct trace_array *tr)
 	return t->init(tr);
 }
 
-static int tracing_resize_ring_buffer(unsigned long size)
+int tracing_resize_ring_buffer(unsigned long size, int need_max_tr)
 {
 	int ret;
 
@@ -2797,6 +2797,11 @@ static int tracing_resize_ring_buffer(unsigned long size)
 	if (ret < 0)
 		return ret;
 
+	if (!need_max_tr) {
+		ring_buffer_resize(max_tr.buffer, 1);
+		goto out;
+	}
+
 	ret = ring_buffer_resize(max_tr.buffer, size);
 	if (ret < 0) {
 		int r;
@@ -2824,7 +2829,9 @@ static int tracing_resize_ring_buffer(unsigned long size)
 		return ret;
 	}
 
+ out:
 	global_trace.entries = size;
+	max_tr.entries = need_max_tr ? size : 1;
 
 	return ret;
 }
@@ -2845,7 +2852,7 @@ int tracing_update_buffers(void)
 
 	mutex_lock(&trace_types_lock);
 	if (!ring_buffer_expanded)
-		ret = tracing_resize_ring_buffer(trace_buf_size);
+		ret = tracing_resize_ring_buffer(trace_buf_size, 0);
 	mutex_unlock(&trace_types_lock);
 
 	return ret;
@@ -2869,7 +2876,7 @@ static int tracing_set_tracer(const char *buf)
 	mutex_lock(&trace_types_lock);
 
 	if (!ring_buffer_expanded) {
-		ret = tracing_resize_ring_buffer(trace_buf_size);
+		ret = tracing_resize_ring_buffer(trace_buf_size, 0);
 		if (ret < 0)
 			goto out;
 		ret = 0;
@@ -3455,7 +3462,7 @@ tracing_entries_write(struct file *filp, const char __user *ubuf,
 	val <<= 10;
 
 	if (val != global_trace.entries) {
-		ret = tracing_resize_ring_buffer(val);
+		ret = tracing_resize_ring_buffer(val, max_tr.entries!=1);
 		if (ret < 0) {
 			cnt = ret;
 			goto out;
@@ -3476,7 +3483,6 @@ tracing_entries_write(struct file *filp, const char __user *ubuf,
 	}
 
 	tracing_start();
-	max_tr.entries = global_trace.entries;
 	mutex_unlock(&trace_types_lock);
 
 	return cnt;
@@ -4574,16 +4580,14 @@ __init static int tracer_alloc_buffers(void)
 
 
 #ifdef CONFIG_TRACER_MAX_TRACE
-	max_tr.buffer = ring_buffer_alloc(ring_buf_size,
-					     TRACE_BUFFER_FLAGS);
+	max_tr.buffer = ring_buffer_alloc(1, TRACE_BUFFER_FLAGS);
 	if (!max_tr.buffer) {
 		printk(KERN_ERR "tracer: failed to allocate max ring buffer!\n");
 		WARN_ON(1);
 		ring_buffer_free(global_trace.buffer);
 		goto out_free_cpumask;
 	}
-	max_tr.entries = ring_buffer_size(max_tr.buffer);
-	WARN_ON(max_tr.entries != global_trace.entries);
+	max_tr.entries = 1;
 #endif
 
 	/* Allocate the first page for all buffers */
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 01ce088..78309e2 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -284,6 +284,7 @@ struct tracer {
 #define TRACE_PIPE_ALL_CPU	-1
 
 int tracer_init(struct tracer *t, struct trace_array *tr);
+int tracing_resize_ring_buffer(unsigned long size, int need_max_tr);
 int tracing_is_enabled(void);
 void trace_wake_up(void);
 void tracing_reset(struct trace_array *tr, int cpu);
diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
index 6fd486e..a3668ca 100644
--- a/kernel/trace/trace_irqsoff.c
+++ b/kernel/trace/trace_irqsoff.c
@@ -612,6 +612,9 @@ static void irqsoff_tracer_reset(struct trace_array *tr)
 
 	if (!save_lat_flag)
 		trace_flags &= ~TRACE_ITER_LATENCY_FMT;
+
+	/* shrink max_tr buffer */
+	tracing_resize_ring_buffer(tr->entries, 0);
 }
 
 static void irqsoff_tracer_start(struct trace_array *tr)
@@ -692,6 +695,12 @@ static struct tracer preemptoff_tracer __read_mostly =
 
 static int preemptirqsoff_tracer_init(struct trace_array *tr)
 {
+	int ret;
+
+	ret = tracing_resize_ring_buffer(tr->entries, 1);
+	if (ret <0)
+		return ret;
+
 	trace_type = TRACER_IRQS_OFF | TRACER_PREEMPT_OFF;
 
 	__irqsoff_tracer_init(tr);
diff --git a/kernel/trace/trace_sched_wakeup.c b/kernel/trace/trace_sched_wakeup.c
index c9fd5bd..c8f0d1d 100644
--- a/kernel/trace/trace_sched_wakeup.c
+++ b/kernel/trace/trace_sched_wakeup.c
@@ -329,6 +329,12 @@ static void stop_wakeup_tracer(struct trace_array *tr)
 
 static int __wakeup_tracer_init(struct trace_array *tr)
 {
+	int ret;
+
+	ret = tracing_resize_ring_buffer(tr->entries, 1);
+	if (ret <0)
+		return ret;
+
 	save_lat_flag = trace_flags & TRACE_ITER_LATENCY_FMT;
 	trace_flags |= TRACE_ITER_LATENCY_FMT;
 
@@ -358,6 +364,9 @@ static void wakeup_tracer_reset(struct trace_array *tr)
 
 	if (!save_lat_flag)
 		trace_flags &= ~TRACE_ITER_LATENCY_FMT;
+
+	/* shrink max_tr buffer */
+	tracing_resize_ring_buffer(tr->entries, 0);
 }
 
 static void wakeup_tracer_start(struct trace_array *tr)
-- 
1.6.5.2

Re: [PATCH] tracing: shrink max latency ringbuffer if unnecessary

[Steven Rostedt]

Hi Kosaki,

FYI, could you send emails to my goodmis account. I can easily miss
emails sent to my RH account since it is usually flooded with RH
Bugzilla reports.

(more below)

On Wed, 2010-06-30 at 12:06 +0900, KOSAKI Motohiro wrote:
> Documentation/trace/ftrace.txt says
> 
>   buffer_size_kb:
> 
>         This sets or displays the number of kilobytes each CPU
>         buffer can hold. The tracer buffers are the same size
>         for each CPU. The displayed number is the size of the
>         CPU buffer and not total size of all buffers. The
>         trace buffers are allocated in pages (blocks of memory
>         that the kernel uses for allocation, usually 4 KB in size).
>         If the last page allocated has room for more bytes
>         than requested, the rest of the page will be used,
>         making the actual allocation bigger than requested.
>         ( Note, the size may not be a multiple of the page size
>           due to buffer management overhead. )
> 
>         This can only be updated when the current_tracer
>         is set to "nop".
> 
> But it's incorrect. currently total memory consumption is
> 'buffer_size_kb x CPUs x 2'.
> 
> Why two times difference is there? because ftrace implicitly allocate
> the buffer for max latency too.
> 
> That makes sad result when admin want to use large buffer. (If admin
> want full logging and makes detail analysis). example, If admin
> have 24 CPUs machine and write 200MB to buffer_size_kb, the system
> consume ~10GB memory (200MB x 24 x 2). umm.. 5GB memory waste is
> usually unacceptable.
> 
> Fortunatelly, almost all users don't use max latency feature.
> The max latency buffer can be disabled easily.
> 
> This patch shrink buffer size of the max latency buffer if
> unnecessary.

Actually, what would be better is to add a "use_max_tr" field to the
struct tracer in trace.h.  Then the latency tracers (irqsoff,
preemptoff, preemptirqsoff, wakeup, and wakeup_rt) can have this field
set.

Then, we can resize or even remove the max ring buffer when the
"use_max_tr" is not set (and on bootup). On enabling a latency tracer,
we can allocate the buffer. When we enable another tracer (or nop) if
the use_max_tr is not set, then we can remove the buffer.

Would you be able to do something like that?

Thanks,

-- Steve

[PATCH v2] tracing: shrink max latency ringbuffer if unnecessary

[KOSAKI Motohiro]

> Hi Kosaki,
> 
> FYI, could you send emails to my goodmis account. I can easily miss
> emails sent to my RH account since it is usually flooded with RH
> Bugzilla reports.

Ahh sorry. yes, I had hear the same thing. I did forgot it.
I've fixed my adress book ;)

> 
> (more below)
> 
> On Wed, 2010-06-30 at 12:06 +0900, KOSAKI Motohiro wrote:
> > Documentation/trace/ftrace.txt says
> > 
> >   buffer_size_kb:
> > 
> >         This sets or displays the number of kilobytes each CPU
> >         buffer can hold. The tracer buffers are the same size
> >         for each CPU. The displayed number is the size of the
> >         CPU buffer and not total size of all buffers. The
> >         trace buffers are allocated in pages (blocks of memory
> >         that the kernel uses for allocation, usually 4 KB in size).
> >         If the last page allocated has room for more bytes
> >         than requested, the rest of the page will be used,
> >         making the actual allocation bigger than requested.
> >         ( Note, the size may not be a multiple of the page size
> >           due to buffer management overhead. )
> > 
> >         This can only be updated when the current_tracer
> >         is set to "nop".
> > 
> > But it's incorrect. currently total memory consumption is
> > 'buffer_size_kb x CPUs x 2'.
> > 
> > Why two times difference is there? because ftrace implicitly allocate
> > the buffer for max latency too.
> > 
> > That makes sad result when admin want to use large buffer. (If admin
> > want full logging and makes detail analysis). example, If admin
> > have 24 CPUs machine and write 200MB to buffer_size_kb, the system
> > consume ~10GB memory (200MB x 24 x 2). umm.. 5GB memory waste is
> > usually unacceptable.
> > 
> > Fortunatelly, almost all users don't use max latency feature.
> > The max latency buffer can be disabled easily.
> > 
> > This patch shrink buffer size of the max latency buffer if
> > unnecessary.
> 
> Actually, what would be better is to add a "use_max_tr" field to the
> struct tracer in trace.h.  Then the latency tracers (irqsoff,
> preemptoff, preemptirqsoff, wakeup, and wakeup_rt) can have this field
> set.

Ah, Cool!


> 
> Then, we can resize or even remove the max ring buffer when the
> "use_max_tr" is not set (and on bootup). On enabling a latency tracer,
> we can allocate the buffer. When we enable another tracer (or nop) if
> the use_max_tr is not set, then we can remove the buffer.
> 
> Would you be able to do something like that?


How about this? 



>From d17bef4652a00c124940cb8c28ff7fcf584c9008 Mon Sep 17 00:00:00 2001
From: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Date: Thu, 1 Jul 2010 13:06:57 +0900
Subject: [PATCH 1/2] tracing: shrink max latency ringbuffer if unnecessary

Documentation/trace/ftrace.txt says

  buffer_size_kb:

        This sets or displays the number of kilobytes each CPU
        buffer can hold. The tracer buffers are the same size
        for each CPU. The displayed number is the size of the
        CPU buffer and not total size of all buffers. The
        trace buffers are allocated in pages (blocks of memory
        that the kernel uses for allocation, usually 4 KB in size).
        If the last page allocated has room for more bytes
        than requested, the rest of the page will be used,
        making the actual allocation bigger than requested.
        ( Note, the size may not be a multiple of the page size
          due to buffer management overhead. )

        This can only be updated when the current_tracer
        is set to "nop".

But it's incorrect. currently total memory consumption is
'buffer_size_kb x CPUs x 2'.

Why two times difference is there? because ftrace implicitly allocate
the buffer for max latency too.

That makes sad result when admin want to use large buffer. (If admin
want full logging and makes detail analysis). example, If admin
have 24 CPUs machine and write 200MB to buffer_size_kb, the system
consume ~10GB memory (200MB x 24 x 2). umm.. 5GB memory waste is
usually unacceptable.

Fortunatelly, almost all users don't use max latency feature.
The max latency buffer can be disabled easily.

This patch shrink buffer size of the max latency buffer if
unnecessary.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
---
 kernel/trace/trace.c              |   39 ++++++++++++++++++++++++++++++------
 kernel/trace/trace.h              |    1 +
 kernel/trace/trace_irqsoff.c      |    3 ++
 kernel/trace/trace_sched_wakeup.c |    2 +
 4 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 8683dec..9027f98 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -659,6 +659,10 @@ update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu)
 		return;
 
 	WARN_ON_ONCE(!irqs_disabled());
+	if (!current_trace->use_max_tr) {
+		WARN_ON_ONCE(1);
+		return;
+	}
 	arch_spin_lock(&ftrace_max_lock);
 
 	tr->buffer = max_tr.buffer;
@@ -685,6 +689,11 @@ update_max_tr_single(struct trace_array *tr, struct task_struct *tsk, int cpu)
 		return;
 
 	WARN_ON_ONCE(!irqs_disabled());
+	if (!current_trace->use_max_tr) {
+		WARN_ON_ONCE(1);
+		return;
+	}
+
 	arch_spin_lock(&ftrace_max_lock);
 
 	ftrace_disable_cpu();
@@ -2797,6 +2806,9 @@ static int tracing_resize_ring_buffer(unsigned long size)
 	if (ret < 0)
 		return ret;
 
+	if (!current_trace->use_max_tr)
+		goto out;
+
 	ret = ring_buffer_resize(max_tr.buffer, size);
 	if (ret < 0) {
 		int r;
@@ -2824,11 +2836,14 @@ static int tracing_resize_ring_buffer(unsigned long size)
 		return ret;
 	}
 
+	max_tr.entries = size;
+ out:
 	global_trace.entries = size;
 
 	return ret;
 }
 
+
 /**
  * tracing_update_buffers - used by tracing facility to expand ring buffers
  *
@@ -2889,12 +2904,26 @@ static int tracing_set_tracer(const char *buf)
 	trace_branch_disable();
 	if (current_trace && current_trace->reset)
 		current_trace->reset(tr);
-
+	if (current_trace && current_trace->use_max_tr) {
+		/*
+		 * We don't free the ring buffer. instead, resize it because
+		 * The max_tr ring buffer has some state (e.g. ring->clock) and
+		 * we want preserve it.
+		 */
+		ring_buffer_resize(max_tr.buffer, 1);
+		max_tr.entries = 1;
+	}
 	destroy_trace_option_files(topts);
 
 	current_trace = t;
 
 	topts = create_trace_option_files(current_trace);
+	if (current_trace->use_max_tr) {
+		ret = ring_buffer_resize(max_tr.buffer, global_trace.entries);
+		if (ret < 0)
+			goto out;
+		max_tr.entries = global_trace.entries;
+	}
 
 	if (t->init) {
 		ret = tracer_init(t, tr);
@@ -3476,7 +3505,6 @@ tracing_entries_write(struct file *filp, const char __user *ubuf,
 	}
 
 	tracing_start();
-	max_tr.entries = global_trace.entries;
 	mutex_unlock(&trace_types_lock);
 
 	return cnt;
@@ -4572,18 +4600,15 @@ __init static int tracer_alloc_buffers(void)
 	}
 	global_trace.entries = ring_buffer_size(global_trace.buffer);
 
 #ifdef CONFIG_TRACER_MAX_TRACE
-	max_tr.buffer = ring_buffer_alloc(ring_buf_size,
-					     TRACE_BUFFER_FLAGS);
+	max_tr.buffer = ring_buffer_alloc(1, TRACE_BUFFER_FLAGS);
 	if (!max_tr.buffer) {
 		printk(KERN_ERR "tracer: failed to allocate max ring buffer!\n");
 		WARN_ON(1);
 		ring_buffer_free(global_trace.buffer);
 		goto out_free_cpumask;
 	}
-	max_tr.entries = ring_buffer_size(max_tr.buffer);
-	WARN_ON(max_tr.entries != global_trace.entries);
+	max_tr.entries = 1;
 #endif
 
 	/* Allocate the first page for all buffers */
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 01ce088..7f67247 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -278,6 +278,7 @@ struct tracer {
 	struct tracer		*next;
 	int			print_max;
 	struct tracer_flags	*flags;
+	int			use_max_tr;
 };
 
 
diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
index 6fd486e..73a6b06 100644
--- a/kernel/trace/trace_irqsoff.c
+++ b/kernel/trace/trace_irqsoff.c
@@ -649,6 +649,7 @@ static struct tracer irqsoff_tracer __read_mostly =
 #endif
 	.open           = irqsoff_trace_open,
 	.close          = irqsoff_trace_close,
+	.use_max_tr	= 1,
 };
 # define register_irqsoff(trace) register_tracer(&trace)
 #else
@@ -681,6 +682,7 @@ static struct tracer preemptoff_tracer __read_mostly =
 #endif
 	.open		= irqsoff_trace_open,
 	.close		= irqsoff_trace_close,
+	.use_max_tr	= 1,
 };
 # define register_preemptoff(trace) register_tracer(&trace)
 #else
@@ -715,6 +717,7 @@ static struct tracer preemptirqsoff_tracer __read_mostly =
 #endif
 	.open		= irqsoff_trace_open,
 	.close		= irqsoff_trace_close,
+	.use_max_tr	= 1,
 };
 
 # define register_preemptirqsoff(trace) register_tracer(&trace)
diff --git a/kernel/trace/trace_sched_wakeup.c b/kernel/trace/trace_sched_wakeup.c
index c9fd5bd..4086eae 100644
--- a/kernel/trace/trace_sched_wakeup.c
+++ b/kernel/trace/trace_sched_wakeup.c
@@ -382,6 +382,7 @@ static struct tracer wakeup_tracer __read_mostly =
 #ifdef CONFIG_FTRACE_SELFTEST
 	.selftest    = trace_selftest_startup_wakeup,
 #endif
+	.use_max_tr	= 1,
 };
 
 static struct tracer wakeup_rt_tracer __read_mostly =
@@ -396,6 +397,7 @@ static struct tracer wakeup_rt_tracer __read_mostly =
 #ifdef CONFIG_FTRACE_SELFTEST
 	.selftest    = trace_selftest_startup_wakeup,
 #endif
+	.use_max_tr	= 1,
 };
 
 __init static int init_wakeup_tracer(void)
-- 
1.6.5.2

Re: [PATCH v2] tracing: shrink max latency ringbuffer if unnecessary

[Lai Jiangshan]

KOSAKI Motohiro wrote:
> 
> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> ---
>  kernel/trace/trace.c              |   39 ++++++++++++++++++++++++++++++------
>  kernel/trace/trace.h              |    1 +
>  kernel/trace/trace_irqsoff.c      |    3 ++
>  kernel/trace/trace_sched_wakeup.c |    2 +
>  4 files changed, 38 insertions(+), 7 deletions(-)
> 

Reviewed-by: Lai Jiangshan <laijs@cn.fujitsu.com


> -
> +	if (current_trace && current_trace->use_max_tr) {
> +		/*
> +		 * We don't free the ring buffer. instead, resize it because
> +		 * The max_tr ring buffer has some state (e.g. ring->clock) and
> +		 * we want preserve it.
> +		 */
> +		ring_buffer_resize(max_tr.buffer, 1);
> +		max_tr.entries = 1;
> +	}
>  	destroy_trace_option_files(topts);
>  
>  	current_trace = t;
>  
>  	topts = create_trace_option_files(current_trace);

I think we can skip the two resize when current_trace->use_max_tr==1 && t->use_max_tr==1

> +	if (current_trace->use_max_tr) {
> +		ret = ring_buffer_resize(max_tr.buffer, global_trace.entries);
> +		if (ret < 0)
> +			goto out;
> +		max_tr.entries = global_trace.entries;
> +	}
>  
>  	if (t->init) {
>  		ret = tracer_init(t, tr);

Does we need to shrink it when tracer_init() fails?
Although tracer_init() hardly fails, and there is no bad effect even we don't shrink it.

Re: [PATCH v2] tracing: shrink max latency ringbuffer if unnecessary

[KOSAKI Motohiro]

2010/7/1 Lai Jiangshan <laijs@cn.fujitsu.com>:
> KOSAKI Motohiro wrote:
>>
>> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
>> ---
>>  kernel/trace/trace.c              |   39 ++++++++++++++++++++++++++++++------
>>  kernel/trace/trace.h              |    1 +
>>  kernel/trace/trace_irqsoff.c      |    3 ++
>>  kernel/trace/trace_sched_wakeup.c |    2 +
>>  4 files changed, 38 insertions(+), 7 deletions(-)
>>
>
> Reviewed-by: Lai Jiangshan <laijs@cn.fujitsu.com
>
>
>> -
>> +     if (current_trace && current_trace->use_max_tr) {
>> +             /*
>> +              * We don't free the ring buffer. instead, resize it because
>> +              * The max_tr ring buffer has some state (e.g. ring->clock) and
>> +              * we want preserve it.
>> +              */
>> +             ring_buffer_resize(max_tr.buffer, 1);
>> +             max_tr.entries = 1;
>> +     }
>>       destroy_trace_option_files(topts);
>>
>>       current_trace = t;
>>
>>       topts = create_trace_option_files(current_trace);
>
> I think we can skip the two resize when current_trace->use_max_tr==1 && t->use_max_tr==1

Yup. but I don't think it's worthful because it's rarely operation.


>
>> +     if (current_trace->use_max_tr) {
>> +             ret = ring_buffer_resize(max_tr.buffer, global_trace.entries);
>> +             if (ret < 0)
>> +                     goto out;
>> +             max_tr.entries = global_trace.entries;
>> +     }
>>
>>       if (t->init) {
>>               ret = tracer_init(t, tr);
>
> Does we need to shrink it when tracer_init() fails?
> Although tracer_init() hardly fails, and there is no bad effect even we don't shrink it.

Nope. brief code of tracing_set_tracer() is here

========================================
        if (current_trace && current_trace->reset)
                current_trace->reset(tr);

        destroy_trace_option_files(topts);

        current_trace = t;

        topts = create_trace_option_files(current_trace);

        if (t->init) {
                ret = tracer_init(t, tr);
                if (ret)
                        goto out;
        }
========================================

That's mean, if t->init fail, we can't rollback old tracer. so your
suggested micro optimization
doesn't makes observable improvement, I think.

Thanks.

[tip:perf/core] tracing: Shrink max latency ringbuffer if unnecessary

[tip-bot for KOSAKI Motohiro]

Commit-ID:  ef710e100c1068d3dd5774d2b34c5485219e06ce
Gitweb:     http://git.kernel.org/tip/ef710e100c1068d3dd5774d2b34c5485219e06ce
Author:     KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
AuthorDate: Thu, 1 Jul 2010 14:34:35 +0900
Committer:  Steven Rostedt <rostedt@goodmis.org>
CommitDate: Wed, 21 Jul 2010 10:20:17 -0400

tracing: Shrink max latency ringbuffer if unnecessary

Documentation/trace/ftrace.txt says

  buffer_size_kb:

        This sets or displays the number of kilobytes each CPU
        buffer can hold. The tracer buffers are the same size
        for each CPU. The displayed number is the size of the
        CPU buffer and not total size of all buffers. The
        trace buffers are allocated in pages (blocks of memory
        that the kernel uses for allocation, usually 4 KB in size).
        If the last page allocated has room for more bytes
        than requested, the rest of the page will be used,
        making the actual allocation bigger than requested.
        ( Note, the size may not be a multiple of the page size
          due to buffer management overhead. )

        This can only be updated when the current_tracer
        is set to "nop".

But it's incorrect. currently total memory consumption is
'buffer_size_kb x CPUs x 2'.

Why two times difference is there? because ftrace implicitly allocate
the buffer for max latency too.

That makes sad result when admin want to use large buffer. (If admin
want full logging and makes detail analysis). example, If admin
have 24 CPUs machine and write 200MB to buffer_size_kb, the system
consume ~10GB memory (200MB x 24 x 2). umm.. 5GB memory waste is
usually unacceptable.

Fortunatelly, almost all users don't use max latency feature.
The max latency buffer can be disabled easily.

This patch shrink buffer size of the max latency buffer if
unnecessary.

Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
LKML-Reference: <20100701104554.DA2D.A69D9226@jp.fujitsu.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace.c              |   38 +++++++++++++++++++++++++++++++-----
 kernel/trace/trace.h              |    1 +
 kernel/trace/trace_irqsoff.c      |    3 ++
 kernel/trace/trace_sched_wakeup.c |    2 +
 4 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index af90429..f7488f4 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -660,6 +660,10 @@ update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu)
 		return;
 
 	WARN_ON_ONCE(!irqs_disabled());
+	if (!current_trace->use_max_tr) {
+		WARN_ON_ONCE(1);
+		return;
+	}
 	arch_spin_lock(&ftrace_max_lock);
 
 	tr->buffer = max_tr.buffer;
@@ -686,6 +690,11 @@ update_max_tr_single(struct trace_array *tr, struct task_struct *tsk, int cpu)
 		return;
 
 	WARN_ON_ONCE(!irqs_disabled());
+	if (!current_trace->use_max_tr) {
+		WARN_ON_ONCE(1);
+		return;
+	}
+
 	arch_spin_lock(&ftrace_max_lock);
 
 	ftrace_disable_cpu();
@@ -2801,6 +2810,9 @@ static int tracing_resize_ring_buffer(unsigned long size)
 	if (ret < 0)
 		return ret;
 
+	if (!current_trace->use_max_tr)
+		goto out;
+
 	ret = ring_buffer_resize(max_tr.buffer, size);
 	if (ret < 0) {
 		int r;
@@ -2828,11 +2840,14 @@ static int tracing_resize_ring_buffer(unsigned long size)
 		return ret;
 	}
 
+	max_tr.entries = size;
+ out:
 	global_trace.entries = size;
 
 	return ret;
 }
 
+
 /**
  * tracing_update_buffers - used by tracing facility to expand ring buffers
  *
@@ -2893,12 +2908,26 @@ static int tracing_set_tracer(const char *buf)
 	trace_branch_disable();
 	if (current_trace && current_trace->reset)
 		current_trace->reset(tr);
-
+	if (current_trace && current_trace->use_max_tr) {
+		/*
+		 * We don't free the ring buffer. instead, resize it because
+		 * The max_tr ring buffer has some state (e.g. ring->clock) and
+		 * we want preserve it.
+		 */
+		ring_buffer_resize(max_tr.buffer, 1);
+		max_tr.entries = 1;
+	}
 	destroy_trace_option_files(topts);
 
 	current_trace = t;
 
 	topts = create_trace_option_files(current_trace);
+	if (current_trace->use_max_tr) {
+		ret = ring_buffer_resize(max_tr.buffer, global_trace.entries);
+		if (ret < 0)
+			goto out;
+		max_tr.entries = global_trace.entries;
+	}
 
 	if (t->init) {
 		ret = tracer_init(t, tr);
@@ -3480,7 +3509,6 @@ tracing_entries_write(struct file *filp, const char __user *ubuf,
 	}
 
 	tracing_start();
-	max_tr.entries = global_trace.entries;
 	mutex_unlock(&trace_types_lock);
 
 	return cnt;
@@ -4578,16 +4606,14 @@ __init static int tracer_alloc_buffers(void)
 
 
 #ifdef CONFIG_TRACER_MAX_TRACE
-	max_tr.buffer = ring_buffer_alloc(ring_buf_size,
-					     TRACE_BUFFER_FLAGS);
+	max_tr.buffer = ring_buffer_alloc(1, TRACE_BUFFER_FLAGS);
 	if (!max_tr.buffer) {
 		printk(KERN_ERR "tracer: failed to allocate max ring buffer!\n");
 		WARN_ON(1);
 		ring_buffer_free(global_trace.buffer);
 		goto out_free_cpumask;
 	}
-	max_tr.entries = ring_buffer_size(max_tr.buffer);
-	WARN_ON(max_tr.entries != global_trace.entries);
+	max_tr.entries = 1;
 #endif
 
 	/* Allocate the first page for all buffers */
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 7778f06..cb629b3 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -276,6 +276,7 @@ struct tracer {
 	struct tracer		*next;
 	int			print_max;
 	struct tracer_flags	*flags;
+	int			use_max_tr;
 };
 
 
diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
index 6fd486e..73a6b06 100644
--- a/kernel/trace/trace_irqsoff.c
+++ b/kernel/trace/trace_irqsoff.c
@@ -649,6 +649,7 @@ static struct tracer irqsoff_tracer __read_mostly =
 #endif
 	.open           = irqsoff_trace_open,
 	.close          = irqsoff_trace_close,
+	.use_max_tr	= 1,
 };
 # define register_irqsoff(trace) register_tracer(&trace)
 #else
@@ -681,6 +682,7 @@ static struct tracer preemptoff_tracer __read_mostly =
 #endif
 	.open		= irqsoff_trace_open,
 	.close		= irqsoff_trace_close,
+	.use_max_tr	= 1,
 };
 # define register_preemptoff(trace) register_tracer(&trace)
 #else
@@ -715,6 +717,7 @@ static struct tracer preemptirqsoff_tracer __read_mostly =
 #endif
 	.open		= irqsoff_trace_open,
 	.close		= irqsoff_trace_close,
+	.use_max_tr	= 1,
 };
 
 # define register_preemptirqsoff(trace) register_tracer(&trace)
diff --git a/kernel/trace/trace_sched_wakeup.c b/kernel/trace/trace_sched_wakeup.c
index c9fd5bd..4086eae 100644
--- a/kernel/trace/trace_sched_wakeup.c
+++ b/kernel/trace/trace_sched_wakeup.c
@@ -382,6 +382,7 @@ static struct tracer wakeup_tracer __read_mostly =
 #ifdef CONFIG_FTRACE_SELFTEST
 	.selftest    = trace_selftest_startup_wakeup,
 #endif
+	.use_max_tr	= 1,
 };
 
 static struct tracer wakeup_rt_tracer __read_mostly =
@@ -396,6 +397,7 @@ static struct tracer wakeup_rt_tracer __read_mostly =
 #ifdef CONFIG_FTRACE_SELFTEST
 	.selftest    = trace_selftest_startup_wakeup,
 #endif
+	.use_max_tr	= 1,
 };
 
 __init static int init_wakeup_tracer(void)