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: Masami Hiramatsu <masami dot hiramatsu dot pt at hitachi dot com>
- To: "Keshavamurthy, Anil S" <anil dot s dot keshavamurthy at intel dot com>, hch at infradead dot org
- Cc: 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>, "Frank Ch. Eigler" <fche at redhat dot com>
- Date: Mon, 26 Mar 2007 12:17:49 +0900
- Subject: Re: [RFC][Patch 1/4] kprobe fast unregistration
- Organization: Systems Development Lab., Hitachi, Ltd., Japan
- References: <4603E7A4.50300@hitachi.com> <20070323180527.GA13728@bambi.jf.intel.com>
Hi Christoph and Anil,
Thank you for your comments.
Christoph Hellwig wrote:
> Speeding up the unregistration is a very good idea, but this interface
> is rather horrible. It's almost a receipe for users to get it wrong.
Keshavamurthy, Anil S wrote:
> I agree with Christop that the interface is horrible and error prone.
OK, I agree. I had chosen a confusable name.
> However, I see the use case where people want to disable the probes quickly and
> would like to reenable them again. Looking closely at your patch,
> I think this can be acheived.
Thank you.
> Here is my suggestion.
>
>> Here is an example code.
>> --
>> struct kprobes *p;
>> for_each_probe(p) {
>> unregister_kprobe_fast(p);
> ^^^^^^^^^^^^^^^^^^^^^^
> Change this to disable_kprobe(p), which is essentially the same as
> what you have implemented. And also provide an opposite function
> to reenable_kprobe(p) which enables the disabled probe again.
I'd like to change that to prepare_to_unregister_kprobe(p) instead of
disable_kprobe(p).
I think Josh and other people want interfaces to disable/reenable all
probes at once when the sysrq is pressed.
So, IMHO, these interfaces should use a global (and per-cpu?) flag which
controls whether kprobes calls user-defined handler or not, instead of
self-modifying.
For example,
if (p && p->pre_handler && kprobe_enable) {
^^^^^^^^^^^^^
...
}
But, I think this would be another story.
I'd like to discuss this topic in other mails.
>> }
>> commit_kprobes();
> ^^^^^^^^^^^^^^
> Change this to unregister_disabled_kprobes(), which essentially
> unregisters all the disabled probes.
And also, I'd like to change it to unregister_prepared_kprobes().
What would you think about this idea?
Thanks,
>
> Thanks,
> Anil Keshavamurthy
>
--
Masami HIRAMATSU
Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com