Messages in this thread | | | Date | Fri, 21 Jun 2013 16:17:20 +0800 | From | "zhangwei(Jovi)" <> | Subject | Re: [PATCH] tracing/uprobes: Support ftrace_event_file base multibuffer |
| |
On 2013/6/21 0:43, Oleg Nesterov wrote: > Hi, > > Now that we finished the discussion of the similar code in kprobes, > let me summarize. > > On 06/14, Oleg Nesterov wrote: >> >> On 06/14, zhangwei(Jovi) wrote: >>> >>> + rcu_assign_pointer(tu->files, new); >>> + tu->flags |= TP_FLAG_TRACE; >>> + >>> + if (old) { >>> + /* Make sure the probe is done with old files */ >>> + synchronize_sched(); >>> + kfree(old); >>> + } >>> + } else >>> + tu->flags |= TP_FLAG_PROFILE; >> >> So it can set both TP_FLAG_TRACE and TP_FLAG_PROFILE, yes? >> >> If yes, this is not right. Until we change the pre-filtering at least. >> Currently TP_FLAG_TRACE/TP_FLAG_PROFILE are mutually exclusive. > > Yes. So please update this patch so that TP_FLAG_PROFILE can't be > set if TP_FLAG_TRACE is set and vice versa. > > Once again, this limitation is artificial. But it was always here, > and we need more changes to remove it. I'll try to do this later. > (but if you want to play with this code - welcome ;) > > Don't add the mutex, and do not use array-of-pointers (I hope you > noticed the recent discussion). > > Locking. Oh, OK. Let it be rcu for now (but please do not forget > that you need rcu_read_lock, uprobe handlers run in the sleepable > context). This is sub-optimal, but this is just another indication > that uprobes API is not perfect, we can't use uprobe->register_sem. > > Also. When I was reading trace_kprobes.c I notice the new (and imho > useful) feature, soft disable/enable. Perhaps you can make a 2nd > patch which adds the FTRACE_EVENT_FL_SOFT_DISABLED_BIT check? > > Oleg. > Thanks Oleg, I will send v2 patch soon.
.jovi
| |