On Sat, May 09, 2009 at 01:33:53PM -0400, Masami Hiramatsu wrote: > Frédéric Weisbecker wrote: > > Hi, > > > > 2009/5/9 Masami Hiramatsu <mhiramat@xxxxxxxxxx>: > [...] > >> + > >> +/* 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, > > Aah, ok :) So this patch looks sane. Thanks. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html