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

Masami Hiramatsu
2009-05-09T17:33:20Z

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,