Re: [Fwd: Re: [PATCH] Return probe]

[Hien Nguyen <hien at us dot ibm dot com>]

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
>
>