On 2021/7/11 PM10:09, Masami Hiramatsu wrote:
On Wed, 7 Jul 2021 22:42:47 +0800
Matt Wu <wuqiang.matt@xxxxxxxxxxxxx> wrote:
On 2021/7/7 PM9:29, Masami Hiramatsu wrote:
On Wed, 7 Jul 2021 19:45:30 +0900
Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote:
On Wed, 7 Jul 2021 12:20:57 +0200
Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
On Wed, Jul 07, 2021 at 07:15:10PM +0900, Masami Hiramatsu wrote:
I actually don't want to keep this feature because no one use it.
(only systemtap needs it?)
Yeah, you mentioned systemtap, but since that's out-of-tree I don't
care. Their problem.
Yeah, maybe it is not hard to update.
Anyway, if we keep the idea-level compatibility (not code level),
what we need is 'void *data' in the struct kretprobe_instance.
User who needs it can allocate their own instance data for their
kretprobes when initialising it and sets in their entry handler.
Then we can have a simple kretprobe_instance.
When would you do the alloc? When installing the retprobe, but that
might be inside the allocator, which means you can't call the allocator
etc.. :-)
Yes, so the user may need to allocate a pool right before register_kretprobe().
(whether per-kretprobe or per-task or global pool, that is user's choice.)
If we look at struct ftrace_ret_stack, it has a few fixed function
fields. The calltime one is all that is needed for the kretprobe
example code.
kretprobe consumes 3 fields, a pointer to 'struct kretprobe' (which
stores callee function address in 'kretprobe::kp.addr'), a return
address and a frame pointer (*).
> Oops, I forgot to add "void *data" for storing user data.
Should use "struct kretprobe_holder *rph", since "struct kretprobe" belongs
to 3rd-party module (which might be unloaded any time).
Good catch. Yes, instead of 'struct kretprobe', we need to use the holder.
User's own pool might not work if the module can be unloaded. Better manage
the pool in kretprobe_holder, which needs no changes from user side.
No, since the 'data' will be only refered from user handler. If the kretprobe
is released, then the kretprobe_holder will clear the refernce to the 'struct
kretprobe'. Then, the user handler is never called. No one access the 'data'.
Indeed, there is no race of "data" accessing, since unregister_kretprobes()
is taking care of it.
This implementation just increases the complexity of caller to keep track
of all allocated instances and release them after unregistration.
But guys are likely to use kmalloc in pre-handler and kfree in post-handler,
which will lead to memory leaks.
