Re: [Patch 11/11] ftrace plugin for kernel symbol tracing using HWBreakpoint interfaces - v2

[K.Prasad]

On Sat, Mar 21, 2009 at 12:39:08PM -0400, Steven Rostedt wrote:
> 
> On Sat, 21 Mar 2009, K.Prasad wrote:
> > > > 
> > 
> > > > +
> > > > +void ksym_collect_stats(unsigned long hbkpt_hit_addr)
> > > > +{
> > > > +	struct hlist_node *node;
> > > > +	struct trace_ksym *entry;
> > > > +
> > > > +	spin_lock(&ksym_stat_lock);
> > > 
> > > 
> > > I see that can be called from ksym_hbkpt_handler which in turn
> > > can be called from interrupt context, right?
> > > You can issue a deadlock if you don't disable interrupts here.
> > > 
> > > Thanks,
> > > Frederic.
> > > 
> > 
> > ksym_collect_stats<--ksym_hbkpt_handler<--hw_breakpoint_handler<--do_debug
> > invocation happens with interrupts enabled (IF bit is set). I do find
> > that a few plugins in kernel/trace enclose the
> > trace_buffer_lock_reserve()--trace_buffer_unlock_commit() invocation
> > within interrupt-disabled code. Is that a requirement there?
> > 
> > The potential deadlock scenario you foresee isn't obvious to me. Can you
> > explain?
> 
> Can that lock ever be taken in an interrupt? If not, document that (and 
> perhaps add a WARN_ON(in_interrupt()); ). Otherwise you have a possible:
> 
> 	spin_lock(&ksym_stat_lock);
> 
> 		===> take interrupt ...
> 
> 			(from interrupt)
> 			spin_lock(&ksym_stat_lock); <== deadlock.
> 
> 
> -- Steve
>

Given that the function pointed by the trigger() routine is invoked with
breakpoints disabled on that CPU, I don't think we'd enter into a loop
a cyclic dependancy as above.

On the other hand, my observation w.r.t. IF bit being set was misplaced
in the sense that it corresponded to the saved stack and not when inside
the breakpoint handler in which case interrupts were disabled.

So we are safe in either ways.

Thanks,
K.Prasad
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/