[Bug kprobes/2062] Return probes does not scale well on SMP box

["jkenisto at us dot ibm dot com" <sourceware-bugzilla at sourceware dot org>]

------- Additional Comments From jkenisto at us dot ibm dot com  2006-04-19 21:44 -------
(In reply to comment #7)
> > Correct.  I suppose we could enable the caller to promise that the probed
> > function won't be preempted, won't sleep, and won't recurse, in which case we
> > could allocate an array of per-cpu kretprobe_instances and avoid the locking
> > associated with the free list.  ...
> 
> Asking probed function not to preempt or sleep won;t work as we will be left 
> with no return probes on most of the kernel functions.

That's not what I meant.  By default, we would NOT assume a "simple" function 
(no preemption, no sleeping, not recursion)  By default, we wouldn't use per-cpu
kretprobe_instances.  But if the caller DOES specify that it's a simple function
(e.g., by setting maxactive to -2 or some such), maybe we could do the per-cpu
trick.

> 
> 
> > And there's still the problem of the array of kretprobe_instances possibly
> > having to stick around after unregister_kretprobe() returns and the kretprobe
> > disappears.  Can we manage that without a lock?
> 
> We can not avoid the lock if return probe instance (ri) is managed either 
> globally or on per_cpu queue since may tasks will be running in parallel.

Well, I wasn't thinking of a per-cpu queue, but rather a NR_CPUS-element array
of kretprobe_instances for each kretprobe that probes a "simple" function.  But
again, if/when we implement a new  approach, we need to pay special attention to
unregister_kretprobe(); that's a likely source of trouble.

...
> 
> > Well, if we could add an hlist_head to struct task_struct (How big an "if" is
> > that?), we could throw out kretprobe_inst_table[] and the locking around 
> that. 
> It is not a big deal, but some people might object that kprobes is touching 
> core kernel data structure like task struct. Not sure about the penalty of 
> this extra space in task struct. We need to submit a patch and see what 
> community has to say.

First verifying, of course, that that would indeed help.  (I think it would, but
we gotta prototype and test it to be sure.)

> 
> > But do you also envision a per-task pool of kretprobe_instances?  If so, how
> > big?  If not, aren't we back to locking some sort of free list, at least for 
> the
> > sleeping and/or recursive functions?
> 
> Having to allocate per-task pool would be very expensive, as every task that 
> gets created will have to allocat the memory and release it when the task is 
> dead no matter the retprobe gets fired or not.

I wasn't suggesting a per-task pool, but thought that maybe you were.

...

> -Anil

Jim



-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=2062

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.