This is the mail archive of the
systemtap@sources.redhat.com
mailing list for the systemtap project.
Re: [Fwd: Re: [PATCH] Return probe]
- From: Richard J Moore <richardj_moore at uk dot ibm dot com>
- To: Hien Nguyen <hien at us dot ibm dot com>
- Cc: jkenisto at us dot ibm dot com, prasanna at in dot ibm dot com, SystemTAP <systemtap at sources dot redhat dot com>, Vara Prasad <varap at us dot ibm dot com>
- Date: Tue, 19 Apr 2005 17:29:03 +0100
- Subject: Re: [Fwd: Re: [PATCH] Return probe]
- Sensitivity:
Do you take into account the problems I documented in my technical
disclosure on implementing return points? These are:
1) What happens if the return address is modified by the called program
(legitimately or otherwise)?
2) What happens if a multi-level function return takes place? This is quite
possible with optimization and some finite-state-machine implementations.
3) What happens in the function call is register based (PPC, zSeries etc,
optimized IA32 etc...)
- -
Richard J Moore
IBM Advanced Linux Response Team - Linux Technology Centre
MOBEX: 264807; Mobile (+44) (0)7739-875237
Office: (+44) (0)1962-817072
Hien Nguyen
<hien@us.ibm.co
m> To
Sent by: prasanna@in.ibm.com
systemtap-owner cc
@sources.redhat jkenisto@us.ibm.com, Vara Prasad
.com <varap@us.ibm.com>, SystemTAP
<systemtap@sources.redhat.com>
bcc
19/04/2005
17:11 Subject
Re: [Fwd: Re: [PATCH] Return
probe]
See my reply below for unregister_kretprobe.
Prasanna S Panchamukhi wrote:
>Hi Jim, Hien,
>
>Please see my comments below.
>
>
>
>>+/*
>>+ * This function is called from do_exit or do_execv when task tk's stack
is
>>+ * about to be recycled. Recycle any function-return probe instances
>>+ * associated with this task. These represent probed functions that have
>>+ * been called but may never return.
>>+ */
>>+void kprobe_flush_task(struct task_struct *tk)
>>+{
>>+ unsigned long flags = 0;
>>+ struct kretprobe_instance *ri;
>>+ struct task_struct *tsk;
>>+ struct hlist_head *head;
>>+ struct hlist_node *node;
>>+
>>+ if (!arch_supports_kretprobes) {
>>+ return;
>>+ }
>>+ spin_lock_irqsave(&kprobe_lock, flags);
>>+ head = &kretprobe_inst_table[hash_ptr(tk, RPROBE_HASH_BITS)];
>>+ hlist_for_each_entry(ri, node, head, hlist) {
>>+ tsk = arch_get_kprobe_task(ri->stack_addr);
>>+ if (tsk == tk) {
>>+ /* Put the original return address
back into stack */
>>+ *((unsigned long *)(ri->stack_addr)) =
(unsigned long) ri->ret_addr;
>>+ hlist_del_rcu(&ri->hlist);
>>+ recycle_kretprobe_instance(ri);
>>+ }
>>+ }
>>+ spin_unlock_irqrestore(&kprobe_lock, flags);
>>+}
>>+
>>
>>
>
>The current implementation modifies the return address on the stack, hence
the
>above routine called for every do_exit, do_execv. How much performance
impact
>will this cause?
>
>
>
>>+
>>+ rp_tmp = kmalloc(sizeof(struct kretprobe), GFP_KERNEL);
>>+ BUG_ON(rp_tmp == NULL);
>>+
>>+ spin_lock_irqsave(&kprobe_lock, flags);
>>+ old_p = get_kprobe(rp->kp.addr);
>>+ if (old_p && (old_p->pre_handler == aggr_pre_handler)) {
>>+ list_del(&rp->kp.list);
>>+ if (list_empty(&old_p->list)) {
>>+ remove_kprobe(old_p, flags);
>>+ kfree(old_p);
>>+ }
>>+ } else if (old_p == &rp->kp) {
>>+ remove_kprobe(&rp->kp, flags);
>>+ }
>>
>>
>
>The patch by Ananth provides multiple probes feature at a given address
and
>handles all the above cases. Can you pls check if the above checks can be
>removed and use multiple probes interface.
>
>
>
I need to get the hold of the lock before unregistering, I could not the
current unregister_kprobe / unregistering_kprobe_single /
unregister_aggr_kprobe as is without any modification, hence this code
is here.
>Thanks
>Prasanna
>
>
>