Messages in this thread | | | Date | Mon, 17 Jun 2013 11:54:59 +0900 | From | Masami Hiramatsu <> | Subject | Re: [PATCH] tracing/uprobes: Support ftrace_event_file base multibuffer |
| |
(2013/06/15 2:25), Paul E. McKenney wrote: > On Fri, Jun 14, 2013 at 12:33:27PM -0400, Steven Rostedt wrote: >> On Fri, 2013-06-14 at 09:21 -0700, Paul E. McKenney wrote: >> >>>>>> @@ -548,15 +556,35 @@ static void uprobe_trace_print(struct trace_uprobe *tu, >>>>>> /* uprobe handler */ >>>>>> static int uprobe_trace_func(struct trace_uprobe *tu, struct pt_regs *regs) >>>>>> { >>>>>> - if (!is_ret_probe(tu)) >>>>>> - uprobe_trace_print(tu, 0, regs); >>>>>> + struct ftrace_event_file **file; >>>>>> + >>>>>> + if (is_ret_probe(tu)) >>>>>> + return 0; >>>>>> + >>>>>> + file = rcu_dereference_raw(tu->files); >>>> >>>> Why are you using rcu_dereference_raw() and not rcu_dereference(). The >>>> _raw() is a bit special, and unless you know what you are doing with RCU >>>> here, don't use it. >>>> >>>> As I see you using rcu_dereference_raw() all over this patch, along with >>>> mutexes, I believe that you are not using RCU correctly here. >>> >>> If irqs and preempt are disabled, I suggest using rcu_dereference_sched(). >>> That is what it is there for. ;-) >> >> I believe this just copied what kprobes did, where irqs and preemption >> is disabled. I don't believe that these probes have that same luxury. >> >> But that begs the question. Should we convert the rcu_dereference_raw() >> in the kprobe code to rcu_dereference_sched()? > > It makes a lot of sense to me, at least assuming no issues with the > interrupts being disabled, but the checks not spotting this. Here > is the check: > > preempt_count() != 0 || irqs_disabled() > > (With additional elaboration for if lockdep is enabled.)
OK, I see. So I'll convert all the rcu_dereference_raw() to rcu_dereference_sched() except kprobe handler, because in the kprobe handler above check always be true. :)
Thank you,
-- Masami HIRAMATSU IT Management Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com
| |