[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1304033561.18763.212.camel@gandalf.stny.rr.com>
Date: Thu, 28 Apr 2011 19:32:41 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Vaibhav Nagarnaik <vnagarnaik@...gle.com>
Cc: Frederic Weisbecker <fweisbec@...il.com>,
Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
Michael Rubin <mrubin@...gle.com>,
David Sharp <dhsharp@...gle.com>
Subject: Re: [PATCH] trace: Add a free on close control mechanism for
buffer_size_kb
Sorry, my mind has been elsewhere and I forgot about your patches.
On Fri, 2011-04-22 at 15:46 -0700, Vaibhav Nagarnaik wrote:
> The proc file entry buffer_size_kb is used to set the size of tracing
> buffer. The memory to expand the buffer size is kernel memory. Consider
> a use case where tracing is handled by a user space utility, which acts
> as a gate keeper for tracing requests. In an OOM condition, tracing is
> considered a low priority task and if the utility gets killed the ring
> buffer memory cannot be released back to the kernel.
>
> This patch adds an IOCTL on the buffer_size_kb file to set a boolean.
> When this boolean is enabled, closing buffer_size_kb file will cause
> tracing to stop and free up the ring buffer memory.
>
> The user space process can then open the buffer_size_kb file to set the
> new buffer size for tracing, enable the boolean through IOCTL and keep
> the file open. Under OOM condition, if the process gets killed, the
> kernel closes the file descriptor for buffer_size_kb. The release
> handler stops the tracing and releases the kernel memory automatically.
>
> Signed-off-by: Vaibhav Nagarnaik <vnagarnaik@...gle.com>
> ---
> include/linux/ftrace.h | 3 +
> kernel/trace/trace.c | 135 ++++++++++++++++++++++++++++++++++++------------
> 2 files changed, 105 insertions(+), 33 deletions(-)
>
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index ca29e03..30c8a23 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -114,6 +114,9 @@ struct ftrace_func_command {
> char *params, int enable);
> };
>
> +/* enable/disable auto free ring buffer on file close */
> +#define TRACE_RINGBUF_FREE_ON_CLOSE _IOW('t', 0x01, int)
> +
> #ifdef CONFIG_DYNAMIC_FTRACE
>
> int ftrace_arch_code_modify_prepare(void);
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index d38c16a..c676f17 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -2817,6 +2817,42 @@ static int tracing_resize_ring_buffer(unsigned long size)
> return ret;
> }
>
> +static ssize_t tracing_buffer_resize_atomic(unsigned long size)
Why call it "atomic"? Because you do the mutex? Anyway I hate the name.
It sounds like it can do it without preemption, which it clearly can't
do.
The rest looks fine to me.
-- Steve
> +{
> + int cpu, ret = 0;
> +
> + mutex_lock(&trace_types_lock);
> +
> + tracing_stop();
> +
> + /* disable all cpu buffers */
> + for_each_tracing_cpu(cpu) {
> + if (global_trace.data[cpu])
> + atomic_inc(&global_trace.data[cpu]->disabled);
> + if (max_tr.data[cpu])
> + atomic_inc(&max_tr.data[cpu]->disabled);
> + }
> +
> + if (size != global_trace.entries)
> + ret = tracing_resize_ring_buffer(size);
> +
> + /* If check pages failed, return ENOMEM */
> + if (tracing_disabled)
> + ret = -ENOMEM;
> +
> + for_each_tracing_cpu(cpu) {
> + if (global_trace.data[cpu])
> + atomic_dec(&global_trace.data[cpu]->disabled);
> + if (max_tr.data[cpu])
> + atomic_dec(&max_tr.data[cpu]->disabled);
> + }
> +
> + tracing_start();
> + mutex_unlock(&trace_types_lock);
> +
> + return ret;
> +}
> +
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists