Messages in this thread | | | Date | Tue, 02 Jul 2013 20:26:51 +0900 | From | Masami Hiramatsu <> | Subject | Re: [PATCH v4] tracing/uprobes: Support ftrace_event_file base multibuffer |
| |
A comment I just have is a cosmetic one.
(2013/06/29 16:51), zhangwei(Jovi) wrote: > @@ -605,33 +629,87 @@ typedef bool (*filter_func_t)(struct uprobe_consumer *self, > struct mm_struct *mm); > > static int > -probe_event_enable(struct trace_uprobe *tu, int flag, filter_func_t filter) > +probe_event_enable(struct trace_uprobe *tu, struct ftrace_event_file *file, > + filter_func_t filter) > { > + bool enabled = is_trace_uprobe_enabled(tu); > + struct event_file_link *link; > int ret = 0; > > - if (is_trace_uprobe_enabled(tu)) > - return -EINTR; > + if (file) { > + if (tu->flags & TP_FLAG_PROFILE) > + return -EINTR; > + > + link = kmalloc(sizeof(*link), GFP_KERNEL); > + if (!link) > + return -ENOMEM; > + > + link->file = file; > + list_add_tail_rcu(&link->list, &tu->files); > + > + tu->flags |= TP_FLAG_TRACE; > + } else { > + if (tu->flags & TP_FLAG_TRACE) > + return -EINTR; > + > + tu->flags |= TP_FLAG_PROFILE; > + } > > WARN_ON(!uprobe_filter_is_empty(&tu->filter)); > > - tu->flags |= flag; > - tu->consumer.filter = filter; > - ret = uprobe_register(tu->inode, tu->offset, &tu->consumer); > - if (ret) > - tu->flags &= ~flag; > + /* we cannot call uprobe_register twice for same tu */ > + if (!enabled) { > + tu->consumer.filter = filter; > + ret = uprobe_register(tu->inode, tu->offset, &tu->consumer); > + } > + > + if (ret) {
Here, since the ret is only changed by uprobe_register, this block would better be included in the above block. :)
> + if (file) { > + list_del_rcu(&link->list); > + kfree(link); > + tu->flags &= ~TP_FLAG_TRACE; > + } else > + tu->flags &= ~TP_FLAG_PROFILE; > + } > > return ret; > }
Thank you,
-- Masami HIRAMATSU IT Management Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com
| |