Hi Hien,
This patch looks good to me, but I have some comments on this patch.
>This patch adds function-return probes (AKA exit probes) to kprobes.
> When establishing a probepoint at the entry to a function, you can also
>establish a handler to be run when the function returns.
>The subsequent post give example of how function-return probes can be used.
>Two new registration interfaces are added to kprobes:
>int register_kretprobe(struct kprobe *kp, struct rprobe *rp);
>Registers a probepoint at the entry to the function whose address is
>kp->addr. Each time that function returns, rp->handler will be run.
>int register_jretprobe(struct jprobe *jp, struct rprobe *rp);
>Like register_kretprobe, except a jprobe is established for the probed
>function.
Why two interfaces for the same feature?
You can provide a simple interface like
register_exitprobe(struct rprobe *rp) {
....................
}
or
int register_returnprobe(struct rprobe *rp) {
....................
}
whichever you feel is a good name.
independent of kprobe and jprobe.
This routine should take care to register entry handler internally if not
present. This routine can check if there are already entry point kprobe/jprobe
and use some flags internally to say if the entry jprobe/kprobe already exists.
>To unregister, you still use unregister_kprobe or unregister_jprobe. To
>probe only a function's returns, call register_kretprobe() and specify
>NULL handlers for the kprobe.
make unregister exitprobes independent of kprobe/jprobe.
To unregister provide this interface
unregister_exitprobe(struct rpobe *rp) {
....................
}
This routine should check if entry point kprobe/jprobe belows to user/
registered by exit probe. Remove the entry probe if no user has registered
entry point kprobe/jprobe. If user has already registered entry point probes,
just leave the entry point probes and remove only the exit point probes.
Please let me know if you need more information.
Thanks
Prasanna
-
Prasanna S Panchamukhi
Linux Technology Center
India Software Labs, IBM Bangalore
Ph: 91-80-25044636
<[email protected]>
On Mon, 2005-04-04 at 01:15, Prasanna S Panchamukhi wrote:
> Hi Hien,
>
> This patch looks good to me, but I have some comments on this patch.
>
> >int register_kretprobe(struct kprobe *kp, struct rprobe *rp);
...
> >int register_jretprobe(struct jprobe *jp, struct rprobe *rp);
...
>
> Why two interfaces for the same feature?
> You can provide a simple interface like
...
> int register_returnprobe(struct rprobe *rp) {
...
> independent of kprobe and jprobe.
> This routine should take care to register entry handler internally if not
> present. This routine can check if there are already entry point kprobe/jprobe
> and use some flags internally to say if the entry jprobe/kprobe already exists.
>
...
>
> make unregister exitprobes independent of kprobe/jprobe.
>
...
>
> Please let me know if you need more information.
>
> Thanks
> Prasanna
We thought about that. It is a nicer interface. But I'm concerned that
if the user has to do
register_kprobe(&foo_entry_probe);
register_retprobe(&foo_return_probe);
then he/she has to be prepared to handle calls to foo that happen
between register_kprobe and register_retprobe -- i.e., calls where the
entry probe fires but the return probe doesn't. Similarly on
unregistration.
Here are a couple of things we could do to support registration and
unregistration of retprobes that can be either dependent on or
independent of the corresponding j/kprobes, as the user wants:
1. When you call register_j/kprobe(), if kprobe->rp is non-null, it is
assumed to point to a retprobe that will be registered and unregistered
along with the kprobe. (But this may make trouble for existing kprobes
applications that didn't need to initialize the (nonexistent) rp
pointer. Probably not a huge deal.)
OR
2. Create the ability to (a) register kprobes, jprobes, and/or retprobes
in a disabled state; and (b) enable a group of probes in an atomic
operation. Then you could register the entry and return probes
independently, but enable them together. We may need to do something
like that for SystemTap anyway.
Jim Keniston
IBM Linux Technology Center
On Tue, 2005-04-05 at 10:19, Jim Keniston wrote:
> On Mon, 2005-04-04 at 01:15, Prasanna S Panchamukhi wrote:
> ...
> > int register_returnprobe(struct rprobe *rp) {
> ...
>
> > independent of kprobe and jprobe.
> ...
> >
> > make unregister exitprobes independent of kprobe/jprobe.
> >
> ...
> >
> > Please let me know if you need more information.
> >
> > Thanks
> > Prasanna
>
> We thought about that. It is a nicer interface. But I'm concerned that
> if the user has to do
> register_kprobe(&foo_entry_probe);
> register_retprobe(&foo_return_probe);
> then he/she has to be prepared to handle calls to foo that happen
> between register_kprobe and register_retprobe -- i.e., calls where the
> entry probe fires but the return probe doesn't. Similarly on
> unregistration.
>
> Here are a couple of things we could do to support registration and
> unregistration of retprobes that can be either dependent on or
> independent of the corresponding j/kprobes, as the user wants:
>
> 1. When you call register_j/kprobe(), if kprobe->rp is non-null, it is
> assumed to point to a retprobe that will be registered and unregistered
> along with the kprobe. (But this may make trouble for existing kprobes
> applications that didn't need to initialize the (nonexistent) rp
> pointer. Probably not a huge deal.)
>
> OR
>
> 2. Create the ability to (a) register kprobes, jprobes, and/or retprobes
> in a disabled state; and (b) enable a group of probes in an atomic
> operation. Then you could register the entry and return probes
> independently, but enable them together. We may need to do something
> like that for SystemTap anyway.
>
> Jim Keniston
> IBM Linux Technology Center
I suppose if pairing of entry and return probes is important for a user,
he/she can always do the following:
static int ready; // 1 = everybody registered
// 2 = everybody knows we're registered
...
ready = 0;
... register_kprobe(&kp)...
... register_retprobe(&rp) ...
/* instant XXX -- see below*/
ready = 1;
and in kp.pre_handler do
if (!ready) {
// return probe not registered yet
return 0;
}
ready = 2;
<body of handler>
and in rp.handler do
if (ready != 2) {
// Probed function entered during instant XXX,
// so kp.pre_handler didn't act on it.
return 0;
}
<body of handler>
Keeping a whole group of kprobes, jprobes, and retprobes in the starting
gate pending a "ready" signal (e.g., for SystemTap) could probably be
handled similarly.
Unregistration shouldn't be an issue. At any time you can have N active
instances of the probed function, and have therefore recorded E entries
and E-N returns. Hien's code handles all that on retprobe
deregistration, but the user's instrumentation should never count on #
probed entries == # probed returns.
Jim
> > > int register_returnprobe(struct rprobe *rp) {
> > ...
> >
> > > independent of kprobe and jprobe.
> > ...
> > >
> > > make unregister exitprobes independent of kprobe/jprobe.
> > >
> > ...
> >
> > 1. When you call register_j/kprobe(), if kprobe->rp is non-null, it is
> > assumed to point to a retprobe that will be registered and unregistered
> > along with the kprobe. (But this may make trouble for existing kprobes
> > applications that didn't need to initialize the (nonexistent) rp
> > pointer. Probably not a huge deal.)
>
> I suppose if pairing of entry and return probes is important for a user,
> he/she can always do the following:
>
> static int ready; // 1 = everybody registered
> // 2 = everybody knows we're registered
> ...
> ready = 0;
> ... register_kprobe(&kp)...
> ... register_retprobe(&rp) ...
> /* instant XXX -- see below*/
> ready = 1;
>
> and in kp.pre_handler do
> if (!ready) {
> // return probe not registered yet
> return 0;
> }
> ready = 2;
> <body of handler>
>
> and in rp.handler do
> if (ready != 2) {
> // Probed function entered during instant XXX,
> // so kp.pre_handler didn't act on it.
> return 0;
> }
> <body of handler>
>
> Keeping a whole group of kprobes, jprobes, and retprobes in the starting
> gate pending a "ready" signal (e.g., for SystemTap) could probably be
> handled similarly.
>
> Unregistration shouldn't be an issue. At any time you can have N active
> instances of the probed function, and have therefore recorded E entries
> and E-N returns. Hien's code handles all that on retprobe
> deregistration, but the user's instrumentation should never count on #
> probed entries == # probed returns.
>
Jim,
You can do something like you explained above to handle the pairing issues.
You need to provide simple and compact interfaces for return probe feature.
Thanks
Prasanna
--
Prasanna S Panchamukhi
Linux Technology Center
India Software Labs, IBM Bangalore
Ph: 91-80-25044636
<[email protected]>