On Tue, 2013-07-16 at 20:57 +0200, Oleg Nesterov wrote: > ftrace_event_id_fops and event_id_read() is overcomplicated. > > 1. Change event_create_dir() to pass "data = call->event.type" > to debugfs_create_file(). > > This means that ftrace_event_id_fops doesn't need .open() > and event_id_read() can simply print (int)i_private > > 2. event_id_read() has no reason to kmalloc "struct trace_seq" > (more than PAGE_SIZE!), it can use a small buffer.
Make #2 a separate patch. You can add it before this one. -- Steve > > Note: "if (*ppos) return 0" looks strange and unneeded, > simple_read_from_buffer(ppos) should handle this case corrrectly. > > Signed-off-by: Oleg Nesterov <[email protected]> > --- > kernel/trace/trace_events.c | 26 +++++++++----------------- > 1 files changed, 9 insertions(+), 17 deletions(-) > > diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c > index cbd1a57..cefbe85 100644 > --- a/kernel/trace/trace_events.c > +++ b/kernel/trace/trace_events.c > @@ -962,24 +962,17 @@ static int trace_format_open(struct inode *inode, > struct file *file) > static ssize_t > event_id_read(struct file *filp, char __user *ubuf, size_t cnt, loff_t *ppos) > { > - struct ftrace_event_call *call = filp->private_data; > - struct trace_seq *s; > - int r; > + int id = (long)ACCESS_ONCE(file_inode(filp)->i_private); > + char buf[32]; > + int len; > > + if (!id) > + return -ENODEV; > if (*ppos) > return 0; > > - s = kmalloc(sizeof(*s), GFP_KERNEL); > - if (!s) > - return -ENOMEM; > - > - trace_seq_init(s); > - trace_seq_printf(s, "%d\n", call->event.type); > - > - r = simple_read_from_buffer(ubuf, cnt, ppos, > - s->buffer, s->len); > - kfree(s); > - return r; > + len = sprintf(buf, "%d\n", id); > + return simple_read_from_buffer(ubuf, cnt, ppos, buf, len); > } > > static ssize_t > @@ -1264,7 +1257,6 @@ static const struct file_operations > ftrace_event_format_fops = { > }; > > static const struct file_operations ftrace_event_id_fops = { > - .open = tracing_open_generic, > .read = event_id_read, > .llseek = default_llseek, > }; > @@ -1496,8 +1488,8 @@ event_create_dir(struct dentry *parent, > > #ifdef CONFIG_PERF_EVENTS > if (call->event.type && call->class->reg) > - trace_create_file("id", 0444, file->dir, call, > - id); > + trace_create_file("id", 0444, file->dir, > + (void*)(long)call->event.type, id); > #endif > > /* -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
