This is the mail archive of the systemtap@sourceware.org mailing list for the systemtap project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH]Kprobes: fix kprobes reentrancy


On Tue, Dec 12, 2006 at 06:49:56PM -0500, Chuck Ebbert wrote:
> > +static inline struct prev_kprobe *get_prev_kprobe(void)
> > +{
> > +     unsigned int i;
> > +     i = atomic_add_return(1, &(__get_cpu_var(prev_kprobe_index)));
> > +     BUG_ON(i > PREV_KPROBE_SIZE);
> 
> Can't you make it fail gracefully instead of going BUG() if this happens
> (i.e. skip the kprobe or something?)
Ideally you should never hit this condition. BUG_ON is there just to help
identify the culprit right then and there instead of causing memory corruption
and eventually crashing the system, which might make hard to debug.
> 
> > +     return (&__get_cpu_var(prev_kprobe_blk)[i-1]);
> > +}
> > +
> > +static inline struct prev_kprobe *restore_prev_kprobe(void)
> > +{
> > +     unsigned int i;
> > +     i = atomic_sub_return(1, &(__get_cpu_var(prev_kprobe_index)));
> 
> And this needs a BUG_ON() or Bad Things could happen on underflow.
The above BUG_ON() triggers if we ever cause reentrancy recursively for more
than allowed number of times. I don;t think we need it here.

> 
> > +     return (&__get_cpu_var(prev_kprobe_blk)[i]);
> > +}
> > +
> 
> -- 
> MBTI: IXTP


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]