This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: [PATCH]Kprobes: fix kprobes reentrancy
- From: "Keshavamurthy, Anil S" <anil dot s dot keshavamurthy at intel dot com>
- To: Chuck Ebbert <76306 dot 1226 at compuserve dot com>
- Cc: "Keshavamurthy, Anil S" <anil dot s dot keshavamurthy at intel dot com>, Systemtap <systemtap at sources dot redhat dot com>, Prasanna S Panchamukhi <prasanna at in dot ibm dot com>, Jim Keniston <jkenisto at us dot ibm dot com>, Ananth N Mavinakayanahalli <ananth at in dot ibm dot com>, akpm at osdl dot org
- Date: Tue, 12 Dec 2006 16:57:05 -0800
- Subject: Re: [PATCH]Kprobes: fix kprobes reentrancy
- References: <200612121853_MC3-1-D4D2-1919@compuserve.com>
- Reply-to: "Keshavamurthy, Anil S" <anil dot s dot keshavamurthy at intel dot com>
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