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

[Prasanna S Panchamukhi <prasanna at in dot ibm dot com>]

Hi,

> >^^^^^^^^^^^^^^^^^^^^^^^^^^^^multiple handler will take care of all these 
> >right?
> >			 see my code below.
> >+	if (rp->num_ri_running != 0) {
> >+		int i;
> >+		struct kretprobe_instance *ri;
> >+		/* Make a copy of kretprobe so we can relinquish the 
> >+		 * user's original.
> >+		 */
> >+		memcpy(rp_tmp, rp, sizeof(struct kretprobe));
> >+		rp_tmp->unregistering = 1;
> >+		for (i = 0 ; i < rp->maxactive; i++) {
> >+			ri = rp->instances + i;
> >+			ri->rp = rp_tmp;
> >	^^^^^^^^^^^^^^^^^^^^^^instead you use ri->ret_add to find you 
> >	if it is on afree list or on a kretprobe_inst_table[], by checking 
> >	if NULL..
> >+		}
> >+}	
> > 
> >
> 
> We need the piece of code above in the case of (ie. a function has been 
> recursive invoked and the return address has been replaced with the 
> trampoline on stack)
> 

Can you explain me what problem you are trying to solve here.
Below is detail explaination, using which you can implement retprobe feature
and use unregister_kprobe() cleanly. Can you post your patch after the changes.

Please let me know if you need any help.

Thanks
Prasanna

You can use two heads and two nodes..along with the kretprobe_table[]
and provide cleaner interface.
free_instances - contain instances which are free.
used_instances - contain instances which are used and also
another link will be present in th kretprobe_table[]
Only one link of an instance structure can  exist on 
free_instances list, but two links will exist for an instance in use(on 
used_instaces and kretprobe_table).

struct kretprobe {
	..................
        struct hlist_head free_instances;
        struct hlist_head used_instances;
};

struct kretprobe_instance {
        struct hlist_node hinst; /*either on free list or used list*/
        struct hlist_node hlist; /*only on kretporbe_table */
	.......................
};

allocate individual kretprobe_instances and add them to the free list.
register_kretprobe() {
........................
.....................
	
 for (i = 0; i < rp->maxactive; i++) {
       inst = kmalloc(sizeof(struct kretprobe_instance), GFP_KERNEL);
            if (inst == NULL) {
        	  while((ri = get_free_rp_inst(rp)) != NULL) {
               	  hlist_del(&ri->hinst);
               	  kfree(ri);
        	 }
                return -ENOMEM;
            }
                INIT_HLIST_NODE(&inst->hinst);
                hlist_add_head(&inst->hinst, &rp->free_instances);
        }
......................
	register_kprobe(&rp->kp);
	remove instances from free list and free them if
	kprobes does not succeed.
	
................
}

During unregisteration you can initially unregister the entrypoint probe.
Later free the instances present on the free_instances, also delete
the intances from the used list and mark them for freeing by 
marking ri->rp = NULL;
These instances under use can be freed either in flush_task() or in 
tarmpoline_posthandler().

No need for a sparate copy of kretprobe after unregistering.
there is no need to call the handlers, just we need to free the
instance structure and replace back the original return address on the
stack.
void unregister_kretprobe(struct kretprobe *rp)
{
	...................	
        unregister_kprobe(&rp->kp);

	...................
	spin_lock()
        while((ri = get_free_rp_inst(rp)) != NULL) {
                hlist_del(&ri->hinst);
                kfree(ri);
        }
        while((ri = get_used_rp_inst(rp)) != NULL) {
                        ri->rp = NULL; 
	/*use this to find out if we are unregistering and free the instance*/
                        hlist_del(&ri->hinst);
        }
	spin_unlock()
}


-- 

Prasanna S Panchamukhi
Linux Technology Center
India Software Labs, IBM Bangalore
Ph: 91-80-25044636
<prasanna@in.ibm.com>