This is the mail archive of the mailing list for the systemtap project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC][Patch 1/4] kprobe fast unregistration

On Fri, Mar 23, 2007 at 02:22:48PM -0400, Frank Ch. Eigler wrote:
> Hi -
> Keshavamurthy, Anil S wrote:
> > I agree with Christoph that the interface is horrible and error prone.
> Really?  What possible problems can occur?  The worst that occurs to
> me is that if someone forgets to call the commit function, the kprobes
> will still be disabled, but memory won't be recycled for a while.  Is
> this really so bad, considering that a kprobe_unregister is to imply a
> commit?  Maybe if kprobe_register can also implied a commit, we can
> bound the conceivable memory leak.
Yes, Have you looked at the code? If someone forgets to call the
commit function, the kprobe will be disabled and yes the memory won't
be recycled but the worst problem is that if the probe is on a module
function then that module can't be unloaded at all as the
register_kprobe() would have taken a reference on that module.

Hence, my suggestion would be to call them as disable_kprobe()
(instead of unregister_kprobes_fast() which is confusing and error prone)
and also to provide an opposite function to reenable_kprobe()
and finally provide unregister_disabled_kprobes() which is
essentially the same as commit_kprobes(). With this 
name changes the intentions of the new exported functions
will be clear for users and we don;t have to discard the hard work
that has gone into this patch.

> Would it be possible to allay even that concern with an automated
> deferred/periodic commit?
I would recomand that users call unregister_disabled_kprobes() explictly.

-Anil Keshavamurthy

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]