[Fwd: Re: [PATCH] Return probe]

[Hien Nguyen]

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