Re: [PATCH -tip v5 4/7] tracing: add kprobe-based event tracer

[Masami Hiramatsu <mhiramat@redhat.com>]

Frédéric Weisbecker wrote:
> Hi,
> 
> 2009/5/9 Masami Hiramatsu <mhiramat@redhat.com>:
[...]
>> +
>> +/* event recording functions */
>> +static void kprobe_trace_record(unsigned long ip, struct trace_probe *tp,
>> +                               struct pt_regs *regs)
>> +{
>> +       __trace_bprintk(ip, "%s%s%+ld\n",
>> +                       probe_is_return(tp) ? "<-" : "@",
>> +                       probe_symbol(tp), probe_offset(tp));
>> +}
> 
> 
> 
> What happens here if you have:
> 
> kprobe_trace_record() {
>       probe_symbol() {
>             ....                         probes_open() {
>                                               cleanup_all_probes() {
>                                                          free_trace_probe();
>      return tp->symbol ? ....; //crack!
>
> I wonder if you shouldn't use a per_cpu list of probes,
> spinlocked/irqsaved  accessed
> and also a kind of prevention against nmi.

Sure, cleanup_all_probes() invokes unregister_kprobe() via
unregister_trace_probe(), which waits running probe-handlers by
using synchronize_sched()(because kprobes disables preemption
around its handlers), before free_trace_probe().

So you don't need any locks there :-)

Thank you,


-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com