Messages in this thread | | | Date | Tue, 25 Jun 2013 22:24:05 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 1/2 v3] tracing/uprobes: Support ftrace_event_file base multibuffer |
| |
Sorry again, didn't have time to review, will try tomorrow.
Looks good but a couple of minor nits, and perhaps we should fix the bugs with unregister_trace_uprobe first... in kprobes too _if_ I am right. I'll return tomorrow.
On 06/25, zhangwei(Jovi) wrote: > > -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) > { > + int enabled = 0; > int ret = 0; > > + /* we cannot call uprobe_register twice for same tu */ > if (is_trace_uprobe_enabled(tu)) > - return -EINTR; > + enabled = 1;
Cosmetic again, "int enabled = 0" and then "if (is_trace_uprobe_enabled)" looks a bit strange,
enabled = is_trace_uprobe_enabled();
looks a bit more clean.
> + if (file) { > + struct event_file_link *link; > + > + if (tu->flags & TP_FLAG_PROFILE) > + return -EINTR; > + > + link = kmalloc(sizeof(*link), GFP_KERNEL); > + if (!link) > + return -ENOMEM; > + > + link->file = file; > + list_add_rcu(&link->list, &tu->files);
I agree with Masami, list_add_rcu_tail() looks better even if this doesn't really matter.
Oleg.
| |