From: Keshavamurthy Anil S <anil.s.keshavamurthy@intel.com>
To: akpm@osdl.org
Cc: ananth@in.ibm.com, akpm@osdl.org, paulmck@us.ibm.com,
linux-kernel@vger.kernel.org, prasanna@in.ibm.com,
systemtap@sources.redhat.com, oleg@tv-sign.ru
Subject: Re: [patch 3/4] Kprobes - Changed from using spinlock to mutext
Date: Wed, 14 Dec 2005 23:53:00 -0000 [thread overview]
Message-ID: <20051214155059.A5350@unix-os.sc.intel.com> (raw)
In-Reply-To: <20051213203901.264302465@csdlinux-2.jf.intel.com>; from anil.s.keshavamurthy@intel.com on Tue, Dec 13, 2005 at 12:35:50PM -0800
On Tue, Dec 13, 2005 at 12:35:50PM -0800, Anil S Keshavamurthy wrote:
> [PATCH] Kprobes - Changed from using spinlock to mutext
>
> Since Kprobes runtime exception handlers is now
> lock free as this code path is now using RCU to walk
> through the list, there is no need for the
> register/unregister{_kprobe} to use
> spin_{lock/unlock}_isr{save/restore}. The serialization
> during registration/unregistration is now possible using
> just a mutex.
>
> In the above process, this patch also fixes a minor memory
> leak for x86_64 and powerpc.
>
> Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
Andrew,
Based on some feedback from Oleg Nesterov, I have
made few changes to previously posted patch.
The below fix should cleanly apply to the patch named
kprobes-changed-form-using-spinlock-to-mutex.patch
in you mm2 tree.
Please consider this for your next mm.
Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
-----------------------------------------------------------------
arch/powerpc/kernel/kprobes.c | 3 ---
arch/x86_64/kernel/kprobes.c | 4 ++--
kernel/kprobes.c | 32 ++++++++++++++++++--------------
3 files changed, 20 insertions(+), 19 deletions(-)
Index: linux-2.6.15-rc5-git3/arch/powerpc/kernel/kprobes.c
===================================================================
--- linux-2.6.15-rc5-git3.orig/arch/powerpc/kernel/kprobes.c
+++ linux-2.6.15-rc5-git3/arch/powerpc/kernel/kprobes.c
@@ -35,7 +35,6 @@
#include <asm/kdebug.h>
#include <asm/sstep.h>
-static DECLARE_MUTEX(kprobe_mutex);
DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
@@ -54,9 +53,7 @@ int __kprobes arch_prepare_kprobe(struct
/* insn must be on a special executable page on ppc64 */
if (!ret) {
- down(&kprobe_mutex);
p->ainsn.insn = get_insn_slot();
- up(&kprobe_mutex);
if (!p->ainsn.insn)
ret = -ENOMEM;
}
Index: linux-2.6.15-rc5-git3/arch/x86_64/kernel/kprobes.c
===================================================================
--- linux-2.6.15-rc5-git3.orig/arch/x86_64/kernel/kprobes.c
+++ linux-2.6.15-rc5-git3/arch/x86_64/kernel/kprobes.c
@@ -43,7 +43,7 @@
#include <asm/kdebug.h>
void jprobe_return_end(void);
-void __kprobes arch_copy_kprobe(struct kprobe *p);
+static void __kprobes arch_copy_kprobe(struct kprobe *p);
DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
@@ -180,7 +180,7 @@ static inline s32 *is_riprel(u8 *insn)
return NULL;
}
-void __kprobes arch_copy_kprobe(struct kprobe *p)
+static void __kprobes arch_copy_kprobe(struct kprobe *p)
{
s32 *ripdisp;
memcpy(p->ainsn.insn, p->addr, MAX_INSN_SIZE);
Index: linux-2.6.15-rc5-git3/kernel/kprobes.c
===================================================================
--- linux-2.6.15-rc5-git3.orig/kernel/kprobes.c
+++ linux-2.6.15-rc5-git3/kernel/kprobes.c
@@ -431,7 +431,7 @@ static int __kprobes register_aggr_kprob
copy_kprobe(old_p, p);
ret = add_new_kprobe(old_p, p);
} else {
- ap = kcalloc(1, sizeof(struct kprobe), GFP_ATOMIC);
+ ap = kcalloc(1, sizeof(struct kprobe), GFP_KERNEL);
if (!ap)
return -ENOMEM;
add_aggr_kprobe(ap, old_p);
@@ -491,7 +491,8 @@ out:
void __kprobes unregister_kprobe(struct kprobe *p)
{
struct module *mod;
- struct kprobe *old_p, *cleanup_p;
+ struct kprobe *old_p, *list_p;
+ int cleanup_p;
down(&kprobe_mutex);
old_p = get_kprobe(p->addr);
@@ -499,22 +500,25 @@ void __kprobes unregister_kprobe(struct
up(&kprobe_mutex);
return;
}
-
- if ((old_p->pre_handler == aggr_pre_handler) &&
+ if (p != old_p) {
+ list_for_each_entry_rcu(list_p, &old_p->list, list)
+ if (list_p == p)
+ /* kprobe p is a valid probe */
+ goto valid_p;
+ up(&kprobe_mutex);
+ return;
+ }
+valid_p:
+ if ((old_p == p) || ((old_p->pre_handler == aggr_pre_handler) &&
(p->list.next == &old_p->list) &&
- (p->list.prev == &old_p->list)) {
- /* Only one element in the aggregate list */
+ (p->list.prev == &old_p->list))) {
+ /* Only probe on the hash list */
arch_disarm_kprobe(p);
hlist_del_rcu(&old_p->hlist);
- cleanup_p = old_p;
- } else if (old_p == p) {
- /* Only one kprobe element in the hash list */
- arch_disarm_kprobe(p);
- hlist_del_rcu(&p->hlist);
- cleanup_p = p;
+ cleanup_p = 1;
} else {
list_del_rcu(&p->list);
- cleanup_p = NULL;
+ cleanup_p = 0;
}
up(&kprobe_mutex);
@@ -524,7 +528,7 @@ void __kprobes unregister_kprobe(struct
module_put(mod);
if (cleanup_p) {
- if (cleanup_p->pre_handler == aggr_pre_handler) {
+ if (p != old_p) {
list_del_rcu(&p->list);
kfree(old_p);
}
next prev parent reply other threads:[~2005-12-14 23:51 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20051213203547.649087523@csdlinux-2.jf.intel.com>
2005-12-13 20:55 ` [patch 2/4] Kprobes - cleanup include_asm_kprobes_h Anil S Keshavamurthy
2005-12-13 20:55 ` [patch 3/4] Kprobes - Changed from using spinlock to mutext Anil S Keshavamurthy
2005-12-14 23:53 ` Keshavamurthy Anil S [this message]
2005-12-13 20:55 ` [patch 4/4] Kprobes Cleanup arch_remove_kprobe Anil S Keshavamurthy
2005-12-13 21:16 ` [patch 1/4] Kprobes - Enable funcions only for required arch Anil S Keshavamurthy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20051214155059.A5350@unix-os.sc.intel.com \
--to=anil.s.keshavamurthy@intel.com \
--cc=akpm@osdl.org \
--cc=ananth@in.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=oleg@tv-sign.ru \
--cc=paulmck@us.ibm.com \
--cc=prasanna@in.ibm.com \
--cc=systemtap@sources.redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).