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/