This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: [RFC][Patch 1/4] kprobe fast unregistration
- From: "Keshavamurthy, Anil S" <anil dot s dot keshavamurthy at intel dot com>
- To: "Frank Ch. Eigler" <fche at redhat dot com>
- Cc: "Stone, Joshua I" <joshua dot i dot stone at intel dot com>, "Keshavamurthy, Anil S" <anil dot s dot keshavamurthy at intel dot com>, Masami Hiramatsu <masami dot hiramatsu dot pt at hitachi dot com>, Ananth N Mavinakayanahalli <ananth at in dot ibm dot com>, Prasanna S Panchamukhi <prasanna at in dot ibm dot com>, linux-kernel <linux-kernel at vger dot kernel dot org>, SystemTAP <systemtap at sources dot redhat dot com>, Satoshi Oshima <soshima at redhat dot com>, Hideo Aoki <haoki at redhat dot com>, Yumiko Sugita <yumiko dot sugita dot yf at hitachi dot com>, hch at infradead dot org
- Date: Fri, 23 Mar 2007 12:01:27 -0700
- Subject: Re: [RFC][Patch 1/4] kprobe fast unregistration
- References: <20070323180527.GA13728@bambi.jf.intel.com> <16D5B9AB904B0B46B22A27002EE3A8C82793BB@scsmsx415.amr.corp.intel.com> <20070323182248.GA32364@redhat.com>
- Reply-to: "Keshavamurthy, Anil S" <anil dot s dot keshavamurthy at intel dot com>
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