linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch 0/4] Kprobes cleanup patches
@ 2005-12-13 20:35 Anil S Keshavamurthy
  2005-12-13 20:35 ` [patch 1/4] Kprobes - Enable funcions only for required arch Anil S Keshavamurthy
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Anil S Keshavamurthy @ 2005-12-13 20:35 UTC (permalink / raw)
  To: ananth, akpm, paulmck; +Cc: linux-kernel

Here are the set of patches tagged to this mail.
1)Subject: Kprobes - Enable funcions only for required arch
	This is a cleanup patch
2)Subject: Kprobes - cleanup include_asm_kprobes_h
	This is a cleanup patch
3)Subject: Kprobes - Changed from using spinlock to mutext
	This patch is a improvement patch which now
		uses mutex over spinlock for 
	registration/unregistration code path.
4)Subject: Kprobes Cleanup arch_remove_kprobe
	This is a cleanup patch

More description inside the patch itself.

Andrew, please include these in your mm tree.

thanks,
Anil Keshavamurthy


--


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [patch 1/4] Kprobes - Enable funcions only for required arch
  2005-12-13 20:35 [patch 0/4] Kprobes cleanup patches Anil S Keshavamurthy
@ 2005-12-13 20:35 ` Anil S Keshavamurthy
  2005-12-13 20:35 ` [patch 2/4] Kprobes - cleanup include_asm_kprobes_h Anil S Keshavamurthy
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Anil S Keshavamurthy @ 2005-12-13 20:35 UTC (permalink / raw)
  To: ananth, akpm, paulmck; +Cc: linux-kernel, systemtap, Anil S Keshavamurthy

[-- Attachment #1: cleanup_inst_slot.patch --]
[-- Type: text/plain, Size: 2394 bytes --]

[PATCH] Kprobes - Enable funcions only for required arch

	Kernel/kprobes.c defines get_insn_slot() and free_insn_slot()
which are currently required _only_ for x86_64 and powerpc (which has
no-exec support).

FYI, get{free}_insn_slot() functions manages the memory
page which is mapped as executable, required for instruction
emulation.

This patch moves those two functions under
__ARCH_WANT_KPROBES_INSN_SLOT and defines
__ARCH_WANT_KPROBES_INSN_SLOT in arch specific
kprobes.h file.

Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
------------------------------------------------------------------

 include/asm-powerpc/kprobes.h |    2 ++
 include/asm-x86_64/kprobes.h  |    2 ++
 kernel/kprobes.c              |    2 ++
 3 files changed, 6 insertions(+)

Index: linux-2.6.15-rc5-mm2/include/asm-powerpc/kprobes.h
===================================================================
--- linux-2.6.15-rc5-mm2.orig/include/asm-powerpc/kprobes.h
+++ linux-2.6.15-rc5-mm2/include/asm-powerpc/kprobes.h
@@ -29,6 +29,8 @@
 #include <linux/ptrace.h>
 #include <linux/percpu.h>
 
+#define  __ARCH_WANT_KPROBES_INSN_SLOT
+
 struct pt_regs;
 
 typedef unsigned int kprobe_opcode_t;
Index: linux-2.6.15-rc5-mm2/include/asm-x86_64/kprobes.h
===================================================================
--- linux-2.6.15-rc5-mm2.orig/include/asm-x86_64/kprobes.h
+++ linux-2.6.15-rc5-mm2/include/asm-x86_64/kprobes.h
@@ -27,6 +27,8 @@
 #include <linux/ptrace.h>
 #include <linux/percpu.h>
 
+#define  __ARCH_WANT_KPROBES_INSN_SLOT
+
 struct pt_regs;
 
 typedef u8 kprobe_opcode_t;
Index: linux-2.6.15-rc5-mm2/kernel/kprobes.c
===================================================================
--- linux-2.6.15-rc5-mm2.orig/kernel/kprobes.c
+++ linux-2.6.15-rc5-mm2/kernel/kprobes.c
@@ -52,6 +52,7 @@ static DEFINE_SPINLOCK(kprobe_lock);	/* 
 DEFINE_SPINLOCK(kretprobe_lock);	/* Protects kretprobe_inst_table */
 static DEFINE_PER_CPU(struct kprobe *, kprobe_instance) = NULL;
 
+#ifdef __ARCH_WANT_KPROBES_INSN_SLOT
 /*
  * kprobe->ainsn.insn points to the copy of the instruction to be
  * single-stepped. x86_64, POWER4 and above have no-exec support and
@@ -151,6 +152,7 @@ void __kprobes free_insn_slot(kprobe_opc
 		}
 	}
 }
+#endif
 
 /* We have preemption disabled.. so it is safe to use __ versions */
 static inline void set_kprobe_instance(struct kprobe *kp)

--


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [patch 2/4] Kprobes - cleanup include_asm_kprobes_h
  2005-12-13 20:35 [patch 0/4] Kprobes cleanup patches Anil S Keshavamurthy
  2005-12-13 20:35 ` [patch 1/4] Kprobes - Enable funcions only for required arch Anil S Keshavamurthy
@ 2005-12-13 20:35 ` Anil S Keshavamurthy
  2005-12-13 20:35 ` [patch 3/4] Kprobes - Changed from using spinlock to mutext Anil S Keshavamurthy
  2005-12-13 20:35 ` [patch 4/4] Kprobes Cleanup arch_remove_kprobe Anil S Keshavamurthy
  3 siblings, 0 replies; 6+ messages in thread
From: Anil S Keshavamurthy @ 2005-12-13 20:35 UTC (permalink / raw)
  To: ananth, akpm, paulmck; +Cc: linux-kernel, systemtap, Anil S Keshavamurthy

[-- Attachment #1: cleanup_arch_kprobe_h.patch --]
[-- Type: text/plain, Size: 3430 bytes --]

[PATCH] kprobes - cleanup include_asm_kprobes_h

The arch specific kprobes.h files never gets included
when CONFIG_KPROBES is turned off. Hence check for
CONFIG_KPROBES is not appropriate here in this 
arch specific kprobes.h files.

Also the below defined function kprobes_exception_notify()
is not needed when CONFIG_KPROBES is off.

Compile tested for both CONFIG_KPROBES=y and N.

Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
--------------------------------------------------------------------

 include/asm-i386/kprobes.h    |    8 --------
 include/asm-ia64/kprobes.h    |    8 --------
 include/asm-powerpc/kprobes.h |    8 --------
 include/asm-sparc64/kprobes.h |    9 ---------
 4 files changed, 33 deletions(-)

Index: linux-2.6.15-rc5-mm2/include/asm-i386/kprobes.h
===================================================================
--- linux-2.6.15-rc5-mm2.orig/include/asm-i386/kprobes.h
+++ linux-2.6.15-rc5-mm2/include/asm-i386/kprobes.h
@@ -76,14 +76,6 @@ static inline void restore_interrupts(st
 		local_irq_enable();
 }
 
-#ifdef CONFIG_KPROBES
 extern int kprobe_exceptions_notify(struct notifier_block *self,
 				    unsigned long val, void *data);
-#else				/* !CONFIG_KPROBES */
-static inline int kprobe_exceptions_notify(struct notifier_block *self,
-					   unsigned long val, void *data)
-{
-	return 0;
-}
-#endif
 #endif				/* _ASM_KPROBES_H */
Index: linux-2.6.15-rc5-mm2/include/asm-ia64/kprobes.h
===================================================================
--- linux-2.6.15-rc5-mm2.orig/include/asm-ia64/kprobes.h
+++ linux-2.6.15-rc5-mm2/include/asm-ia64/kprobes.h
@@ -115,7 +115,6 @@ static inline void arch_copy_kprobe(stru
 {
 }
 
-#ifdef CONFIG_KPROBES
 extern int kprobe_exceptions_notify(struct notifier_block *self,
 				    unsigned long val, void *data);
 
@@ -124,11 +123,4 @@ static inline void jprobe_return(void)
 {
 }
 
-#else				/* !CONFIG_KPROBES */
-static inline int kprobe_exceptions_notify(struct notifier_block *self,
-					   unsigned long val, void *data)
-{
-	return 0;
-}
-#endif
 #endif				/* _ASM_KPROBES_H */
Index: linux-2.6.15-rc5-mm2/include/asm-powerpc/kprobes.h
===================================================================
--- linux-2.6.15-rc5-mm2.orig/include/asm-powerpc/kprobes.h
+++ linux-2.6.15-rc5-mm2/include/asm-powerpc/kprobes.h
@@ -70,14 +70,6 @@ struct kprobe_ctlblk {
 	struct prev_kprobe prev_kprobe;
 };
 
-#ifdef CONFIG_KPROBES
 extern int kprobe_exceptions_notify(struct notifier_block *self,
 				    unsigned long val, void *data);
-#else				/* !CONFIG_KPROBES */
-static inline int kprobe_exceptions_notify(struct notifier_block *self,
-					   unsigned long val, void *data)
-{
-	return 0;
-}
-#endif
 #endif	/* _ASM_POWERPC_KPROBES_H */
Index: linux-2.6.15-rc5-mm2/include/asm-sparc64/kprobes.h
===================================================================
--- linux-2.6.15-rc5-mm2.orig/include/asm-sparc64/kprobes.h
+++ linux-2.6.15-rc5-mm2/include/asm-sparc64/kprobes.h
@@ -38,15 +38,6 @@ struct kprobe_ctlblk {
 	struct prev_kprobe prev_kprobe;
 };
 
-#ifdef CONFIG_KPROBES
 extern int kprobe_exceptions_notify(struct notifier_block *self,
 				    unsigned long val, void *data);
-#else				/* !CONFIG_KPROBES */
-static inline int kprobe_exceptions_notify(struct notifier_block *self,
-					   unsigned long val, void *data)
-{
-	return 0;
-}
-#endif
-
 #endif /* _SPARC64_KPROBES_H */

--


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [patch 3/4] Kprobes - Changed from using spinlock to mutext
  2005-12-13 20:35 [patch 0/4] Kprobes cleanup patches Anil S Keshavamurthy
  2005-12-13 20:35 ` [patch 1/4] Kprobes - Enable funcions only for required arch Anil S Keshavamurthy
  2005-12-13 20:35 ` [patch 2/4] Kprobes - cleanup include_asm_kprobes_h Anil S Keshavamurthy
@ 2005-12-13 20:35 ` Anil S Keshavamurthy
  2005-12-14 23:50   ` Keshavamurthy Anil S
  2005-12-13 20:35 ` [patch 4/4] Kprobes Cleanup arch_remove_kprobe Anil S Keshavamurthy
  3 siblings, 1 reply; 6+ messages in thread
From: Anil S Keshavamurthy @ 2005-12-13 20:35 UTC (permalink / raw)
  To: ananth, akpm, paulmck
  Cc: linux-kernel, prasanna, systemtap, Anil S Keshavamurthy

[-- Attachment #1: rcu_unregister_race_fix_v2.patch --]
[-- Type: text/plain, Size: 10040 bytes --]

[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>
===================================================================

 arch/i386/kernel/kprobes.c    |    6 --
 arch/powerpc/kernel/kprobes.c |   14 ++----
 arch/sparc64/kernel/kprobes.c |    6 --
 arch/x86_64/kernel/kprobes.c  |    7 ---
 include/asm-ia64/kprobes.h    |    5 --
 include/linux/kprobes.h       |    1 
 kernel/kprobes.c              |   91 +++++++++++++++++++-----------------------
 7 files changed, 53 insertions(+), 77 deletions(-)

Index: linux-2.6.15-rc5-mm2/arch/i386/kernel/kprobes.c
===================================================================
--- linux-2.6.15-rc5-mm2.orig/arch/i386/kernel/kprobes.c
+++ linux-2.6.15-rc5-mm2/arch/i386/kernel/kprobes.c
@@ -58,13 +58,9 @@ static inline int is_IF_modifier(kprobe_
 
 int __kprobes arch_prepare_kprobe(struct kprobe *p)
 {
-	return 0;
-}
-
-void __kprobes arch_copy_kprobe(struct kprobe *p)
-{
 	memcpy(p->ainsn.insn, p->addr, MAX_INSN_SIZE * sizeof(kprobe_opcode_t));
 	p->opcode = *p->addr;
+	return 0;
 }
 
 void __kprobes arch_arm_kprobe(struct kprobe *p)
Index: linux-2.6.15-rc5-mm2/arch/powerpc/kernel/kprobes.c
===================================================================
--- linux-2.6.15-rc5-mm2.orig/arch/powerpc/kernel/kprobes.c
+++ linux-2.6.15-rc5-mm2/arch/powerpc/kernel/kprobes.c
@@ -60,13 +60,13 @@ int __kprobes arch_prepare_kprobe(struct
 		if (!p->ainsn.insn)
 			ret = -ENOMEM;
 	}
-	return ret;
-}
 
-void __kprobes arch_copy_kprobe(struct kprobe *p)
-{
-	memcpy(p->ainsn.insn, p->addr, MAX_INSN_SIZE * sizeof(kprobe_opcode_t));
-	p->opcode = *p->addr;
+	if (!ret) {
+		memcpy(p->ainsn.insn, p->addr, MAX_INSN_SIZE * sizeof(kprobe_opcode_t));
+		p->opcode = *p->addr;
+	}
+
+	return ret;
 }
 
 void __kprobes arch_arm_kprobe(struct kprobe *p)
@@ -85,9 +85,7 @@ void __kprobes arch_disarm_kprobe(struct
 
 void __kprobes arch_remove_kprobe(struct kprobe *p)
 {
-	down(&kprobe_mutex);
 	free_insn_slot(p->ainsn.insn);
-	up(&kprobe_mutex);
 }
 
 static inline void prepare_singlestep(struct kprobe *p, struct pt_regs *regs)
Index: linux-2.6.15-rc5-mm2/arch/sparc64/kernel/kprobes.c
===================================================================
--- linux-2.6.15-rc5-mm2.orig/arch/sparc64/kernel/kprobes.c
+++ linux-2.6.15-rc5-mm2/arch/sparc64/kernel/kprobes.c
@@ -43,14 +43,10 @@ DEFINE_PER_CPU(struct kprobe_ctlblk, kpr
 
 int __kprobes arch_prepare_kprobe(struct kprobe *p)
 {
-	return 0;
-}
-
-void __kprobes arch_copy_kprobe(struct kprobe *p)
-{
 	p->ainsn.insn[0] = *p->addr;
 	p->ainsn.insn[1] = BREAKPOINT_INSTRUCTION_2;
 	p->opcode = *p->addr;
+	return 0;
 }
 
 void __kprobes arch_arm_kprobe(struct kprobe *p)
Index: linux-2.6.15-rc5-mm2/arch/x86_64/kernel/kprobes.c
===================================================================
--- linux-2.6.15-rc5-mm2.orig/arch/x86_64/kernel/kprobes.c
+++ linux-2.6.15-rc5-mm2/arch/x86_64/kernel/kprobes.c
@@ -42,8 +42,8 @@
 #include <asm/pgtable.h>
 #include <asm/kdebug.h>
 
-static DECLARE_MUTEX(kprobe_mutex);
 void jprobe_return_end(void);
+void __kprobes arch_copy_kprobe(struct kprobe *p);
 
 DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
 DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
@@ -69,12 +69,11 @@ static inline int is_IF_modifier(kprobe_
 int __kprobes arch_prepare_kprobe(struct kprobe *p)
 {
 	/* insn: must be on special executable page on x86_64. */
-	down(&kprobe_mutex);
 	p->ainsn.insn = get_insn_slot();
-	up(&kprobe_mutex);
 	if (!p->ainsn.insn) {
 		return -ENOMEM;
 	}
+	arch_copy_kprobe(p);
 	return 0;
 }
 
@@ -223,9 +222,7 @@ void __kprobes arch_disarm_kprobe(struct
 
 void __kprobes arch_remove_kprobe(struct kprobe *p)
 {
-	down(&kprobe_mutex);
 	free_insn_slot(p->ainsn.insn);
-	up(&kprobe_mutex);
 }
 
 static inline void save_previous_kprobe(struct kprobe_ctlblk *kcb)
Index: linux-2.6.15-rc5-mm2/include/asm-ia64/kprobes.h
===================================================================
--- linux-2.6.15-rc5-mm2.orig/include/asm-ia64/kprobes.h
+++ linux-2.6.15-rc5-mm2/include/asm-ia64/kprobes.h
@@ -110,11 +110,6 @@ struct arch_specific_insn {
  	unsigned short target_br_reg;
 };
 
-/* ia64 does not need this */
-static inline void arch_copy_kprobe(struct kprobe *p)
-{
-}
-
 extern int kprobe_exceptions_notify(struct notifier_block *self,
 				    unsigned long val, void *data);
 
Index: linux-2.6.15-rc5-mm2/include/linux/kprobes.h
===================================================================
--- linux-2.6.15-rc5-mm2.orig/include/linux/kprobes.h
+++ linux-2.6.15-rc5-mm2/include/linux/kprobes.h
@@ -150,7 +150,6 @@ struct kretprobe_instance {
 
 extern spinlock_t kretprobe_lock;
 extern int arch_prepare_kprobe(struct kprobe *p);
-extern void arch_copy_kprobe(struct kprobe *p);
 extern void arch_arm_kprobe(struct kprobe *p);
 extern void arch_disarm_kprobe(struct kprobe *p);
 extern void arch_remove_kprobe(struct kprobe *p);
Index: linux-2.6.15-rc5-mm2/kernel/kprobes.c
===================================================================
--- linux-2.6.15-rc5-mm2.orig/kernel/kprobes.c
+++ linux-2.6.15-rc5-mm2/kernel/kprobes.c
@@ -48,7 +48,7 @@
 static struct hlist_head kprobe_table[KPROBE_TABLE_SIZE];
 static struct hlist_head kretprobe_inst_table[KPROBE_TABLE_SIZE];
 
-static DEFINE_SPINLOCK(kprobe_lock);	/* Protects kprobe_table */
+static DECLARE_MUTEX(kprobe_mutex);	/* Protects kprobe_table */
 DEFINE_SPINLOCK(kretprobe_lock);	/* Protects kretprobe_inst_table */
 static DEFINE_PER_CPU(struct kprobe *, kprobe_instance) = NULL;
 
@@ -167,7 +167,7 @@ static inline void reset_kprobe_instance
 
 /*
  * This routine is called either:
- * 	- under the kprobe_lock spinlock - during kprobe_[un]register()
+ * 	- under the kprobe_mutex - during kprobe_[un]register()
  * 				OR
  * 	- with preemption disabled - from arch/xxx/kernel/kprobes.c
  */
@@ -420,7 +420,6 @@ static inline void add_aggr_kprobe(struc
 /*
  * This is the second or subsequent kprobe at the address - handle
  * the intricacies
- * TODO: Move kcalloc outside the spin_lock
  */
 static int __kprobes register_aggr_kprobe(struct kprobe *old_p,
 					  struct kprobe *p)
@@ -442,25 +441,6 @@ static int __kprobes register_aggr_kprob
 	return ret;
 }
 
-/* kprobe removal house-keeping routines */
-static inline void cleanup_kprobe(struct kprobe *p, unsigned long flags)
-{
-	arch_disarm_kprobe(p);
-	hlist_del_rcu(&p->hlist);
-	spin_unlock_irqrestore(&kprobe_lock, flags);
-	arch_remove_kprobe(p);
-}
-
-static inline void cleanup_aggr_kprobe(struct kprobe *old_p,
-		struct kprobe *p, unsigned long flags)
-{
-	list_del_rcu(&p->list);
-	if (list_empty(&old_p->list))
-		cleanup_kprobe(old_p, flags);
-	else
-		spin_unlock_irqrestore(&kprobe_lock, flags);
-}
-
 static int __kprobes in_kprobes_functions(unsigned long addr)
 {
 	if (addr >= (unsigned long)__kprobes_text_start
@@ -472,7 +452,6 @@ static int __kprobes in_kprobes_function
 int __kprobes register_kprobe(struct kprobe *p)
 {
 	int ret = 0;
-	unsigned long flags = 0;
 	struct kprobe *old_p;
 	struct module *mod;
 
@@ -484,18 +463,17 @@ int __kprobes register_kprobe(struct kpr
 			(unlikely(!try_module_get(mod))))
 		return -EINVAL;
 
-	if ((ret = arch_prepare_kprobe(p)) != 0)
-		goto rm_kprobe;
-
 	p->nmissed = 0;
-	spin_lock_irqsave(&kprobe_lock, flags);
+	down(&kprobe_mutex);
 	old_p = get_kprobe(p->addr);
 	if (old_p) {
 		ret = register_aggr_kprobe(old_p, p);
 		goto out;
 	}
 
-	arch_copy_kprobe(p);
+	if ((ret = arch_prepare_kprobe(p)) != 0)
+		goto out;
+
 	INIT_HLIST_NODE(&p->hlist);
 	hlist_add_head_rcu(&p->hlist,
 		       &kprobe_table[hash_ptr(p->addr, KPROBE_HASH_BITS)]);
@@ -503,10 +481,8 @@ int __kprobes register_kprobe(struct kpr
   	arch_arm_kprobe(p);
 
 out:
-	spin_unlock_irqrestore(&kprobe_lock, flags);
-rm_kprobe:
-	if (ret == -EEXIST)
-		arch_remove_kprobe(p);
+	up(&kprobe_mutex);
+
 	if (ret && mod)
 		module_put(mod);
 	return ret;
@@ -514,29 +490,48 @@ rm_kprobe:
 
 void __kprobes unregister_kprobe(struct kprobe *p)
 {
-	unsigned long flags;
-	struct kprobe *old_p;
 	struct module *mod;
+	struct kprobe *old_p, *cleanup_p;
 
-	spin_lock_irqsave(&kprobe_lock, flags);
+	down(&kprobe_mutex);
 	old_p = get_kprobe(p->addr);
-	if (old_p) {
-		/* cleanup_*_kprobe() does the spin_unlock_irqrestore */
-		if (old_p->pre_handler == aggr_pre_handler)
-			cleanup_aggr_kprobe(old_p, p, flags);
-		else
-			cleanup_kprobe(p, flags);
+	if (unlikely(!old_p)) {
+		up(&kprobe_mutex);
+		return;
+	}
+
+	if ((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 */
+		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;
+	} else {
+		list_del_rcu(&p->list);
+		cleanup_p = NULL;
+	}
 
-		synchronize_sched();
+	up(&kprobe_mutex);
 
-		if ((mod = module_text_address((unsigned long)p->addr)))
-			module_put(mod);
+	synchronize_sched();
+	if ((mod = module_text_address((unsigned long)p->addr)))
+		module_put(mod);
 
-		if (old_p->pre_handler == aggr_pre_handler &&
-				list_empty(&old_p->list))
+	if (cleanup_p) {
+		if (cleanup_p->pre_handler == aggr_pre_handler) {
+			list_del_rcu(&p->list);
 			kfree(old_p);
-	} else
-		spin_unlock_irqrestore(&kprobe_lock, flags);
+		}
+		down(&kprobe_mutex);
+		arch_remove_kprobe(p);
+		up(&kprobe_mutex);
+	}
 }
 
 static struct notifier_block kprobe_exceptions_nb = {

--


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [patch 4/4] Kprobes Cleanup arch_remove_kprobe
  2005-12-13 20:35 [patch 0/4] Kprobes cleanup patches Anil S Keshavamurthy
                   ` (2 preceding siblings ...)
  2005-12-13 20:35 ` [patch 3/4] Kprobes - Changed from using spinlock to mutext Anil S Keshavamurthy
@ 2005-12-13 20:35 ` Anil S Keshavamurthy
  3 siblings, 0 replies; 6+ messages in thread
From: Anil S Keshavamurthy @ 2005-12-13 20:35 UTC (permalink / raw)
  To: ananth, akpm, paulmck; +Cc: linux-kernel, systemtap, Anil S Keshavamurthy

[-- Attachment #1: cleanup_arch_remove_kprobe.patch --]
[-- Type: text/plain, Size: 7537 bytes --]

[PATCH] Kprobes Cleanup arch_remove_kprobe

Currently arch_remove_kprobes() is only implemented/required
for x86_64 and powerpc. All other architecture like
IA64, i386 and sparc64 implementes a dummy function which
is being called from arch independent kprobes.c file.

This patch removes the dummy functions and replaces it with
#define arch_remove_kprobe(p, s)	do { } while(0)

Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
-------------------------------------------------------------------------------
 arch/i386/kernel/kprobes.c    |    4 ----
 arch/ia64/kernel/kprobes.c    |    4 ----
 arch/powerpc/kernel/kprobes.c |    4 +++-
 arch/sparc64/kernel/kprobes.c |    4 ----
 arch/x86_64/kernel/kprobes.c  |    4 +++-
 include/asm-i386/kprobes.h    |    1 +
 include/asm-ia64/kprobes.h    |    1 +
 include/asm-powerpc/kprobes.h |    1 +
 include/asm-sparc64/kprobes.h |    1 +
 include/asm-x86_64/kprobes.h  |    1 +
 include/linux/kprobes.h       |    1 -
 kernel/kprobes.c              |    4 +---
 12 files changed, 12 insertions(+), 18 deletions(-)

Index: linux-2.6.15-rc5-mm2/arch/i386/kernel/kprobes.c
===================================================================
--- linux-2.6.15-rc5-mm2.orig/arch/i386/kernel/kprobes.c
+++ linux-2.6.15-rc5-mm2/arch/i386/kernel/kprobes.c
@@ -77,10 +77,6 @@ void __kprobes arch_disarm_kprobe(struct
 			   (unsigned long) p->addr + sizeof(kprobe_opcode_t));
 }
 
-void __kprobes arch_remove_kprobe(struct kprobe *p)
-{
-}
-
 static inline void save_previous_kprobe(struct kprobe_ctlblk *kcb)
 {
 	kcb->prev_kprobe.kp = kprobe_running();
Index: linux-2.6.15-rc5-mm2/arch/ia64/kernel/kprobes.c
===================================================================
--- linux-2.6.15-rc5-mm2.orig/arch/ia64/kernel/kprobes.c
+++ linux-2.6.15-rc5-mm2/arch/ia64/kernel/kprobes.c
@@ -467,10 +467,6 @@ void __kprobes arch_disarm_kprobe(struct
 	flush_icache_range(arm_addr, arm_addr + sizeof(bundle_t));
 }
 
-void __kprobes arch_remove_kprobe(struct kprobe *p)
-{
-}
-
 /*
  * We are resuming execution after a single step fault, so the pt_regs
  * structure reflects the register state after we executed the instruction
Index: linux-2.6.15-rc5-mm2/arch/powerpc/kernel/kprobes.c
===================================================================
--- linux-2.6.15-rc5-mm2.orig/arch/powerpc/kernel/kprobes.c
+++ linux-2.6.15-rc5-mm2/arch/powerpc/kernel/kprobes.c
@@ -83,9 +83,11 @@ void __kprobes arch_disarm_kprobe(struct
 			   (unsigned long) p->addr + sizeof(kprobe_opcode_t));
 }
 
-void __kprobes arch_remove_kprobe(struct kprobe *p)
+void __kprobes arch_remove_kprobe(struct kprobe *p, struct semaphore *s)
 {
+	down(s);
 	free_insn_slot(p->ainsn.insn);
+	up(s);
 }
 
 static inline void prepare_singlestep(struct kprobe *p, struct pt_regs *regs)
Index: linux-2.6.15-rc5-mm2/arch/sparc64/kernel/kprobes.c
===================================================================
--- linux-2.6.15-rc5-mm2.orig/arch/sparc64/kernel/kprobes.c
+++ linux-2.6.15-rc5-mm2/arch/sparc64/kernel/kprobes.c
@@ -61,10 +61,6 @@ void __kprobes arch_disarm_kprobe(struct
 	flushi(p->addr);
 }
 
-void __kprobes arch_remove_kprobe(struct kprobe *p)
-{
-}
-
 static inline void save_previous_kprobe(struct kprobe_ctlblk *kcb)
 {
 	kcb->prev_kprobe.kp = kprobe_running();
Index: linux-2.6.15-rc5-mm2/arch/x86_64/kernel/kprobes.c
===================================================================
--- linux-2.6.15-rc5-mm2.orig/arch/x86_64/kernel/kprobes.c
+++ linux-2.6.15-rc5-mm2/arch/x86_64/kernel/kprobes.c
@@ -220,9 +220,11 @@ void __kprobes arch_disarm_kprobe(struct
 			   (unsigned long) p->addr + sizeof(kprobe_opcode_t));
 }
 
-void __kprobes arch_remove_kprobe(struct kprobe *p)
+void __kprobes arch_remove_kprobe(struct kprobe *p, struct semaphore *s)
 {
+	down(s);
 	free_insn_slot(p->ainsn.insn);
+	up(s);
 }
 
 static inline void save_previous_kprobe(struct kprobe_ctlblk *kcb)
Index: linux-2.6.15-rc5-mm2/include/asm-i386/kprobes.h
===================================================================
--- linux-2.6.15-rc5-mm2.orig/include/asm-i386/kprobes.h
+++ linux-2.6.15-rc5-mm2/include/asm-i386/kprobes.h
@@ -40,6 +40,7 @@ typedef u8 kprobe_opcode_t;
 
 #define JPROBE_ENTRY(pentry)	(kprobe_opcode_t *)pentry
 #define ARCH_SUPPORTS_KRETPROBES
+#define arch_remove_kprobe(p, s)	do { } while(0)
 
 void kretprobe_trampoline(void);
 
Index: linux-2.6.15-rc5-mm2/include/asm-ia64/kprobes.h
===================================================================
--- linux-2.6.15-rc5-mm2.orig/include/asm-ia64/kprobes.h
+++ linux-2.6.15-rc5-mm2/include/asm-ia64/kprobes.h
@@ -89,6 +89,7 @@ struct kprobe_ctlblk {
 #define IP_RELATIVE_PREDICT_OPCODE	(7)
 #define LONG_BRANCH_OPCODE		(0xC)
 #define LONG_CALL_OPCODE		(0xD)
+#define arch_remove_kprobe(p, s)	do { } while(0)
 
 typedef struct kprobe_opcode {
 	bundle_t bundle;
Index: linux-2.6.15-rc5-mm2/include/asm-powerpc/kprobes.h
===================================================================
--- linux-2.6.15-rc5-mm2.orig/include/asm-powerpc/kprobes.h
+++ linux-2.6.15-rc5-mm2/include/asm-powerpc/kprobes.h
@@ -49,6 +49,7 @@ typedef unsigned int kprobe_opcode_t;
 
 #define ARCH_SUPPORTS_KRETPROBES
 void kretprobe_trampoline(void);
+extern void arch_remove_kprobe(struct kprobe *p, struct semaphore *s);
 
 /* Architecture specific copy of original instruction */
 struct arch_specific_insn {
Index: linux-2.6.15-rc5-mm2/include/asm-sparc64/kprobes.h
===================================================================
--- linux-2.6.15-rc5-mm2.orig/include/asm-sparc64/kprobes.h
+++ linux-2.6.15-rc5-mm2/include/asm-sparc64/kprobes.h
@@ -12,6 +12,7 @@ typedef u32 kprobe_opcode_t;
 #define MAX_INSN_SIZE 2
 
 #define JPROBE_ENTRY(pentry)	(kprobe_opcode_t *)pentry
+#define arch_remove_kprobe(p, s)	do { } while(0)
 
 /* Architecture specific copy of original instruction*/
 struct arch_specific_insn {
Index: linux-2.6.15-rc5-mm2/include/asm-x86_64/kprobes.h
===================================================================
--- linux-2.6.15-rc5-mm2.orig/include/asm-x86_64/kprobes.h
+++ linux-2.6.15-rc5-mm2/include/asm-x86_64/kprobes.h
@@ -78,6 +78,7 @@ static inline void restore_interrupts(st
 		local_irq_enable();
 }
 
+extern void arch_remove_kprobe(struct kprobe *p, struct semaphore *s);
 extern int post_kprobe_handler(struct pt_regs *regs);
 extern int kprobe_fault_handler(struct pt_regs *regs, int trapnr);
 extern int kprobe_handler(struct pt_regs *regs);
Index: linux-2.6.15-rc5-mm2/include/linux/kprobes.h
===================================================================
--- linux-2.6.15-rc5-mm2.orig/include/linux/kprobes.h
+++ linux-2.6.15-rc5-mm2/include/linux/kprobes.h
@@ -152,7 +152,6 @@ extern spinlock_t kretprobe_lock;
 extern int arch_prepare_kprobe(struct kprobe *p);
 extern void arch_arm_kprobe(struct kprobe *p);
 extern void arch_disarm_kprobe(struct kprobe *p);
-extern void arch_remove_kprobe(struct kprobe *p);
 extern int arch_init_kprobes(void);
 extern void show_registers(struct pt_regs *regs);
 extern kprobe_opcode_t *get_insn_slot(void);
Index: linux-2.6.15-rc5-mm2/kernel/kprobes.c
===================================================================
--- linux-2.6.15-rc5-mm2.orig/kernel/kprobes.c
+++ linux-2.6.15-rc5-mm2/kernel/kprobes.c
@@ -528,9 +528,7 @@ void __kprobes unregister_kprobe(struct 
 			list_del_rcu(&p->list);
 			kfree(old_p);
 		}
-		down(&kprobe_mutex);
-		arch_remove_kprobe(p);
-		up(&kprobe_mutex);
+		arch_remove_kprobe(p, &kprobe_mutex);
 	}
 }
 

--


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [patch 3/4] Kprobes - Changed from using spinlock to mutext
  2005-12-13 20:35 ` [patch 3/4] Kprobes - Changed from using spinlock to mutext Anil S Keshavamurthy
@ 2005-12-14 23:50   ` Keshavamurthy Anil S
  0 siblings, 0 replies; 6+ messages in thread
From: Keshavamurthy Anil S @ 2005-12-14 23:50 UTC (permalink / raw)
  To: akpm; +Cc: ananth, akpm, paulmck, linux-kernel, prasanna, systemtap, oleg

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);
 		}


	

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2005-12-14 23:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-12-13 20:35 [patch 0/4] Kprobes cleanup patches Anil S Keshavamurthy
2005-12-13 20:35 ` [patch 1/4] Kprobes - Enable funcions only for required arch Anil S Keshavamurthy
2005-12-13 20:35 ` [patch 2/4] Kprobes - cleanup include_asm_kprobes_h Anil S Keshavamurthy
2005-12-13 20:35 ` [patch 3/4] Kprobes - Changed from using spinlock to mutext Anil S Keshavamurthy
2005-12-14 23:50   ` Keshavamurthy Anil S
2005-12-13 20:35 ` [patch 4/4] Kprobes Cleanup arch_remove_kprobe Anil S Keshavamurthy

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 NNTP newsgroup(s).