lkml.org 
[lkml]   [2008]   [Apr]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC Patch 1/1] debugfs_printk and debugfs_dump interface

debugfs_print makes a great improvement to the trace interface. I
missing a couple of bugs the first time I reviewed it. Please see my
comments in-line.

Dave..


> +static inline struct trace_info *init_trace_interface(struct
> + debugfs_printk_data *dpk)
> +{
> + struct trace_info *ti;
> +
> + dpk->exists = trace_exists(dpk->parent_dir, dpk->dir, &ti);
> +
> + switch(dpk->exists) {
> +
> + case TRACE_PARENT_DIR_EXISTS:
> + case TRACE_PARENT_DIR_ABSENT:
> + if(!dpk->buf_size)
> + dpk->buf_size = DEFAULT_TRACE_BUF_SIZE;
> + if(!dpk->sub_buf_size)
> + dpk->sub_buf_size = DEFAULT_TRACE_SUB_BUF_NR;
> + if(!dpk->flags)
> + dpk->flags = TRACE_FLIGHT_CHANNEL;
> + ti = trace_setup(dpk->parent_dir, dpk->dir,
> + dpk->buf_size, dpk->sub_buf_size, dpk->flags);
> + printk(KERN_INFO "Trace interface %s setup through "
> + "debugfs_printk\n",
> + ti->dir->d_iname);
> + if (IS_ERR(ti)) {
> + printk(KERN_ERR "Trace interface could not be "
> + "initialised\n");
> + return PTR_ERR(ti);
> + }
> + /* Fall through */
> + case TRACE_DIR_EXISTS:
> + if (ti->state != TRACE_RUNNING) {

You should start the trace only if the current state is TRACE_SETUP (I
think). See my comment in debugfs_print()


if (ti->state == TRACE_SETUP)
> + trace_start(ti);
> + }
> + }
> + return ti;
> +}
> +
> +/*
> + * debugfs_printk - A function to write into the debugfs mount 'directly'
> + * using 'trace' infrastructure
> + * @dpk - Structure containing info such as parent_dir and directory
> + * @format - String containing format string specifiers
> + * @ap - List of arguments
> + */
> +int debugfs_printk(struct debugfs_printk_data *dpk, char *format, ...)
> +{
> + int ret;
> + struct trace_info *ti;
> + va_list(ap);
> + unsigned long flags;
> +
> + va_start(ap, format);
> +
> + ti = init_trace_interface(dpk);

init_trace_interface() alway sets trace->state to TRACE_RUNNING . The
results is that the user is prevented from stopping the trace. You can
see this in fork_new_trace.

$ cat debug/trace_new_example/do_fork/state
running
$ echo stop > debug/trace_new_example/do_fork/state
$ cat debug/trace_new_example/do_fork/state
running

> +
> + /* Now do the actual printing */
> + /* Take an RCU Lock over the trace_info state */
> + rcu_read_lock();
> + /* Take a spinlock for the global buffer used by relay */
> + if (dpk->flags & TRACE_GLOBAL_CHANNEL)
> + spin_lock_irqsave(&trace_lock, flags);
> + ret = trace_printf(ti, format, ap);
> + if (dpk->flags & TRACE_GLOBAL_CHANNEL)
> + spin_unlock_irqrestore(&trace_lock, flags);
> + rcu_read_unlock();
> +
> + va_end(ap);
> + return ret;
> +}
> +EXPORT_SYMBOL(debugfs_printk);
> +
> +/*
> + * debugfs_printk - A function to write into the debugfs mount 'directly'
> + * using 'trace' infrastructure
> + * @dpk - Structure containing info such as parent_dir and directory
> + * @output - Data that needs to be output
> + * @output_len - Length of the output data
> + */
> +int debugfs_dump(struct debugfs_printk_data *dpk, const void *output,
> + const int output_len)
> +{
> + struct trace_info *ti;
> + char *record;
> +
> + ti = init_trace_interface(dpk);

Same issue as debugfs_printf,

You should also check for trace_running as you did for trace_printf.


> +
> + /* Now do the actual printing */
> + rcu_read_lock();
> + record = relay_reserve(ti->rchan, output_len);
> + if (record)
> + memcpy(record, output, output_len);
> + else
> + return -ENOMEM;
> + rcu_read_unlock();
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(debugfs_dump);


\
 
 \ /
  Last update: 2008-04-24 23:27    [W:0.177 / U:0.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site