Re: [PATCH] trace: Add a free on close control mechanism for buffer_size_kb
From: Vaibhav Nagarnaik
Date: Thu Apr 28 2011 - 19:14:54 EST
All
Can you please take a look at this patch and provide your thoughts?
Thanks
Vaibhav Nagarnaik
On Fri, Apr 22, 2011 at 3:46 PM, Vaibhav Nagarnaik
<vnagarnaik@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
> 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)
> +{
> + 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;
> +}
> +
>
> /**
> * tracing_update_buffers - used by tracing facility to expand ring buffers
> @@ -3399,11 +3435,37 @@ out_err:
> goto out;
> }
>
> +struct ftrace_entries_info {
> + struct trace_array *tr;
> + int free_buffer_on_close;
> +};
> +
> +static int
> +tracing_entries_open(struct inode *inode, struct file *filp)
> +{
> + struct ftrace_entries_info *info;
> +
> + if (tracing_disabled)
> + return -ENODEV;
> +
> + info = kzalloc(sizeof(*info), GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> +
> + info->tr = (struct trace_array *)inode->i_private;
> + info->free_buffer_on_close = 0;
> +
> + filp->private_data = info;
> +
> + return 0;
> +}
> +
> static ssize_t
> tracing_entries_read(struct file *filp, char __user *ubuf,
> size_t cnt, loff_t *ppos)
> {
> - struct trace_array *tr = filp->private_data;
> + struct ftrace_entries_info *info = filp->private_data;
> + struct trace_array *tr = info->tr;
> char buf[96];
> int r;
>
> @@ -3425,7 +3487,7 @@ tracing_entries_write(struct file *filp, const char __user *ubuf,
> {
> unsigned long val;
> char buf[64];
> - int ret, cpu;
> + int ret;
>
> if (cnt >= sizeof(buf))
> return -EINVAL;
> @@ -3443,46 +3505,50 @@ tracing_entries_write(struct file *filp, const char __user *ubuf,
> if (!val)
> return -EINVAL;
>
> - mutex_lock(&trace_types_lock);
> + /* value is in KB */
> + val <<= 10;
>
> - tracing_stop();
> + ret = tracing_buffer_resize_atomic(val);
> + if (ret < 0)
> + return ret;
>
> - /* 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);
> - }
> + *ppos += cnt;
>
> - /* value is in KB */
> - val <<= 10;
> + return cnt;
> +}
>
> - if (val != global_trace.entries) {
> - ret = tracing_resize_ring_buffer(val);
> - if (ret < 0) {
> - cnt = ret;
> - goto out;
> - }
> +static long
> +tracing_entries_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> +{
> + int ret = -ENOIOCTLCMD;
> + struct ftrace_entries_info *info = filp->private_data;
> +
> + switch (cmd) {
> + case TRACE_RINGBUF_FREE_ON_CLOSE: {
> + info->free_buffer_on_close = !!arg;
> + ret = 0;
> + break;
> + }
> }
>
> - *ppos += cnt;
> + return ret;
> +}
>
> - /* If check pages failed, return ENOMEM */
> - if (tracing_disabled)
> - cnt = -ENOMEM;
> - out:
> - 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);
> +static int
> +tracing_entries_release(struct inode *inode, struct file *filp)
> +{
> + struct ftrace_entries_info *info = filp->private_data;
> +
> + if (info->free_buffer_on_close) {
> + /* disable tracing */
> + tracing_off();
> + /* resize the ring buffer to 0 */
> + tracing_buffer_resize_atomic(0);
> }
>
> - tracing_start();
> - mutex_unlock(&trace_types_lock);
> + kfree(info);
>
> - return cnt;
> + return 0;
> }
>
> static int mark_printk(const char *fmt, ...)
> @@ -3624,9 +3690,12 @@ static const struct file_operations tracing_pipe_fops = {
> };
>
> static const struct file_operations tracing_entries_fops = {
> - .open = tracing_open_generic,
> + .open = tracing_entries_open,
> .read = tracing_entries_read,
> .write = tracing_entries_write,
> + .unlocked_ioctl = tracing_entries_ioctl,
> + .compat_ioctl = tracing_entries_ioctl,
> + .release = tracing_entries_release,
> .llseek = generic_file_llseek,
> };
>
> --
> 1.7.3.1
>
>
--
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/