[PATCH 0/9] Kprobes: scalability enhancements

[PATCH 0/9] Kprobes: scalability enhancements

[Ananth N Mavinakayanahalli]

Hi,

The following set of patches are aimed at improving kprobes scalability.
We currently serialize kprobe registration, unregistration and handler
execution using a single spinlock - kprobe_lock.

With these changes, kprobe handlers can run without any locks held.
It also allows for simultaneous kprobe handler executions on different
processors as we now track kprobe execution on a per processor basis.
It is now necessary that the handlers be re-entrant since handlers can
run concurrently on multiple processors.

All changes have been tested on i386, ia64, ppc64 and x86_64, while
sparc64 has been compile tested only.

The patches can be viewed as 3 logical chunks:

patch 1: 	Reorder preempt_(dis/en)able calls
patches 2-7: 	Introduce per_cpu data areas to track kprobe execution
patches 8-9: 	Use RCU to synchronize kprobe (un)registration and handler
		execution.

Thanks to Maneesh Soni, James Keniston and Anil Keshavamurthy for their
review and suggestions. Thanks again to Anil, Hien Nguyen and Kevin Stafford
for testing the patches.

Ananth

[PATCH 1/9] Kprobes: rearrange preempt_disable/enable() calls

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

Reorder preempt_disable/enable() calls in arch kprobes files in
preparation to introduce locking changes. No functional changes
introduced by this patch.

Signed-off-by: Ananth N Mavinakayahanalli <ananth@in.ibm.com>
Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
---

 arch/i386/kernel/kprobes.c    |   27 ++++++++++++++-------------
 arch/ia64/kernel/kprobes.c    |   22 ++++++++++++++--------
 arch/ppc64/kernel/kprobes.c   |   11 ++++++-----
 arch/sparc64/kernel/kprobes.c |   25 +++++++++++++------------
 arch/x86_64/kernel/kprobes.c  |   28 ++++++++++++++--------------
 5 files changed, 61 insertions(+), 52 deletions(-)

Index: linux-2.6.14-rc3/arch/i386/kernel/kprobes.c
===================================================================
--- linux-2.6.14-rc3.orig/arch/i386/kernel/kprobes.c	2005-09-30 17:17:35.000000000 -0400
+++ linux-2.6.14-rc3/arch/i386/kernel/kprobes.c	2005-10-05 16:07:48.000000000 -0400
@@ -158,8 +158,6 @@ static int __kprobes kprobe_handler(stru
 	kprobe_opcode_t *addr = NULL;
 	unsigned long *lp;
 
-	/* We're in an interrupt, but this is clear and BUG()-safe. */
-	preempt_disable();
 	/* Check if the application is using LDT entry for its code segment and
 	 * calculate the address by reading the base address from the LDT entry.
 	 */
@@ -232,6 +230,11 @@ static int __kprobes kprobe_handler(stru
 		goto no_kprobe;
 	}
 
+	/*
+	 * This preempt_disable() matches the preempt_enable_no_resched()
+	 * in post_kprobe_handler()
+	 */
+	preempt_disable();
 	kprobe_status = KPROBE_HIT_ACTIVE;
 	set_current_kprobe(p, regs);
 
@@ -245,7 +248,6 @@ ss_probe:
 	return 1;
 
 no_kprobe:
-	preempt_enable_no_resched();
 	return ret;
 }
 
@@ -316,7 +318,7 @@ int __kprobes trampoline_probe_handler(s
         /*
          * By returning a non-zero value, we are telling
          * kprobe_handler() that we have handled unlocking
-         * and re-enabling preemption.
+	 * and re-enabling preemption
          */
         return 1;
 }
@@ -453,29 +455,29 @@ int __kprobes kprobe_exceptions_notify(s
 				       unsigned long val, void *data)
 {
 	struct die_args *args = (struct die_args *)data;
+	int ret = NOTIFY_DONE;
+
+	preempt_disable();
 	switch (val) {
 	case DIE_INT3:
 		if (kprobe_handler(args->regs))
-			return NOTIFY_STOP;
+			ret = NOTIFY_STOP;
 		break;
 	case DIE_DEBUG:
 		if (post_kprobe_handler(args->regs))
-			return NOTIFY_STOP;
+			ret = NOTIFY_STOP;
 		break;
 	case DIE_GPF:
-		if (kprobe_running() &&
-		    kprobe_fault_handler(args->regs, args->trapnr))
-			return NOTIFY_STOP;
-		break;
 	case DIE_PAGE_FAULT:
 		if (kprobe_running() &&
 		    kprobe_fault_handler(args->regs, args->trapnr))
-			return NOTIFY_STOP;
+			ret = NOTIFY_STOP;
 		break;
 	default:
 		break;
 	}
-	return NOTIFY_DONE;
+	preempt_enable();
+	return ret;
 }
 
 int __kprobes setjmp_pre_handler(struct kprobe *p, struct pt_regs *regs)
@@ -502,7 +504,6 @@ int __kprobes setjmp_pre_handler(struct 
 
 void __kprobes jprobe_return(void)
 {
-	preempt_enable_no_resched();
 	asm volatile ("       xchgl   %%ebx,%%esp     \n"
 		      "       int3			\n"
 		      "       .globl jprobe_return_end	\n"
Index: linux-2.6.14-rc3/arch/ia64/kernel/kprobes.c
===================================================================
--- linux-2.6.14-rc3.orig/arch/ia64/kernel/kprobes.c	2005-09-30 17:17:35.000000000 -0400
+++ linux-2.6.14-rc3/arch/ia64/kernel/kprobes.c	2005-10-05 16:07:47.000000000 -0400
@@ -395,7 +395,7 @@ int __kprobes trampoline_probe_handler(s
         /*
          * By returning a non-zero value, we are telling
          * kprobe_handler() that we have handled unlocking
-         * and re-enabling preemption.
+	 * and re-enabling preemption
          */
         return 1;
 }
@@ -607,8 +607,6 @@ static int __kprobes pre_kprobes_handler
 	struct pt_regs *regs = args->regs;
 	kprobe_opcode_t *addr = (kprobe_opcode_t *)instruction_pointer(regs);
 
-	preempt_disable();
-
 	/* Handle recursion cases */
 	if (kprobe_running()) {
 		p = get_kprobe(addr);
@@ -665,6 +663,11 @@ static int __kprobes pre_kprobes_handler
 		goto no_kprobe;
 	}
 
+	/*
+	 * This preempt_disable() matches the preempt_enable_no_resched()
+	 * in post_kprobes_handler()
+	 */
+	preempt_disable();
 	kprobe_status = KPROBE_HIT_ACTIVE;
 	set_current_kprobe(p);
 
@@ -682,7 +685,6 @@ ss_probe:
 	return 1;
 
 no_kprobe:
-	preempt_enable_no_resched();
 	return ret;
 }
 
@@ -733,22 +735,26 @@ int __kprobes kprobe_exceptions_notify(s
 				       unsigned long val, void *data)
 {
 	struct die_args *args = (struct die_args *)data;
+	int ret = NOTIFY_DONE;
+
+	preempt_disable();
 	switch(val) {
 	case DIE_BREAK:
 		if (pre_kprobes_handler(args))
-			return NOTIFY_STOP;
+			ret = NOTIFY_STOP;
 		break;
 	case DIE_SS:
 		if (post_kprobes_handler(args->regs))
-			return NOTIFY_STOP;
+			ret = NOTIFY_STOP;
 		break;
 	case DIE_PAGE_FAULT:
 		if (kprobes_fault_handler(args->regs, args->trapnr))
-			return NOTIFY_STOP;
+			ret = NOTIFY_STOP;
 	default:
 		break;
 	}
-	return NOTIFY_DONE;
+	preempt_enable();
+	return ret;
 }
 
 int __kprobes setjmp_pre_handler(struct kprobe *p, struct pt_regs *regs)
Index: linux-2.6.14-rc3/arch/ppc64/kernel/kprobes.c
===================================================================
--- linux-2.6.14-rc3.orig/arch/ppc64/kernel/kprobes.c	2005-09-30 17:17:35.000000000 -0400
+++ linux-2.6.14-rc3/arch/ppc64/kernel/kprobes.c	2005-10-05 16:07:45.000000000 -0400
@@ -209,6 +209,11 @@ static inline int kprobe_handler(struct 
 		goto no_kprobe;
 	}
 
+	/*
+	 * This preempt_disable() matches the preempt_enable_no_resched()
+	 * in post_kprobe_handler().
+	 */
+	preempt_disable();
 	kprobe_status = KPROBE_HIT_ACTIVE;
 	current_kprobe = p;
 	kprobe_saved_msr = regs->msr;
@@ -219,11 +224,6 @@ static inline int kprobe_handler(struct 
 ss_probe:
 	prepare_singlestep(p, regs);
 	kprobe_status = KPROBE_HIT_SS;
-	/*
-	 * This preempt_disable() matches the preempt_enable_no_resched()
-	 * in post_kprobe_handler().
-	 */
-	preempt_disable();
 	return 1;
 
 no_kprobe:
@@ -293,6 +293,7 @@ int __kprobes trampoline_probe_handler(s
 	regs->nip = orig_ret_address;
 
 	unlock_kprobes();
+	preempt_enable_no_resched();
 
         /*
          * By returning a non-zero value, we are telling
Index: linux-2.6.14-rc3/arch/sparc64/kernel/kprobes.c
===================================================================
--- linux-2.6.14-rc3.orig/arch/sparc64/kernel/kprobes.c	2005-09-30 17:17:35.000000000 -0400
+++ linux-2.6.14-rc3/arch/sparc64/kernel/kprobes.c	2005-10-05 16:07:44.000000000 -0400
@@ -118,8 +118,6 @@ static int __kprobes kprobe_handler(stru
 	void *addr = (void *) regs->tpc;
 	int ret = 0;
 
-	preempt_disable();
-
 	if (kprobe_running()) {
 		/* We *are* holding lock here, so this is safe.
 		 * Disarm the probe we just hit, and ignore it.
@@ -171,6 +169,11 @@ static int __kprobes kprobe_handler(stru
 		goto no_kprobe;
 	}
 
+	/*
+	 * This preempt_disable() matches the preempt_enable_no_resched()
+	 * in post_kprobes_handler()
+	 */
+	preempt_disable();
 	set_current_kprobe(p, regs);
 	kprobe_status = KPROBE_HIT_ACTIVE;
 	if (p->pre_handler && p->pre_handler(p, regs))
@@ -182,7 +185,6 @@ ss_probe:
 	return 1;
 
 no_kprobe:
-	preempt_enable_no_resched();
 	return ret;
 }
 
@@ -322,29 +324,29 @@ int __kprobes kprobe_exceptions_notify(s
 				       unsigned long val, void *data)
 {
 	struct die_args *args = (struct die_args *)data;
+	int ret = NOTIFY_DONE;
+
+	preempt_disable();
 	switch (val) {
 	case DIE_DEBUG:
 		if (kprobe_handler(args->regs))
-			return NOTIFY_STOP;
+			ret = NOTIFY_STOP;
 		break;
 	case DIE_DEBUG_2:
 		if (post_kprobe_handler(args->regs))
-			return NOTIFY_STOP;
+			ret = NOTIFY_STOP;
 		break;
 	case DIE_GPF:
-		if (kprobe_running() &&
-		    kprobe_fault_handler(args->regs, args->trapnr))
-			return NOTIFY_STOP;
-		break;
 	case DIE_PAGE_FAULT:
 		if (kprobe_running() &&
 		    kprobe_fault_handler(args->regs, args->trapnr))
-			return NOTIFY_STOP;
+			ret = NOTIFY_STOP;
 		break;
 	default:
 		break;
 	}
-	return NOTIFY_DONE;
+	preempt_enable();
+	return ret;
 }
 
 asmlinkage void __kprobes kprobe_trap(unsigned long trap_level,
@@ -396,7 +398,6 @@ int __kprobes setjmp_pre_handler(struct 
 
 void __kprobes jprobe_return(void)
 {
-	preempt_enable_no_resched();
 	__asm__ __volatile__(
 		".globl	jprobe_return_trap_instruction\n"
 "jprobe_return_trap_instruction:\n\t"
Index: linux-2.6.14-rc3/arch/x86_64/kernel/kprobes.c
===================================================================
--- linux-2.6.14-rc3.orig/arch/x86_64/kernel/kprobes.c	2005-09-30 17:17:35.000000000 -0400
+++ linux-2.6.14-rc3/arch/x86_64/kernel/kprobes.c	2005-10-05 16:08:04.000000000 -0400
@@ -302,9 +302,6 @@ int __kprobes kprobe_handler(struct pt_r
 	int ret = 0;
 	kprobe_opcode_t *addr = (kprobe_opcode_t *)(regs->rip - sizeof(kprobe_opcode_t));
 
-	/* We're in an interrupt, but this is clear and BUG()-safe. */
-	preempt_disable();
-
 	/* Check we're not actually recursing */
 	if (kprobe_running()) {
 		/* We *are* holding lock here, so this is safe.
@@ -372,6 +369,11 @@ int __kprobes kprobe_handler(struct pt_r
 		goto no_kprobe;
 	}
 
+	/*
+	 * This preempt_disable() matches the preempt_enable_no_resched()
+	 * in post_kprobe_handler()
+	 */
+	preempt_disable();
 	kprobe_status = KPROBE_HIT_ACTIVE;
 	set_current_kprobe(p, regs);
 
@@ -385,7 +387,6 @@ ss_probe:
 	return 1;
 
 no_kprobe:
-	preempt_enable_no_resched();
 	return ret;
 }
 
@@ -456,7 +457,7 @@ int __kprobes trampoline_probe_handler(s
         /*
          * By returning a non-zero value, we are telling
          * kprobe_handler() that we have handled unlocking
-         * and re-enabling preemption.
+	 * and re-enabling preemption
          */
         return 1;
 }
@@ -599,29 +600,29 @@ int __kprobes kprobe_exceptions_notify(s
 				       unsigned long val, void *data)
 {
 	struct die_args *args = (struct die_args *)data;
+	int ret = NOTIFY_DONE;
+
+	preempt_disable();
 	switch (val) {
 	case DIE_INT3:
 		if (kprobe_handler(args->regs))
-			return NOTIFY_STOP;
+			ret = NOTIFY_STOP;
 		break;
 	case DIE_DEBUG:
 		if (post_kprobe_handler(args->regs))
-			return NOTIFY_STOP;
+			ret = NOTIFY_STOP;
 		break;
 	case DIE_GPF:
-		if (kprobe_running() &&
-		    kprobe_fault_handler(args->regs, args->trapnr))
-			return NOTIFY_STOP;
-		break;
 	case DIE_PAGE_FAULT:
 		if (kprobe_running() &&
 		    kprobe_fault_handler(args->regs, args->trapnr))
-			return NOTIFY_STOP;
+			ret = NOTIFY_STOP;
 		break;
 	default:
 		break;
 	}
-	return NOTIFY_DONE;
+	preempt_enable();
+	return ret;
 }
 
 int __kprobes setjmp_pre_handler(struct kprobe *p, struct pt_regs *regs)
@@ -647,7 +648,6 @@ int __kprobes setjmp_pre_handler(struct 
 
 void __kprobes jprobe_return(void)
 {
-	preempt_enable_no_resched();
 	asm volatile ("       xchg   %%rbx,%%rsp     \n"
 		      "       int3			\n"
 		      "       .globl jprobe_return_end	\n"

[PATCH 2/9] Kprobes: Track kprobe on a per_cpu basis - base changes

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

Changes to the base kprobe infrastructure to track kprobe execution on a
per-cpu basis.

Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
---

 include/linux/kprobes.h |   31 ++++++++++++++++++++++---------
 kernel/kprobes.c        |   43 ++++++++++++++++++++++++++++---------------
 2 files changed, 50 insertions(+), 24 deletions(-)

Index: linux-2.6.14-rc3/include/linux/kprobes.h
===================================================================
--- linux-2.6.14-rc3.orig/include/linux/kprobes.h	2005-09-30 17:17:35.000000000 -0400
+++ linux-2.6.14-rc3/include/linux/kprobes.h	2005-10-05 15:25:39.000000000 -0400
@@ -33,6 +33,7 @@
 #include <linux/list.h>
 #include <linux/notifier.h>
 #include <linux/smp.h>
+#include <linux/percpu.h>
 
 #include <asm/kprobes.h>
 
@@ -106,6 +107,9 @@ struct jprobe {
 	kprobe_opcode_t *entry;	/* probe handling code to jump to */
 };
 
+DECLARE_PER_CPU(struct kprobe *, current_kprobe);
+DECLARE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
+
 #ifdef ARCH_SUPPORTS_KRETPROBES
 extern void arch_prepare_kretprobe(struct kretprobe *rp, struct pt_regs *regs);
 #else /* ARCH_SUPPORTS_KRETPROBES */
@@ -146,13 +150,6 @@ struct kretprobe_instance {
 void lock_kprobes(void);
 void unlock_kprobes(void);
 
-/* kprobe running now on this CPU? */
-static inline int kprobe_running(void)
-{
-	extern unsigned int kprobe_cpu;
-	return kprobe_cpu == smp_processor_id();
-}
-
 extern int arch_prepare_kprobe(struct kprobe *p);
 extern void arch_copy_kprobe(struct kprobe *p);
 extern void arch_arm_kprobe(struct kprobe *p);
@@ -167,6 +164,22 @@ extern void free_insn_slot(kprobe_opcode
 struct kprobe *get_kprobe(void *addr);
 struct hlist_head * kretprobe_inst_table_head(struct task_struct *tsk);
 
+/* kprobe_running() will just return the current_kprobe on this CPU */
+static inline struct kprobe *kprobe_running(void)
+{
+	return (__get_cpu_var(current_kprobe));
+}
+
+static inline void reset_current_kprobe(void)
+{
+	__get_cpu_var(current_kprobe) = NULL;
+}
+
+static inline struct kprobe_ctlblk *get_kprobe_ctlblk(void)
+{
+	return (&__get_cpu_var(kprobe_ctlblk));
+}
+
 int register_kprobe(struct kprobe *p);
 void unregister_kprobe(struct kprobe *p);
 int setjmp_pre_handler(struct kprobe *, struct pt_regs *);
@@ -183,9 +196,9 @@ void add_rp_inst(struct kretprobe_instan
 void kprobe_flush_task(struct task_struct *tk);
 void recycle_rp_inst(struct kretprobe_instance *ri);
 #else /* CONFIG_KPROBES */
-static inline int kprobe_running(void)
+static inline struct kprobe *kprobe_running(void)
 {
-	return 0;
+	return NULL;
 }
 static inline int register_kprobe(struct kprobe *p)
 {
Index: linux-2.6.14-rc3/kernel/kprobes.c
===================================================================
--- linux-2.6.14-rc3.orig/kernel/kprobes.c	2005-09-30 17:17:35.000000000 -0400
+++ linux-2.6.14-rc3/kernel/kprobes.c	2005-10-05 15:25:39.000000000 -0400
@@ -50,7 +50,7 @@ static struct hlist_head kretprobe_inst_
 
 unsigned int kprobe_cpu = NR_CPUS;
 static DEFINE_SPINLOCK(kprobe_lock);
-static struct kprobe *curr_kprobe;
+static DEFINE_PER_CPU(struct kprobe *, kprobe_instance) = NULL;
 
 /*
  * kprobe->ainsn.insn points to the copy of the instruction to be
@@ -187,6 +187,17 @@ void __kprobes unlock_kprobes(void)
  	local_irq_restore(flags);
 }
 
+/* We have preemption disabled.. so it is safe to use __ versions */
+static inline void set_kprobe_instance(struct kprobe *kp)
+{
+	__get_cpu_var(kprobe_instance) = kp;
+}
+
+static inline void reset_kprobe_instance(void)
+{
+	__get_cpu_var(kprobe_instance) = NULL;
+}
+
 /* You have to be holding the kprobe_lock */
 struct kprobe __kprobes *get_kprobe(void *addr)
 {
@@ -212,11 +223,11 @@ static int __kprobes aggr_pre_handler(st
 
 	list_for_each_entry(kp, &p->list, list) {
 		if (kp->pre_handler) {
-			curr_kprobe = kp;
+			set_kprobe_instance(kp);
 			if (kp->pre_handler(kp, regs))
 				return 1;
 		}
-		curr_kprobe = NULL;
+		reset_kprobe_instance();
 	}
 	return 0;
 }
@@ -228,9 +239,9 @@ static void __kprobes aggr_post_handler(
 
 	list_for_each_entry(kp, &p->list, list) {
 		if (kp->post_handler) {
-			curr_kprobe = kp;
+			set_kprobe_instance(kp);
 			kp->post_handler(kp, regs, flags);
-			curr_kprobe = NULL;
+			reset_kprobe_instance();
 		}
 	}
 	return;
@@ -239,12 +250,14 @@ static void __kprobes aggr_post_handler(
 static int __kprobes aggr_fault_handler(struct kprobe *p, struct pt_regs *regs,
 					int trapnr)
 {
+	struct kprobe *cur = __get_cpu_var(kprobe_instance);
+
 	/*
 	 * if we faulted "during" the execution of a user specified
 	 * probe handler, invoke just that probe's fault handler
 	 */
-	if (curr_kprobe && curr_kprobe->fault_handler) {
-		if (curr_kprobe->fault_handler(curr_kprobe, regs, trapnr))
+	if (cur && cur->fault_handler) {
+		if (cur->fault_handler(cur, regs, trapnr))
 			return 1;
 	}
 	return 0;
@@ -252,15 +265,15 @@ static int __kprobes aggr_fault_handler(
 
 static int __kprobes aggr_break_handler(struct kprobe *p, struct pt_regs *regs)
 {
-	struct kprobe *kp = curr_kprobe;
-	if (curr_kprobe && kp->break_handler) {
-		if (kp->break_handler(kp, regs)) {
-			curr_kprobe = NULL;
-			return 1;
-		}
+	struct kprobe *cur = __get_cpu_var(kprobe_instance);
+	int ret = 0;
+
+	if (cur && cur->break_handler) {
+		if (cur->break_handler(cur, regs))
+			ret = 1;
 	}
-	curr_kprobe = NULL;
-	return 0;
+	reset_kprobe_instance();
+	return ret;
 }
 
 struct kretprobe_instance __kprobes *get_free_rp_inst(struct kretprobe *rp)

[PATCH 3/9] Kprobes: Track kprobe on a per_cpu basis - i386 changes

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

I386 changes to track kprobe execution on a per-cpu basis. We now track
the kprobe state machine independently on each cpu, using an arch specific
kprobe control block.

Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
---

 arch/i386/kernel/kprobes.c |  126 ++++++++++++++++++++++++---------------------
 include/asm-i386/kprobes.h |   17 ++++++
 2 files changed, 86 insertions(+), 57 deletions(-)

Index: linux-2.6.14-rc3/arch/i386/kernel/kprobes.c
===================================================================
--- linux-2.6.14-rc3.orig/arch/i386/kernel/kprobes.c	2005-10-05 15:22:37.000000000 -0400
+++ linux-2.6.14-rc3/arch/i386/kernel/kprobes.c	2005-10-05 15:27:19.000000000 -0400
@@ -37,16 +37,11 @@
 #include <asm/kdebug.h>
 #include <asm/desc.h>
 
-static struct kprobe *current_kprobe;
-static unsigned long kprobe_status, kprobe_old_eflags, kprobe_saved_eflags;
-static struct kprobe *kprobe_prev;
-static unsigned long kprobe_status_prev, kprobe_old_eflags_prev, kprobe_saved_eflags_prev;
-static struct pt_regs jprobe_saved_regs;
-static long *jprobe_saved_esp;
-/* copy of the kernel stack at the probe fire time */
-static kprobe_opcode_t jprobes_stack[MAX_STACK_SIZE];
 void jprobe_return_end(void);
 
+DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
+DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
+
 /*
  * returns non-zero if opcode modifies the interrupt flag.
  */
@@ -91,29 +86,30 @@ void __kprobes arch_remove_kprobe(struct
 {
 }
 
-static inline void save_previous_kprobe(void)
+static inline void save_previous_kprobe(struct kprobe_ctlblk *kcb)
 {
-	kprobe_prev = current_kprobe;
-	kprobe_status_prev = kprobe_status;
-	kprobe_old_eflags_prev = kprobe_old_eflags;
-	kprobe_saved_eflags_prev = kprobe_saved_eflags;
+	kcb->prev_kprobe.kp = kprobe_running();
+	kcb->prev_kprobe.status = kcb->kprobe_status;
+	kcb->prev_kprobe.old_eflags = kcb->kprobe_old_eflags;
+	kcb->prev_kprobe.saved_eflags = kcb->kprobe_saved_eflags;
 }
 
-static inline void restore_previous_kprobe(void)
+static inline void restore_previous_kprobe(struct kprobe_ctlblk *kcb)
 {
-	current_kprobe = kprobe_prev;
-	kprobe_status = kprobe_status_prev;
-	kprobe_old_eflags = kprobe_old_eflags_prev;
-	kprobe_saved_eflags = kprobe_saved_eflags_prev;
+	__get_cpu_var(current_kprobe) = kcb->prev_kprobe.kp;
+	kcb->kprobe_status = kcb->prev_kprobe.status;
+	kcb->kprobe_old_eflags = kcb->prev_kprobe.old_eflags;
+	kcb->kprobe_saved_eflags = kcb->prev_kprobe.saved_eflags;
 }
 
-static inline void set_current_kprobe(struct kprobe *p, struct pt_regs *regs)
+static inline void set_current_kprobe(struct kprobe *p, struct pt_regs *regs,
+				struct kprobe_ctlblk *kcb)
 {
-	current_kprobe = p;
-	kprobe_saved_eflags = kprobe_old_eflags
+	__get_cpu_var(current_kprobe) = p;
+	kcb->kprobe_saved_eflags = kcb->kprobe_old_eflags
 		= (regs->eflags & (TF_MASK | IF_MASK));
 	if (is_IF_modifier(p->opcode))
-		kprobe_saved_eflags &= ~IF_MASK;
+		kcb->kprobe_saved_eflags &= ~IF_MASK;
 }
 
 static inline void prepare_singlestep(struct kprobe *p, struct pt_regs *regs)
@@ -157,6 +153,7 @@ static int __kprobes kprobe_handler(stru
 	int ret = 0;
 	kprobe_opcode_t *addr = NULL;
 	unsigned long *lp;
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 
 	/* Check if the application is using LDT entry for its code segment and
 	 * calculate the address by reading the base address from the LDT entry.
@@ -175,10 +172,10 @@ static int __kprobes kprobe_handler(stru
 		   Disarm the probe we just hit, and ignore it. */
 		p = get_kprobe(addr);
 		if (p) {
-			if (kprobe_status == KPROBE_HIT_SS &&
+			if (kcb->kprobe_status == KPROBE_HIT_SS &&
 				*p->ainsn.insn == BREAKPOINT_INSTRUCTION) {
 				regs->eflags &= ~TF_MASK;
-				regs->eflags |= kprobe_saved_eflags;
+				regs->eflags |= kcb->kprobe_saved_eflags;
 				unlock_kprobes();
 				goto no_kprobe;
 			}
@@ -188,14 +185,14 @@ static int __kprobes kprobe_handler(stru
 			 * just single step on the instruction of the new probe
 			 * without calling any user handlers.
 			 */
-			save_previous_kprobe();
-			set_current_kprobe(p, regs);
+			save_previous_kprobe(kcb);
+			set_current_kprobe(p, regs, kcb);
 			p->nmissed++;
 			prepare_singlestep(p, regs);
-			kprobe_status = KPROBE_REENTER;
+			kcb->kprobe_status = KPROBE_REENTER;
 			return 1;
 		} else {
-			p = current_kprobe;
+			p = __get_cpu_var(current_kprobe);
 			if (p->break_handler && p->break_handler(p, regs)) {
 				goto ss_probe;
 			}
@@ -235,8 +232,8 @@ static int __kprobes kprobe_handler(stru
 	 * in post_kprobe_handler()
 	 */
 	preempt_disable();
-	kprobe_status = KPROBE_HIT_ACTIVE;
-	set_current_kprobe(p, regs);
+	set_current_kprobe(p, regs, kcb);
+	kcb->kprobe_status = KPROBE_HIT_ACTIVE;
 
 	if (p->pre_handler && p->pre_handler(p, regs))
 		/* handler has already set things up, so skip ss setup */
@@ -244,7 +241,7 @@ static int __kprobes kprobe_handler(stru
 
 ss_probe:
 	prepare_singlestep(p, regs);
-	kprobe_status = KPROBE_HIT_SS;
+	kcb->kprobe_status = KPROBE_HIT_SS;
 	return 1;
 
 no_kprobe:
@@ -312,6 +309,7 @@ int __kprobes trampoline_probe_handler(s
 	BUG_ON(!orig_ret_address || (orig_ret_address == trampoline_address));
 	regs->eip = orig_ret_address;
 
+	reset_current_kprobe();
 	unlock_kprobes();
 	preempt_enable_no_resched();
 
@@ -345,7 +343,8 @@ int __kprobes trampoline_probe_handler(s
  * that is atop the stack is the address following the copied instruction.
  * We need to make it the address following the original instruction.
  */
-static void __kprobes resume_execution(struct kprobe *p, struct pt_regs *regs)
+static void __kprobes resume_execution(struct kprobe *p,
+		struct pt_regs *regs, struct kprobe_ctlblk *kcb)
 {
 	unsigned long *tos = (unsigned long *)&regs->esp;
 	unsigned long next_eip = 0;
@@ -355,7 +354,7 @@ static void __kprobes resume_execution(s
 	switch (p->ainsn.insn[0]) {
 	case 0x9c:		/* pushfl */
 		*tos &= ~(TF_MASK | IF_MASK);
-		*tos |= kprobe_old_eflags;
+		*tos |= kcb->kprobe_old_eflags;
 		break;
 	case 0xc3:		/* ret/lret */
 	case 0xcb:
@@ -400,22 +399,26 @@ static void __kprobes resume_execution(s
  */
 static inline int post_kprobe_handler(struct pt_regs *regs)
 {
-	if (!kprobe_running())
+	struct kprobe *cur = kprobe_running();
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+
+	if (!cur)
 		return 0;
 
-	if ((kprobe_status != KPROBE_REENTER) && current_kprobe->post_handler) {
-		kprobe_status = KPROBE_HIT_SSDONE;
-		current_kprobe->post_handler(current_kprobe, regs, 0);
+	if ((kcb->kprobe_status != KPROBE_REENTER) && cur->post_handler) {
+		kcb->kprobe_status = KPROBE_HIT_SSDONE;
+		cur->post_handler(cur, regs, 0);
 	}
 
-	resume_execution(current_kprobe, regs);
-	regs->eflags |= kprobe_saved_eflags;
+	resume_execution(cur, regs, kcb);
+	regs->eflags |= kcb->kprobe_saved_eflags;
 
 	/*Restore back the original saved kprobes variables and continue. */
-	if (kprobe_status == KPROBE_REENTER) {
-		restore_previous_kprobe();
+	if (kcb->kprobe_status == KPROBE_REENTER) {
+		restore_previous_kprobe(kcb);
 		goto out;
 	}
+	reset_current_kprobe();
 	unlock_kprobes();
 out:
 	preempt_enable_no_resched();
@@ -434,14 +437,17 @@ out:
 /* Interrupts disabled, kprobe_lock held. */
 static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
 {
-	if (current_kprobe->fault_handler
-	    && current_kprobe->fault_handler(current_kprobe, regs, trapnr))
+	struct kprobe *cur = kprobe_running();
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+
+	if (cur->fault_handler && cur->fault_handler(cur, regs, trapnr))
 		return 1;
 
-	if (kprobe_status & KPROBE_HIT_SS) {
-		resume_execution(current_kprobe, regs);
-		regs->eflags |= kprobe_old_eflags;
+	if (kcb->kprobe_status & KPROBE_HIT_SS) {
+		resume_execution(cur, regs, kcb);
+		regs->eflags |= kcb->kprobe_old_eflags;
 
+		reset_current_kprobe();
 		unlock_kprobes();
 		preempt_enable_no_resched();
 	}
@@ -484,10 +490,11 @@ int __kprobes setjmp_pre_handler(struct 
 {
 	struct jprobe *jp = container_of(p, struct jprobe, kp);
 	unsigned long addr;
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 
-	jprobe_saved_regs = *regs;
-	jprobe_saved_esp = &regs->esp;
-	addr = (unsigned long)jprobe_saved_esp;
+	kcb->jprobe_saved_regs = *regs;
+	kcb->jprobe_saved_esp = &regs->esp;
+	addr = (unsigned long)(kcb->jprobe_saved_esp);
 
 	/*
 	 * TBD: As Linus pointed out, gcc assumes that the callee
@@ -496,7 +503,8 @@ int __kprobes setjmp_pre_handler(struct 
 	 * we also save and restore enough stack bytes to cover
 	 * the argument area.
 	 */
-	memcpy(jprobes_stack, (kprobe_opcode_t *) addr, MIN_STACK_SIZE(addr));
+	memcpy(kcb->jprobes_stack, (kprobe_opcode_t *)addr,
+			MIN_STACK_SIZE(addr));
 	regs->eflags &= ~IF_MASK;
 	regs->eip = (unsigned long)(jp->entry);
 	return 1;
@@ -504,34 +512,38 @@ int __kprobes setjmp_pre_handler(struct 
 
 void __kprobes jprobe_return(void)
 {
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+
 	asm volatile ("       xchgl   %%ebx,%%esp     \n"
 		      "       int3			\n"
 		      "       .globl jprobe_return_end	\n"
 		      "       jprobe_return_end:	\n"
 		      "       nop			\n"::"b"
-		      (jprobe_saved_esp):"memory");
+		      (kcb->jprobe_saved_esp):"memory");
 }
 
 int __kprobes longjmp_break_handler(struct kprobe *p, struct pt_regs *regs)
 {
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 	u8 *addr = (u8 *) (regs->eip - 1);
-	unsigned long stack_addr = (unsigned long)jprobe_saved_esp;
+	unsigned long stack_addr = (unsigned long)(kcb->jprobe_saved_esp);
 	struct jprobe *jp = container_of(p, struct jprobe, kp);
 
 	if ((addr > (u8 *) jprobe_return) && (addr < (u8 *) jprobe_return_end)) {
-		if (&regs->esp != jprobe_saved_esp) {
+		if (&regs->esp != kcb->jprobe_saved_esp) {
 			struct pt_regs *saved_regs =
-			    container_of(jprobe_saved_esp, struct pt_regs, esp);
+			    container_of(kcb->jprobe_saved_esp,
+					    struct pt_regs, esp);
 			printk("current esp %p does not match saved esp %p\n",
-			       &regs->esp, jprobe_saved_esp);
+			       &regs->esp, kcb->jprobe_saved_esp);
 			printk("Saved registers for jprobe %p\n", jp);
 			show_registers(saved_regs);
 			printk("Current registers\n");
 			show_registers(regs);
 			BUG();
 		}
-		*regs = jprobe_saved_regs;
-		memcpy((kprobe_opcode_t *) stack_addr, jprobes_stack,
+		*regs = kcb->jprobe_saved_regs;
+		memcpy((kprobe_opcode_t *) stack_addr, kcb->jprobes_stack,
 		       MIN_STACK_SIZE(stack_addr));
 		return 1;
 	}
Index: linux-2.6.14-rc3/include/asm-i386/kprobes.h
===================================================================
--- linux-2.6.14-rc3.orig/include/asm-i386/kprobes.h	2005-09-30 17:17:35.000000000 -0400
+++ linux-2.6.14-rc3/include/asm-i386/kprobes.h	2005-10-05 15:26:29.000000000 -0400
@@ -49,6 +49,23 @@ struct arch_specific_insn {
 	kprobe_opcode_t insn[MAX_INSN_SIZE];
 };
 
+struct prev_kprobe {
+	struct kprobe *kp;
+	unsigned long status;
+	unsigned long old_eflags;
+	unsigned long saved_eflags;
+};
+
+/* per-cpu kprobe control block */
+struct kprobe_ctlblk {
+	unsigned long kprobe_status;
+	unsigned long kprobe_old_eflags;
+	unsigned long kprobe_saved_eflags;
+	long *jprobe_saved_esp;
+	struct pt_regs jprobe_saved_regs;
+	kprobe_opcode_t jprobes_stack[MAX_STACK_SIZE];
+	struct prev_kprobe prev_kprobe;
+};
 
 /* trap3/1 are intr gates for kprobes.  So, restore the status of IF,
  * if necessary, before executing the original int3/1 (trap) handler.

[PATCH 4/9] Kprobes: Track kprobe on a per_cpu basis - ia64 changes

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

IA64 changes to track kprobe execution on a per-cpu basis. We now track
the kprobe state machine independently on each cpu using an arch specific
kprobe control block.

Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
---

 arch/ia64/kernel/kprobes.c |   83 ++++++++++++++++++++++++---------------------
 include/asm-ia64/kprobes.h |   13 +++++++
 2 files changed, 58 insertions(+), 38 deletions(-)

Index: linux-2.6.14-rc3/arch/ia64/kernel/kprobes.c
===================================================================
--- linux-2.6.14-rc3.orig/arch/ia64/kernel/kprobes.c	2005-10-05 15:23:17.000000000 -0400
+++ linux-2.6.14-rc3/arch/ia64/kernel/kprobes.c	2005-10-05 15:29:28.000000000 -0400
@@ -38,13 +38,8 @@
 
 extern void jprobe_inst_return(void);
 
-/* kprobe_status settings */
-#define KPROBE_HIT_ACTIVE	0x00000001
-#define KPROBE_HIT_SS		0x00000002
-
-static struct kprobe *current_kprobe, *kprobe_prev;
-static unsigned long kprobe_status, kprobe_status_prev;
-static struct pt_regs jprobe_saved_regs;
+DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
+DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
 
 enum instruction_type {A, I, M, F, B, L, X, u};
 static enum instruction_type bundle_encoding[32][3] = {
@@ -313,21 +308,22 @@ static int __kprobes valid_kprobe_addr(i
 	return 0;
 }
 
-static inline void save_previous_kprobe(void)
+static inline void save_previous_kprobe(struct kprobe_ctlblk *kcb)
 {
-	kprobe_prev = current_kprobe;
-	kprobe_status_prev = kprobe_status;
+	kcb->prev_kprobe.kp = kprobe_running();
+	kcb->prev_kprobe.status = kcb->kprobe_status;
 }
 
-static inline void restore_previous_kprobe(void)
+static inline void restore_previous_kprobe(struct kprobe_ctlblk *kcb)
 {
-	current_kprobe = kprobe_prev;
-	kprobe_status = kprobe_status_prev;
+	__get_cpu_var(current_kprobe) = kcb->prev_kprobe.kp;
+	kcb->kprobe_status = kcb->prev_kprobe.status;
 }
 
-static inline void set_current_kprobe(struct kprobe *p)
+static inline void set_current_kprobe(struct kprobe *p,
+			struct kprobe_ctlblk *kcb)
 {
-	current_kprobe = p;
+	__get_cpu_var(current_kprobe) = p;
 }
 
 static void kretprobe_trampoline(void)
@@ -389,6 +385,7 @@ int __kprobes trampoline_probe_handler(s
 	BUG_ON(!orig_ret_address || (orig_ret_address == trampoline_address));
 	regs->cr_iip = orig_ret_address;
 
+	reset_current_kprobe();
 	unlock_kprobes();
 	preempt_enable_no_resched();
 
@@ -606,12 +603,13 @@ static int __kprobes pre_kprobes_handler
 	int ret = 0;
 	struct pt_regs *regs = args->regs;
 	kprobe_opcode_t *addr = (kprobe_opcode_t *)instruction_pointer(regs);
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 
 	/* Handle recursion cases */
 	if (kprobe_running()) {
 		p = get_kprobe(addr);
 		if (p) {
-			if ( (kprobe_status == KPROBE_HIT_SS) &&
+			if ((kcb->kprobe_status == KPROBE_HIT_SS) &&
 	 		     (p->ainsn.inst_flag == INST_FLAG_BREAK_INST)) {
   				ia64_psr(regs)->ss = 0;
 				unlock_kprobes();
@@ -623,17 +621,17 @@ static int __kprobes pre_kprobes_handler
 			 * just single step on the instruction of the new probe
 			 * without calling any user handlers.
 			 */
-			save_previous_kprobe();
-			set_current_kprobe(p);
+			save_previous_kprobe(kcb);
+			set_current_kprobe(p, kcb);
 			p->nmissed++;
 			prepare_ss(p, regs);
-			kprobe_status = KPROBE_REENTER;
+			kcb->kprobe_status = KPROBE_REENTER;
 			return 1;
 		} else if (args->err == __IA64_BREAK_JPROBE) {
 			/*
 			 * jprobe instrumented function just completed
 			 */
-			p = current_kprobe;
+			p = __get_cpu_var(current_kprobe);
 			if (p->break_handler && p->break_handler(p, regs)) {
 				goto ss_probe;
 			}
@@ -668,8 +666,8 @@ static int __kprobes pre_kprobes_handler
 	 * in post_kprobes_handler()
 	 */
 	preempt_disable();
-	kprobe_status = KPROBE_HIT_ACTIVE;
-	set_current_kprobe(p);
+	set_current_kprobe(p, kcb);
+	kcb->kprobe_status = KPROBE_HIT_ACTIVE;
 
 	if (p->pre_handler && p->pre_handler(p, regs))
 		/*
@@ -681,7 +679,7 @@ static int __kprobes pre_kprobes_handler
 
 ss_probe:
 	prepare_ss(p, regs);
-	kprobe_status = KPROBE_HIT_SS;
+	kcb->kprobe_status = KPROBE_HIT_SS;
 	return 1;
 
 no_kprobe:
@@ -690,22 +688,25 @@ no_kprobe:
 
 static int __kprobes post_kprobes_handler(struct pt_regs *regs)
 {
-	if (!kprobe_running())
+	struct kprobe *cur = kprobe_running();
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+
+	if (!cur)
 		return 0;
 
-	if ((kprobe_status != KPROBE_REENTER) && current_kprobe->post_handler) {
-		kprobe_status = KPROBE_HIT_SSDONE;
-		current_kprobe->post_handler(current_kprobe, regs, 0);
+	if ((kcb->kprobe_status != KPROBE_REENTER) && cur->post_handler) {
+		kcb->kprobe_status = KPROBE_HIT_SSDONE;
+		cur->post_handler(cur, regs, 0);
 	}
 
-	resume_execution(current_kprobe, regs);
+	resume_execution(cur, regs);
 
 	/*Restore back the original saved kprobes variables and continue. */
-	if (kprobe_status == KPROBE_REENTER) {
-		restore_previous_kprobe();
+	if (kcb->kprobe_status == KPROBE_REENTER) {
+		restore_previous_kprobe(kcb);
 		goto out;
 	}
-
+	reset_current_kprobe();
 	unlock_kprobes();
 
 out:
@@ -715,15 +716,18 @@ out:
 
 static int __kprobes kprobes_fault_handler(struct pt_regs *regs, int trapnr)
 {
-	if (!kprobe_running())
+	struct kprobe *cur = kprobe_running();
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+
+	if (!cur)
 		return 0;
 
-	if (current_kprobe->fault_handler &&
-	    current_kprobe->fault_handler(current_kprobe, regs, trapnr))
+	if (cur->fault_handler && cur->fault_handler(cur, regs, trapnr))
 		return 1;
 
-	if (kprobe_status & KPROBE_HIT_SS) {
-		resume_execution(current_kprobe, regs);
+	if (kcb->kprobe_status & KPROBE_HIT_SS) {
+		resume_execution(cur, regs);
+		reset_current_kprobe();
 		unlock_kprobes();
 		preempt_enable_no_resched();
 	}
@@ -761,9 +765,10 @@ int __kprobes setjmp_pre_handler(struct 
 {
 	struct jprobe *jp = container_of(p, struct jprobe, kp);
 	unsigned long addr = ((struct fnptr *)(jp->entry))->ip;
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 
 	/* save architectural state */
-	jprobe_saved_regs = *regs;
+	kcb->jprobe_saved_regs = *regs;
 
 	/* after rfi, execute the jprobe instrumented function */
 	regs->cr_iip = addr & ~0xFULL;
@@ -781,7 +786,9 @@ int __kprobes setjmp_pre_handler(struct 
 
 int __kprobes longjmp_break_handler(struct kprobe *p, struct pt_regs *regs)
 {
-	*regs = jprobe_saved_regs;
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+
+	*regs = kcb->jprobe_saved_regs;
 	return 1;
 }
 
Index: linux-2.6.14-rc3/include/asm-ia64/kprobes.h
===================================================================
--- linux-2.6.14-rc3.orig/include/asm-ia64/kprobes.h	2005-09-30 17:17:35.000000000 -0400
+++ linux-2.6.14-rc3/include/asm-ia64/kprobes.h	2005-10-05 15:28:35.000000000 -0400
@@ -26,6 +26,7 @@
  */
 #include <linux/types.h>
 #include <linux/ptrace.h>
+#include <linux/percpu.h>
 #include <asm/break.h>
 
 #define MAX_INSN_SIZE   16
@@ -62,6 +63,18 @@ typedef struct _bundle {
 	} quad1;
 } __attribute__((__aligned__(16)))  bundle_t;
 
+struct prev_kprobe {
+	struct kprobe *kp;
+	unsigned long status;
+};
+
+/* per-cpu kprobe control block */
+struct kprobe_ctlblk {
+	unsigned long kprobe_status;
+	struct pt_regs jprobe_saved_regs;
+	struct prev_kprobe prev_kprobe;
+};
+
 #define JPROBE_ENTRY(pentry)	(kprobe_opcode_t *)pentry
 
 #define ARCH_SUPPORTS_KRETPROBES

[PATCH 5/9] Kprobes: Track kprobe on a per_cpu basis - ppc64 changes

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

PPC64 changes to track kprobe execution on a per-cpu basis. We now track
the kprobe state machine independently on each cpu using an arch specific
kprobe control block.

Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
---

 arch/ppc64/kernel/kprobes.c |   94 +++++++++++++++++++++++++-------------------
 include/asm-ppc64/kprobes.h |   15 +++++++
 2 files changed, 69 insertions(+), 40 deletions(-)

Index: linux-2.6.14-rc3/arch/ppc64/kernel/kprobes.c
===================================================================
--- linux-2.6.14-rc3.orig/arch/ppc64/kernel/kprobes.c	2005-10-05 15:23:42.000000000 -0400
+++ linux-2.6.14-rc3/arch/ppc64/kernel/kprobes.c	2005-10-05 15:32:00.000000000 -0400
@@ -37,12 +37,8 @@
 #include <asm/sstep.h>
 
 static DECLARE_MUTEX(kprobe_mutex);
-
-static struct kprobe *current_kprobe;
-static unsigned long kprobe_status, kprobe_saved_msr;
-static struct kprobe *kprobe_prev;
-static unsigned long kprobe_status_prev, kprobe_saved_msr_prev;
-static struct pt_regs jprobe_saved_regs;
+DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
+DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
 
 int __kprobes arch_prepare_kprobe(struct kprobe *p)
 {
@@ -108,18 +104,25 @@ static inline void prepare_singlestep(st
 		regs->nip = (unsigned long)p->ainsn.insn;
 }
 
-static inline void save_previous_kprobe(void)
+static inline void save_previous_kprobe(struct kprobe_ctlblk *kcb)
+{
+	kcb->prev_kprobe.kp = kprobe_running();
+	kcb->prev_kprobe.status = kcb->kprobe_status;
+	kcb->prev_kprobe.saved_msr = kcb->kprobe_saved_msr;
+}
+
+static inline void restore_previous_kprobe(struct kprobe_ctlblk *kcb)
 {
-	kprobe_prev = current_kprobe;
-	kprobe_status_prev = kprobe_status;
-	kprobe_saved_msr_prev = kprobe_saved_msr;
+	__get_cpu_var(current_kprobe) = kcb->prev_kprobe.kp;
+	kcb->kprobe_status = kcb->prev_kprobe.status;
+	kcb->kprobe_saved_msr = kcb->prev_kprobe.saved_msr;
 }
 
-static inline void restore_previous_kprobe(void)
+static inline void set_current_kprobe(struct kprobe *p, struct pt_regs *regs,
+				struct kprobe_ctlblk *kcb)
 {
-	current_kprobe = kprobe_prev;
-	kprobe_status = kprobe_status_prev;
-	kprobe_saved_msr = kprobe_saved_msr_prev;
+	__get_cpu_var(current_kprobe) = p;
+	kcb->kprobe_saved_msr = regs->msr;
 }
 
 void __kprobes arch_prepare_kretprobe(struct kretprobe *rp,
@@ -145,6 +148,7 @@ static inline int kprobe_handler(struct 
 	struct kprobe *p;
 	int ret = 0;
 	unsigned int *addr = (unsigned int *)regs->nip;
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 
 	/* Check we're not actually recursing */
 	if (kprobe_running()) {
@@ -153,10 +157,10 @@ static inline int kprobe_handler(struct 
 		p = get_kprobe(addr);
 		if (p) {
 			kprobe_opcode_t insn = *p->ainsn.insn;
-			if (kprobe_status == KPROBE_HIT_SS &&
+			if (kcb->kprobe_status == KPROBE_HIT_SS &&
 					is_trap(insn)) {
 				regs->msr &= ~MSR_SE;
-				regs->msr |= kprobe_saved_msr;
+				regs->msr |= kcb->kprobe_saved_msr;
 				unlock_kprobes();
 				goto no_kprobe;
 			}
@@ -166,15 +170,15 @@ static inline int kprobe_handler(struct 
 			 * just single step on the instruction of the new probe
 			 * without calling any user handlers.
 			 */
-			save_previous_kprobe();
-			current_kprobe = p;
-			kprobe_saved_msr = regs->msr;
+			save_previous_kprobe(kcb);
+			set_current_kprobe(p, regs, kcb);
+			kcb->kprobe_saved_msr = regs->msr;
 			p->nmissed++;
 			prepare_singlestep(p, regs);
-			kprobe_status = KPROBE_REENTER;
+			kcb->kprobe_status = KPROBE_REENTER;
 			return 1;
 		} else {
-			p = current_kprobe;
+			p = __get_cpu_var(current_kprobe);
 			if (p->break_handler && p->break_handler(p, regs)) {
 				goto ss_probe;
 			}
@@ -214,16 +218,15 @@ static inline int kprobe_handler(struct 
 	 * in post_kprobe_handler().
 	 */
 	preempt_disable();
-	kprobe_status = KPROBE_HIT_ACTIVE;
-	current_kprobe = p;
-	kprobe_saved_msr = regs->msr;
+	kcb->kprobe_status = KPROBE_HIT_ACTIVE;
+	set_current_kprobe(p, regs, kcb);
 	if (p->pre_handler && p->pre_handler(p, regs))
 		/* handler has already set things up, so skip ss setup */
 		return 1;
 
 ss_probe:
 	prepare_singlestep(p, regs);
-	kprobe_status = KPROBE_HIT_SS;
+	kcb->kprobe_status = KPROBE_HIT_SS;
 	return 1;
 
 no_kprobe:
@@ -292,6 +295,7 @@ int __kprobes trampoline_probe_handler(s
 	BUG_ON(!orig_ret_address || (orig_ret_address == trampoline_address));
 	regs->nip = orig_ret_address;
 
+	reset_current_kprobe();
 	unlock_kprobes();
 	preempt_enable_no_resched();
 
@@ -324,22 +328,26 @@ static void __kprobes resume_execution(s
 
 static inline int post_kprobe_handler(struct pt_regs *regs)
 {
-	if (!kprobe_running())
+	struct kprobe *cur = kprobe_running();
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+
+	if (!cur)
 		return 0;
 
-	if ((kprobe_status != KPROBE_REENTER) && current_kprobe->post_handler) {
-		kprobe_status = KPROBE_HIT_SSDONE;
-		current_kprobe->post_handler(current_kprobe, regs, 0);
+	if ((kcb->kprobe_status != KPROBE_REENTER) && cur->post_handler) {
+		kcb->kprobe_status = KPROBE_HIT_SSDONE;
+		cur->post_handler(cur, regs, 0);
 	}
 
-	resume_execution(current_kprobe, regs);
-	regs->msr |= kprobe_saved_msr;
+	resume_execution(cur, regs);
+	regs->msr |= kcb->kprobe_saved_msr;
 
 	/*Restore back the original saved kprobes variables and continue. */
-	if (kprobe_status == KPROBE_REENTER) {
-		restore_previous_kprobe();
+	if (kcb->kprobe_status == KPROBE_REENTER) {
+		restore_previous_kprobe(kcb);
 		goto out;
 	}
+	reset_current_kprobe();
 	unlock_kprobes();
 out:
 	preempt_enable_no_resched();
@@ -358,15 +366,18 @@ out:
 /* Interrupts disabled, kprobe_lock held. */
 static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
 {
-	if (current_kprobe->fault_handler
-	    && current_kprobe->fault_handler(current_kprobe, regs, trapnr))
+	struct kprobe *cur = kprobe_running();
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+
+	if (cur->fault_handler && cur->fault_handler(cur, regs, trapnr))
 		return 1;
 
-	if (kprobe_status & KPROBE_HIT_SS) {
-		resume_execution(current_kprobe, regs);
+	if (kcb->kprobe_status & KPROBE_HIT_SS) {
+		resume_execution(cur, regs);
 		regs->msr &= ~MSR_SE;
-		regs->msr |= kprobe_saved_msr;
+		regs->msr |= kcb->kprobe_saved_msr;
 
+		reset_current_kprobe();
 		unlock_kprobes();
 		preempt_enable_no_resched();
 	}
@@ -412,8 +423,9 @@ int __kprobes kprobe_exceptions_notify(s
 int __kprobes setjmp_pre_handler(struct kprobe *p, struct pt_regs *regs)
 {
 	struct jprobe *jp = container_of(p, struct jprobe, kp);
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 
-	memcpy(&jprobe_saved_regs, regs, sizeof(struct pt_regs));
+	memcpy(&kcb->jprobe_saved_regs, regs, sizeof(struct pt_regs));
 
 	/* setup return addr to the jprobe handler routine */
 	regs->nip = (unsigned long)(((func_descr_t *)jp->entry)->entry);
@@ -433,12 +445,14 @@ void __kprobes jprobe_return_end(void)
 
 int __kprobes longjmp_break_handler(struct kprobe *p, struct pt_regs *regs)
 {
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+
 	/*
 	 * FIXME - we should ideally be validating that we got here 'cos
 	 * of the "trap" in jprobe_return() above, before restoring the
 	 * saved regs...
 	 */
-	memcpy(regs, &jprobe_saved_regs, sizeof(struct pt_regs));
+	memcpy(regs, &kcb->jprobe_saved_regs, sizeof(struct pt_regs));
 	return 1;
 }
 
Index: linux-2.6.14-rc3/include/asm-ppc64/kprobes.h
===================================================================
--- linux-2.6.14-rc3.orig/include/asm-ppc64/kprobes.h	2005-09-30 17:17:35.000000000 -0400
+++ linux-2.6.14-rc3/include/asm-ppc64/kprobes.h	2005-10-05 15:31:15.000000000 -0400
@@ -28,6 +28,7 @@
  */
 #include <linux/types.h>
 #include <linux/ptrace.h>
+#include <linux/percpu.h>
 
 struct pt_regs;
 
@@ -54,6 +55,20 @@ struct arch_specific_insn {
 	kprobe_opcode_t *insn;
 };
 
+struct prev_kprobe {
+	struct kprobe *kp;
+	unsigned long status;
+	unsigned long saved_msr;
+};
+
+/* per-cpu kprobe control block */
+struct kprobe_ctlblk {
+	unsigned long kprobe_status;
+	unsigned long kprobe_saved_msr;
+	struct pt_regs jprobe_saved_regs;
+	struct prev_kprobe prev_kprobe;
+};
+
 #ifdef CONFIG_KPROBES
 extern int kprobe_exceptions_notify(struct notifier_block *self,
 				    unsigned long val, void *data);

[PATCH 6/9] Kprobes: Track kprobe on a per_cpu basis - sparc64 changes

[Ananth N Mavinakayanahalli]

Dave,
This patch is similar to the one you reviewed sometime back:
http://sourceware.org/ml/systemtap/2005-q3/msg00196.html

From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

Sparc64 changes to track kprobe execution on a per-cpu basis. We now track
the kprobe state machine independently on each cpu using an arch
specific kprobe control block.

Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
---

 arch/sparc64/kernel/kprobes.c |  131 +++++++++++++++++++++---------------------
 include/asm-sparc64/kprobes.h |   20 ++++++
 2 files changed, 87 insertions(+), 64 deletions(-)

Index: linux-2.6.14-rc3/arch/sparc64/kernel/kprobes.c
===================================================================
--- linux-2.6.14-rc3.orig/arch/sparc64/kernel/kprobes.c	2005-10-05 15:24:09.000000000 -0400
+++ linux-2.6.14-rc3/arch/sparc64/kernel/kprobes.c	2005-10-05 16:06:10.000000000 -0400
@@ -38,6 +38,9 @@
  * - Mark that we are no longer actively in a kprobe.
  */
 
+DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
+DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
+
 int __kprobes arch_prepare_kprobe(struct kprobe *p)
 {
 	return 0;
@@ -66,46 +69,39 @@ void __kprobes arch_remove_kprobe(struct
 {
 }
 
-static struct kprobe *current_kprobe;
-static unsigned long current_kprobe_orig_tnpc;
-static unsigned long current_kprobe_orig_tstate_pil;
-static unsigned int kprobe_status;
-static struct kprobe *kprobe_prev;
-static unsigned long kprobe_orig_tnpc_prev;
-static unsigned long kprobe_orig_tstate_pil_prev;
-static unsigned int kprobe_status_prev;
-
-static inline void save_previous_kprobe(void)
+static inline void save_previous_kprobe(struct kprobe_ctlblk *kcb)
 {
-	kprobe_status_prev = kprobe_status;
-	kprobe_orig_tnpc_prev = current_kprobe_orig_tnpc;
-	kprobe_orig_tstate_pil_prev = current_kprobe_orig_tstate_pil;
-	kprobe_prev = current_kprobe;
+	kcb->prev_kprobe.kp = kprobe_running();
+	kcb->prev_kprobe.status = kcb->kprobe_status;
+	kcb->prev_kprobe.orig_tnpc = kcb->kprobe_orig_tnpc;
+	kcb->prev_kprobe.orig_tstate_pil = kcb->kprobe_orig_tstate_pil;
 }
 
-static inline void restore_previous_kprobe(void)
+static inline void restore_previous_kprobe(struct kprobe_ctlblk *kcb)
 {
-	kprobe_status = kprobe_status_prev;
-	current_kprobe_orig_tnpc = kprobe_orig_tnpc_prev;
-	current_kprobe_orig_tstate_pil = kprobe_orig_tstate_pil_prev;
-	current_kprobe = kprobe_prev;
+	__get_cpu_var(current_kprobe) = kcb->prev_kprobe.kp;
+	kcb->kprobe_status = kcb->prev_kprobe.status;
+	kcb->kprobe_orig_tnpc = kcb->prev_kprobe.orig_tnpc;
+	kcb->kprobe_orig_tstate_pil = kcb->prev_kprobe.orig_tstate_pil;
 }
 
-static inline void set_current_kprobe(struct kprobe *p, struct pt_regs *regs)
+static inline void set_current_kprobe(struct kprobe *p, struct pt_regs *regs,
+				struct kprobe_ctlblk *kcb)
 {
-	current_kprobe_orig_tnpc = regs->tnpc;
-	current_kprobe_orig_tstate_pil = (regs->tstate & TSTATE_PIL);
-	current_kprobe = p;
+	__get_cpu_var(current_kprobe) = p;
+	kcb->kprobe_orig_tnpc = regs->tnpc;
+	kcb->kprobe_orig_tstate_pil = (regs->tstate & TSTATE_PIL);
 }
 
-static inline void prepare_singlestep(struct kprobe *p, struct pt_regs *regs)
+static inline void prepare_singlestep(struct kprobe *p, struct pt_regs *regs,
+			struct kprobe_ctlblk *kcb)
 {
 	regs->tstate |= TSTATE_PIL;
 
 	/*single step inline, if it a breakpoint instruction*/
 	if (p->opcode == BREAKPOINT_INSTRUCTION) {
 		regs->tpc = (unsigned long) p->addr;
-		regs->tnpc = current_kprobe_orig_tnpc;
+		regs->tnpc = kcb->kprobe_orig_tnpc;
 	} else {
 		regs->tpc = (unsigned long) &p->ainsn.insn[0];
 		regs->tnpc = (unsigned long) &p->ainsn.insn[1];
@@ -117,6 +113,7 @@ static int __kprobes kprobe_handler(stru
 	struct kprobe *p;
 	void *addr = (void *) regs->tpc;
 	int ret = 0;
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 
 	if (kprobe_running()) {
 		/* We *are* holding lock here, so this is safe.
@@ -124,9 +121,9 @@ static int __kprobes kprobe_handler(stru
 		 */
 		p = get_kprobe(addr);
 		if (p) {
-			if (kprobe_status == KPROBE_HIT_SS) {
+			if (kcb->kprobe_status == KPROBE_HIT_SS) {
 				regs->tstate = ((regs->tstate & ~TSTATE_PIL) |
-					current_kprobe_orig_tstate_pil);
+					kcb->kprobe_orig_tstate_pil);
 				unlock_kprobes();
 				goto no_kprobe;
 			}
@@ -136,14 +133,14 @@ static int __kprobes kprobe_handler(stru
 			 * just single step on the instruction of the new probe
 			 * without calling any user handlers.
 			 */
-			save_previous_kprobe();
-			set_current_kprobe(p, regs);
+			save_previous_kprobe(kcb);
+			set_current_kprobe(p, regs, kcb);
 			p->nmissed++;
-			kprobe_status = KPROBE_REENTER;
-			prepare_singlestep(p, regs);
+			kcb->kprobe_status = KPROBE_REENTER;
+			prepare_singlestep(p, regs, kcb);
 			return 1;
 		} else {
-			p = current_kprobe;
+			p = __get_cpu_var(current_kprobe);
 			if (p->break_handler && p->break_handler(p, regs))
 				goto ss_probe;
 		}
@@ -174,14 +171,14 @@ static int __kprobes kprobe_handler(stru
 	 * in post_kprobes_handler()
 	 */
 	preempt_disable();
-	set_current_kprobe(p, regs);
-	kprobe_status = KPROBE_HIT_ACTIVE;
+	set_current_kprobe(p, regs, kcb);
+	kcb->kprobe_status = KPROBE_HIT_ACTIVE;
 	if (p->pre_handler && p->pre_handler(p, regs))
 		return 1;
 
 ss_probe:
-	prepare_singlestep(p, regs);
-	kprobe_status = KPROBE_HIT_SS;
+	prepare_singlestep(p, regs, kcb);
+	kcb->kprobe_status = KPROBE_HIT_SS;
 	return 1;
 
 no_kprobe:
@@ -262,11 +259,12 @@ static void __kprobes retpc_fixup(struct
  * This function prepares to return from the post-single-step
  * breakpoint trap.
  */
-static void __kprobes resume_execution(struct kprobe *p, struct pt_regs *regs)
+static void __kprobes resume_execution(struct kprobe *p,
+		struct pt_regs *regs, struct kprobe_ctlblk *kcb)
 {
 	u32 insn = p->ainsn.insn[0];
 
-	regs->tpc = current_kprobe_orig_tnpc;
+	regs->tpc = kcb->kprobe_orig_tnpc;
 	regs->tnpc = relbranch_fixup(insn,
 				     (unsigned long) p->addr,
 				     (unsigned long) &p->ainsn.insn[0],
@@ -274,26 +272,30 @@ static void __kprobes resume_execution(s
 	retpc_fixup(regs, insn, (unsigned long) p->addr);
 
 	regs->tstate = ((regs->tstate & ~TSTATE_PIL) |
-			current_kprobe_orig_tstate_pil);
+			kcb->kprobe_orig_tstate_pil);
 }
 
 static inline int post_kprobe_handler(struct pt_regs *regs)
 {
-	if (!kprobe_running())
+	struct kprobe *cur = kprobe_running();
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+
+	if (!cur)
 		return 0;
 
-	if ((kprobe_status != KPROBE_REENTER) && current_kprobe->post_handler) {
-		kprobe_status = KPROBE_HIT_SSDONE;
-		current_kprobe->post_handler(current_kprobe, regs, 0);
+	if ((kcb->kprobe_status != KPROBE_REENTER) && cur->post_handler) {
+		kcb->kprobe_status = KPROBE_HIT_SSDONE;
+		cur->post_handler(cur, regs, 0);
 	}
 
-	resume_execution(current_kprobe, regs);
+	resume_execution(cur, regs, kcb);
 
 	/*Restore back the original saved kprobes variables and continue. */
-	if (kprobe_status == KPROBE_REENTER) {
-		restore_previous_kprobe();
+	if (kcb->kprobe_status == KPROBE_REENTER) {
+		restore_previous_kprobe(kcb);
 		goto out;
 	}
+	reset_current_kprobe();
 	unlock_kprobes();
 out:
 	preempt_enable_no_resched();
@@ -304,13 +306,16 @@ out:
 /* Interrupts disabled, kprobe_lock held. */
 static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
 {
-	if (current_kprobe->fault_handler
-	    && current_kprobe->fault_handler(current_kprobe, regs, trapnr))
+	struct kprobe *cur = kprobe_running();
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+
+	if (cur->fault_handler && cur->fault_handler(cur, regs, trapnr))
 		return 1;
 
-	if (kprobe_status & KPROBE_HIT_SS) {
-		resume_execution(current_kprobe, regs);
+	if (kcb->kprobe_status & KPROBE_HIT_SS) {
+		resume_execution(cur, regs, kcb);
 
+		reset_current_kprobe();
 		unlock_kprobes();
 		preempt_enable_no_resched();
 	}
@@ -370,24 +375,21 @@ asmlinkage void __kprobes kprobe_trap(un
 }
 
 /* Jprobes support.  */
-static struct pt_regs jprobe_saved_regs;
-static struct pt_regs *jprobe_saved_regs_location;
-static struct sparc_stackf jprobe_saved_stack;
-
 int __kprobes setjmp_pre_handler(struct kprobe *p, struct pt_regs *regs)
 {
 	struct jprobe *jp = container_of(p, struct jprobe, kp);
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 
-	jprobe_saved_regs_location = regs;
-	memcpy(&jprobe_saved_regs, regs, sizeof(*regs));
+	kcb->jprobe_saved_regs_location = regs;
+	memcpy(&(kcb->jprobe_saved_regs), regs, sizeof(*regs));
 
 	/* Save a whole stack frame, this gets arguments
 	 * pushed onto the stack after using up all the
 	 * arg registers.
 	 */
-	memcpy(&jprobe_saved_stack,
+	memcpy(&(kcb->jprobe_saved_stack),
 	       (char *) (regs->u_regs[UREG_FP] + STACK_BIAS),
-	       sizeof(jprobe_saved_stack));
+	       sizeof(kcb->jprobe_saved_stack));
 
 	regs->tpc  = (unsigned long) jp->entry;
 	regs->tnpc = ((unsigned long) jp->entry) + 0x4UL;
@@ -411,14 +413,15 @@ extern void __show_regs(struct pt_regs *
 int __kprobes longjmp_break_handler(struct kprobe *p, struct pt_regs *regs)
 {
 	u32 *addr = (u32 *) regs->tpc;
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 
 	if (addr == (u32 *) jprobe_return_trap_instruction) {
-		if (jprobe_saved_regs_location != regs) {
+		if (kcb->jprobe_saved_regs_location != regs) {
 			printk("JPROBE: Current regs (%p) does not match "
 			       "saved regs (%p).\n",
-			       regs, jprobe_saved_regs_location);
+			       regs, kcb->jprobe_saved_regs_location);
 			printk("JPROBE: Saved registers\n");
-			__show_regs(jprobe_saved_regs_location);
+			__show_regs(kcb->jprobe_saved_regs_location);
 			printk("JPROBE: Current registers\n");
 			__show_regs(regs);
 			BUG();
@@ -427,11 +430,11 @@ int __kprobes longjmp_break_handler(stru
 		 * first so that UREG_FP is the original one for
 		 * the stack frame restore.
 		 */
-		memcpy(regs, &jprobe_saved_regs, sizeof(*regs));
+		memcpy(regs, &(kcb->jprobe_saved_regs), sizeof(*regs));
 
 		memcpy((char *) (regs->u_regs[UREG_FP] + STACK_BIAS),
-		       &jprobe_saved_stack,
-		       sizeof(jprobe_saved_stack));
+		       &(kcb->jprobe_saved_stack),
+		       sizeof(kcb->jprobe_saved_stack));
 
 		return 1;
 	}
Index: linux-2.6.14-rc3/include/asm-sparc64/kprobes.h
===================================================================
--- linux-2.6.14-rc3.orig/include/asm-sparc64/kprobes.h	2005-09-30 17:17:35.000000000 -0400
+++ linux-2.6.14-rc3/include/asm-sparc64/kprobes.h	2005-10-05 15:33:35.000000000 -0400
@@ -3,6 +3,7 @@
 
 #include <linux/config.h>
 #include <linux/types.h>
+#include <linux/percpu.h>
 
 typedef u32 kprobe_opcode_t;
 
@@ -18,6 +19,25 @@ struct arch_specific_insn {
 	kprobe_opcode_t insn[MAX_INSN_SIZE];
 };
 
+struct prev_kprobe {
+	struct kprobe *kp;
+	unsigned int status;
+	unsigned long orig_tnpc;
+	unsigned long orig_tstate_pil;
+};
+
+/* per-cpu kprobe control block */
+struct kprobe_ctlblk {
+	unsigned long kprobe_status;
+	unsigned long kprobe_orig_tnpc;
+	unsigned long kprobe_orig_tstate_pil;
+	long *jprobe_saved_esp;
+	struct pt_regs jprobe_saved_regs;
+	struct pt_regs *jprobe_saved_regs_location;
+	struct sparc_stackf jprobe_saved_stack;
+	struct prev_kprobe prev_kprobe;
+};
+
 #ifdef CONFIG_KPROBES
 extern int kprobe_exceptions_notify(struct notifier_block *self,
 				    unsigned long val, void *data);

[PATCH 7/9] Kprobes: Track kprobe on a per_cpu basis - x86_64 changes

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

x86_64 changes to track kprobe execution on a per-cpu basis. We now track
the kprobe state machine independently on each cpu using a arch specific
kprobe control block.

Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
---

 arch/x86_64/kernel/kprobes.c |  129 +++++++++++++++++++++++--------------------
 include/asm-x86_64/kprobes.h |   19 ++++++
 2 files changed, 89 insertions(+), 59 deletions(-)

Index: linux-2.6.14-rc3/arch/x86_64/kernel/kprobes.c
===================================================================
--- linux-2.6.14-rc3.orig/arch/x86_64/kernel/kprobes.c	2005-10-05 16:08:04.000000000 -0400
+++ linux-2.6.14-rc3/arch/x86_64/kernel/kprobes.c	2005-10-05 16:08:18.000000000 -0400
@@ -44,17 +44,10 @@
 #include <asm/kdebug.h>
 
 static DECLARE_MUTEX(kprobe_mutex);
-
-static struct kprobe *current_kprobe;
-static unsigned long kprobe_status, kprobe_old_rflags, kprobe_saved_rflags;
-static struct kprobe *kprobe_prev;
-static unsigned long kprobe_status_prev, kprobe_old_rflags_prev, kprobe_saved_rflags_prev;
-static struct pt_regs jprobe_saved_regs;
-static long *jprobe_saved_rsp;
 void jprobe_return_end(void);
 
-/* copy of the kernel stack at the probe fire time */
-static kprobe_opcode_t jprobes_stack[MAX_STACK_SIZE];
+DEFINE_PER_CPU(struct kprobe *, current_kprobe) = NULL;
+DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
 
 /*
  * returns non-zero if opcode modifies the interrupt flag.
@@ -236,29 +229,30 @@ void __kprobes arch_remove_kprobe(struct
 	up(&kprobe_mutex);
 }
 
-static inline void save_previous_kprobe(void)
+static inline void save_previous_kprobe(struct kprobe_ctlblk *kcb)
 {
-	kprobe_prev = current_kprobe;
-	kprobe_status_prev = kprobe_status;
-	kprobe_old_rflags_prev = kprobe_old_rflags;
-	kprobe_saved_rflags_prev = kprobe_saved_rflags;
+	kcb->prev_kprobe.kp = kprobe_running();
+	kcb->prev_kprobe.status = kcb->kprobe_status;
+	kcb->prev_kprobe.old_rflags = kcb->kprobe_old_rflags;
+	kcb->prev_kprobe.saved_rflags = kcb->kprobe_saved_rflags;
 }
 
-static inline void restore_previous_kprobe(void)
+static inline void restore_previous_kprobe(struct kprobe_ctlblk *kcb)
 {
-	current_kprobe = kprobe_prev;
-	kprobe_status = kprobe_status_prev;
-	kprobe_old_rflags = kprobe_old_rflags_prev;
-	kprobe_saved_rflags = kprobe_saved_rflags_prev;
+	__get_cpu_var(current_kprobe) = kcb->prev_kprobe.kp;
+	kcb->kprobe_status = kcb->prev_kprobe.status;
+	kcb->kprobe_old_rflags = kcb->prev_kprobe.old_rflags;
+	kcb->kprobe_saved_rflags = kcb->prev_kprobe.saved_rflags;
 }
 
-static inline void set_current_kprobe(struct kprobe *p, struct pt_regs *regs)
+static inline void set_current_kprobe(struct kprobe *p, struct pt_regs *regs,
+				struct kprobe_ctlblk *kcb)
 {
-	current_kprobe = p;
-	kprobe_saved_rflags = kprobe_old_rflags
+	__get_cpu_var(current_kprobe) = p;
+	kcb->kprobe_saved_rflags = kcb->kprobe_old_rflags
 		= (regs->eflags & (TF_MASK | IF_MASK));
 	if (is_IF_modifier(p->ainsn.insn))
-		kprobe_saved_rflags &= ~IF_MASK;
+		kcb->kprobe_saved_rflags &= ~IF_MASK;
 }
 
 static void __kprobes prepare_singlestep(struct kprobe *p, struct pt_regs *regs)
@@ -301,6 +295,7 @@ int __kprobes kprobe_handler(struct pt_r
 	struct kprobe *p;
 	int ret = 0;
 	kprobe_opcode_t *addr = (kprobe_opcode_t *)(regs->rip - sizeof(kprobe_opcode_t));
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 
 	/* Check we're not actually recursing */
 	if (kprobe_running()) {
@@ -308,13 +303,13 @@ int __kprobes kprobe_handler(struct pt_r
 		   Disarm the probe we just hit, and ignore it. */
 		p = get_kprobe(addr);
 		if (p) {
-			if (kprobe_status == KPROBE_HIT_SS &&
+			if (kcb->kprobe_status == KPROBE_HIT_SS &&
 				*p->ainsn.insn == BREAKPOINT_INSTRUCTION) {
 				regs->eflags &= ~TF_MASK;
-				regs->eflags |= kprobe_saved_rflags;
+				regs->eflags |= kcb->kprobe_saved_rflags;
 				unlock_kprobes();
 				goto no_kprobe;
-			} else if (kprobe_status == KPROBE_HIT_SSDONE) {
+			} else if (kcb->kprobe_status == KPROBE_HIT_SSDONE) {
 				/* TODO: Provide re-entrancy from
 				 * post_kprobes_handler() and avoid exception
 				 * stack corruption while single-stepping on
@@ -322,6 +317,7 @@ int __kprobes kprobe_handler(struct pt_r
 				 */
 				arch_disarm_kprobe(p);
 				regs->rip = (unsigned long)p->addr;
+				reset_current_kprobe();
 				ret = 1;
 			} else {
 				/* We have reentered the kprobe_handler(), since
@@ -331,15 +327,15 @@ int __kprobes kprobe_handler(struct pt_r
 				 * of the new probe without calling any user
 				 * handlers.
 				 */
-				save_previous_kprobe();
-				set_current_kprobe(p, regs);
+				save_previous_kprobe(kcb);
+				set_current_kprobe(p, regs, kcb);
 				p->nmissed++;
 				prepare_singlestep(p, regs);
-				kprobe_status = KPROBE_REENTER;
+				kcb->kprobe_status = KPROBE_REENTER;
 				return 1;
 			}
 		} else {
-			p = current_kprobe;
+			p = __get_cpu_var(current_kprobe);
 			if (p->break_handler && p->break_handler(p, regs)) {
 				goto ss_probe;
 			}
@@ -374,8 +370,8 @@ int __kprobes kprobe_handler(struct pt_r
 	 * in post_kprobe_handler()
 	 */
 	preempt_disable();
-	kprobe_status = KPROBE_HIT_ACTIVE;
-	set_current_kprobe(p, regs);
+	set_current_kprobe(p, regs, kcb);
+	kcb->kprobe_status = KPROBE_HIT_ACTIVE;
 
 	if (p->pre_handler && p->pre_handler(p, regs))
 		/* handler has already set things up, so skip ss setup */
@@ -383,7 +379,7 @@ int __kprobes kprobe_handler(struct pt_r
 
 ss_probe:
 	prepare_singlestep(p, regs);
-	kprobe_status = KPROBE_HIT_SS;
+	kcb->kprobe_status = KPROBE_HIT_SS;
 	return 1;
 
 no_kprobe:
@@ -451,6 +447,7 @@ int __kprobes trampoline_probe_handler(s
 	BUG_ON(!orig_ret_address || (orig_ret_address == trampoline_address));
 	regs->rip = orig_ret_address;
 
+	reset_current_kprobe();
 	unlock_kprobes();
 	preempt_enable_no_resched();
 
@@ -484,7 +481,8 @@ int __kprobes trampoline_probe_handler(s
  * that is atop the stack is the address following the copied instruction.
  * We need to make it the address following the original instruction.
  */
-static void __kprobes resume_execution(struct kprobe *p, struct pt_regs *regs)
+static void __kprobes resume_execution(struct kprobe *p,
+		struct pt_regs *regs, struct kprobe_ctlblk *kcb)
 {
 	unsigned long *tos = (unsigned long *)regs->rsp;
 	unsigned long next_rip = 0;
@@ -499,7 +497,7 @@ static void __kprobes resume_execution(s
 	switch (*insn) {
 	case 0x9c:		/* pushfl */
 		*tos &= ~(TF_MASK | IF_MASK);
-		*tos |= kprobe_old_rflags;
+		*tos |= kcb->kprobe_old_rflags;
 		break;
 	case 0xc3:		/* ret/lret */
 	case 0xcb:
@@ -544,24 +542,28 @@ static void __kprobes resume_execution(s
  */
 int __kprobes post_kprobe_handler(struct pt_regs *regs)
 {
-	if (!kprobe_running())
+	struct kprobe *cur = kprobe_running();
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+
+	if (!cur)
 		return 0;
 
-	if ((kprobe_status != KPROBE_REENTER) && current_kprobe->post_handler) {
-		kprobe_status = KPROBE_HIT_SSDONE;
-		current_kprobe->post_handler(current_kprobe, regs, 0);
+	if ((kcb->kprobe_status != KPROBE_REENTER) && cur->post_handler) {
+		kcb->kprobe_status = KPROBE_HIT_SSDONE;
+		cur->post_handler(cur, regs, 0);
 	}
 
-	resume_execution(current_kprobe, regs);
-	regs->eflags |= kprobe_saved_rflags;
+	resume_execution(cur, regs, kcb);
+	regs->eflags |= kcb->kprobe_saved_rflags;
 
 	/* Restore the original saved kprobes variables and continue. */
-	if (kprobe_status == KPROBE_REENTER) {
-		restore_previous_kprobe();
+	if (kcb->kprobe_status == KPROBE_REENTER) {
+		restore_previous_kprobe(kcb);
 		goto out;
 	} else {
 		unlock_kprobes();
 	}
+	reset_current_kprobe();
 out:
 	preempt_enable_no_resched();
 
@@ -579,14 +581,17 @@ out:
 /* Interrupts disabled, kprobe_lock held. */
 int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr)
 {
-	if (current_kprobe->fault_handler
-	    && current_kprobe->fault_handler(current_kprobe, regs, trapnr))
+	struct kprobe *cur = kprobe_running();
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+
+	if (cur->fault_handler && cur->fault_handler(cur, regs, trapnr))
 		return 1;
 
-	if (kprobe_status & KPROBE_HIT_SS) {
-		resume_execution(current_kprobe, regs);
-		regs->eflags |= kprobe_old_rflags;
+	if (kcb->kprobe_status & KPROBE_HIT_SS) {
+		resume_execution(cur, regs, kcb);
+		regs->eflags |= kcb->kprobe_old_rflags;
 
+		reset_current_kprobe();
 		unlock_kprobes();
 		preempt_enable_no_resched();
 	}
@@ -629,10 +634,11 @@ int __kprobes setjmp_pre_handler(struct 
 {
 	struct jprobe *jp = container_of(p, struct jprobe, kp);
 	unsigned long addr;
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 
-	jprobe_saved_regs = *regs;
-	jprobe_saved_rsp = (long *) regs->rsp;
-	addr = (unsigned long)jprobe_saved_rsp;
+	kcb->jprobe_saved_regs = *regs;
+	kcb->jprobe_saved_rsp = (long *) regs->rsp;
+	addr = (unsigned long)(kcb->jprobe_saved_rsp);
 	/*
 	 * As Linus pointed out, gcc assumes that the callee
 	 * owns the argument space and could overwrite it, e.g.
@@ -640,7 +646,8 @@ int __kprobes setjmp_pre_handler(struct 
 	 * we also save and restore enough stack bytes to cover
 	 * the argument area.
 	 */
-	memcpy(jprobes_stack, (kprobe_opcode_t *) addr, MIN_STACK_SIZE(addr));
+	memcpy(kcb->jprobes_stack, (kprobe_opcode_t *)addr,
+			MIN_STACK_SIZE(addr));
 	regs->eflags &= ~IF_MASK;
 	regs->rip = (unsigned long)(jp->entry);
 	return 1;
@@ -648,34 +655,38 @@ int __kprobes setjmp_pre_handler(struct 
 
 void __kprobes jprobe_return(void)
 {
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+
 	asm volatile ("       xchg   %%rbx,%%rsp     \n"
 		      "       int3			\n"
 		      "       .globl jprobe_return_end	\n"
 		      "       jprobe_return_end:	\n"
 		      "       nop			\n"::"b"
-		      (jprobe_saved_rsp):"memory");
+		      (kcb->jprobe_saved_rsp):"memory");
 }
 
 int __kprobes longjmp_break_handler(struct kprobe *p, struct pt_regs *regs)
 {
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 	u8 *addr = (u8 *) (regs->rip - 1);
-	unsigned long stack_addr = (unsigned long)jprobe_saved_rsp;
+	unsigned long stack_addr = (unsigned long)(kcb->jprobe_saved_rsp);
 	struct jprobe *jp = container_of(p, struct jprobe, kp);
 
 	if ((addr > (u8 *) jprobe_return) && (addr < (u8 *) jprobe_return_end)) {
-		if ((long *)regs->rsp != jprobe_saved_rsp) {
+		if ((long *)regs->rsp != kcb->jprobe_saved_rsp) {
 			struct pt_regs *saved_regs =
-			    container_of(jprobe_saved_rsp, struct pt_regs, rsp);
+			    container_of(kcb->jprobe_saved_rsp,
+					    struct pt_regs, rsp);
 			printk("current rsp %p does not match saved rsp %p\n",
-			       (long *)regs->rsp, jprobe_saved_rsp);
+			       (long *)regs->rsp, kcb->jprobe_saved_rsp);
 			printk("Saved registers for jprobe %p\n", jp);
 			show_registers(saved_regs);
 			printk("Current registers\n");
 			show_registers(regs);
 			BUG();
 		}
-		*regs = jprobe_saved_regs;
-		memcpy((kprobe_opcode_t *) stack_addr, jprobes_stack,
+		*regs = kcb->jprobe_saved_regs;
+		memcpy((kprobe_opcode_t *) stack_addr, kcb->jprobes_stack,
 		       MIN_STACK_SIZE(stack_addr));
 		return 1;
 	}
Index: linux-2.6.14-rc3/include/asm-x86_64/kprobes.h
===================================================================
--- linux-2.6.14-rc3.orig/include/asm-x86_64/kprobes.h	2005-10-05 16:07:44.000000000 -0400
+++ linux-2.6.14-rc3/include/asm-x86_64/kprobes.h	2005-10-05 16:08:18.000000000 -0400
@@ -25,6 +25,7 @@
  */
 #include <linux/types.h>
 #include <linux/ptrace.h>
+#include <linux/percpu.h>
 
 struct pt_regs;
 
@@ -48,6 +49,24 @@ struct arch_specific_insn {
 	kprobe_opcode_t *insn;
 };
 
+struct prev_kprobe {
+	struct kprobe *kp;
+	unsigned long status;
+	unsigned long old_rflags;
+	unsigned long saved_rflags;
+};
+
+/* per-cpu kprobe control block */
+struct kprobe_ctlblk {
+	unsigned long kprobe_status;
+	unsigned long kprobe_old_rflags;
+	unsigned long kprobe_saved_rflags;
+	long *jprobe_saved_rsp;
+	struct pt_regs jprobe_saved_regs;
+	kprobe_opcode_t jprobes_stack[MAX_STACK_SIZE];
+	struct prev_kprobe prev_kprobe;
+};
+
 /* trap3/1 are intr gates for kprobes.  So, restore the status of IF,
  * if necessary, before executing the original int3/1 (trap) handler.
  */

[PATCH 8/9] Kprobes: Use RCU for (un)register synchronization - base changes

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

Changes to the base kprobes infrastructure to use RCU for synchronization
during kprobe registration and unregistration. These changes coupled with
the arch kprobe changes (next in series):

a. serialize registration and unregistration of kprobes.
b. enable lockless execution of handlers. Handlers can now run in parallel.

Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
---

 include/linux/kprobes.h |    9 +---
 kernel/kprobes.c        |  101 +++++++++++++++++++-----------------------------
 2 files changed, 45 insertions(+), 65 deletions(-)

Index: linux-2.6.14-rc3/include/linux/kprobes.h
===================================================================
--- linux-2.6.14-rc3.orig/include/linux/kprobes.h	2005-10-07 21:40:43.000000000 -0400
+++ linux-2.6.14-rc3/include/linux/kprobes.h	2005-10-07 21:40:49.000000000 -0400
@@ -34,6 +34,8 @@
 #include <linux/notifier.h>
 #include <linux/smp.h>
 #include <linux/percpu.h>
+#include <linux/spinlock.h>
+#include <linux/rcupdate.h>
 
 #include <asm/kprobes.h>
 
@@ -146,10 +148,7 @@ struct kretprobe_instance {
 };
 
 #ifdef CONFIG_KPROBES
-/* Locks kprobe: irq must be disabled */
-void lock_kprobes(void);
-void unlock_kprobes(void);
-
+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);
@@ -160,7 +159,7 @@ extern void show_registers(struct pt_reg
 extern kprobe_opcode_t *get_insn_slot(void);
 extern void free_insn_slot(kprobe_opcode_t *slot);
 
-/* Get the kprobe at this addr (if any).  Must have called lock_kprobes */
+/* Get the kprobe at this addr (if any) - called under a rcu_read_lock() */
 struct kprobe *get_kprobe(void *addr);
 struct hlist_head * kretprobe_inst_table_head(struct task_struct *tsk);
 
Index: linux-2.6.14-rc3/kernel/kprobes.c
===================================================================
--- linux-2.6.14-rc3.orig/kernel/kprobes.c	2005-10-07 21:40:43.000000000 -0400
+++ linux-2.6.14-rc3/kernel/kprobes.c	2005-10-07 21:41:11.000000000 -0400
@@ -32,7 +32,6 @@
  *		<prasanna@in.ibm.com> added function-return probes.
  */
 #include <linux/kprobes.h>
-#include <linux/spinlock.h>
 #include <linux/hash.h>
 #include <linux/init.h>
 #include <linux/module.h>
@@ -48,8 +47,8 @@
 static struct hlist_head kprobe_table[KPROBE_TABLE_SIZE];
 static struct hlist_head kretprobe_inst_table[KPROBE_TABLE_SIZE];
 
-unsigned int kprobe_cpu = NR_CPUS;
-static DEFINE_SPINLOCK(kprobe_lock);
+static DEFINE_SPINLOCK(kprobe_lock);	/* Protects kprobe_table */
+DEFINE_SPINLOCK(kretprobe_lock);	/* Protects kretprobe_inst_table */
 static DEFINE_PER_CPU(struct kprobe *, kprobe_instance) = NULL;
 
 /*
@@ -152,41 +151,6 @@ void __kprobes free_insn_slot(kprobe_opc
 	}
 }
 
-/* Locks kprobe: irqs must be disabled */
-void __kprobes lock_kprobes(void)
-{
-	unsigned long flags = 0;
-
-	/* Avoiding local interrupts to happen right after we take the kprobe_lock
-	 * and before we get a chance to update kprobe_cpu, this to prevent
-	 * deadlock when we have a kprobe on ISR routine and a kprobe on task
-	 * routine
-	 */
-	local_irq_save(flags);
-
-	spin_lock(&kprobe_lock);
-	kprobe_cpu = smp_processor_id();
-
- 	local_irq_restore(flags);
-}
-
-void __kprobes unlock_kprobes(void)
-{
-	unsigned long flags = 0;
-
-	/* Avoiding local interrupts to happen right after we update
-	 * kprobe_cpu and before we get a a chance to release kprobe_lock,
-	 * this to prevent deadlock when we have a kprobe on ISR routine and
-	 * a kprobe on task routine
-	 */
-	local_irq_save(flags);
-
-	kprobe_cpu = NR_CPUS;
-	spin_unlock(&kprobe_lock);
-
- 	local_irq_restore(flags);
-}
-
 /* We have preemption disabled.. so it is safe to use __ versions */
 static inline void set_kprobe_instance(struct kprobe *kp)
 {
@@ -198,14 +162,19 @@ static inline void reset_kprobe_instance
 	__get_cpu_var(kprobe_instance) = NULL;
 }
 
-/* You have to be holding the kprobe_lock */
+/*
+ * This routine is called either:
+ * 	- under the kprobe_lock spinlock - during kprobe_[un]register()
+ * 				OR
+ * 	- under an rcu_read_lock() - from arch/xxx/kernel/kprobes.c
+ */
 struct kprobe __kprobes *get_kprobe(void *addr)
 {
 	struct hlist_head *head;
 	struct hlist_node *node;
 
 	head = &kprobe_table[hash_ptr(addr, KPROBE_HASH_BITS)];
-	hlist_for_each(node, head) {
+	hlist_for_each_rcu(node, head) {
 		struct kprobe *p = hlist_entry(node, struct kprobe, hlist);
 		if (p->addr == addr)
 			return p;
@@ -221,7 +190,7 @@ static int __kprobes aggr_pre_handler(st
 {
 	struct kprobe *kp;
 
-	list_for_each_entry(kp, &p->list, list) {
+	list_for_each_entry_rcu(kp, &p->list, list) {
 		if (kp->pre_handler) {
 			set_kprobe_instance(kp);
 			if (kp->pre_handler(kp, regs))
@@ -237,7 +206,7 @@ static void __kprobes aggr_post_handler(
 {
 	struct kprobe *kp;
 
-	list_for_each_entry(kp, &p->list, list) {
+	list_for_each_entry_rcu(kp, &p->list, list) {
 		if (kp->post_handler) {
 			set_kprobe_instance(kp);
 			kp->post_handler(kp, regs, flags);
@@ -276,6 +245,7 @@ static int __kprobes aggr_break_handler(
 	return ret;
 }
 
+/* Called with kretprobe_lock held */
 struct kretprobe_instance __kprobes *get_free_rp_inst(struct kretprobe *rp)
 {
 	struct hlist_node *node;
@@ -285,6 +255,7 @@ struct kretprobe_instance __kprobes *get
 	return NULL;
 }
 
+/* Called with kretprobe_lock held */
 static struct kretprobe_instance __kprobes *get_used_rp_inst(struct kretprobe
 							      *rp)
 {
@@ -295,6 +266,7 @@ static struct kretprobe_instance __kprob
 	return NULL;
 }
 
+/* Called with kretprobe_lock held */
 void __kprobes add_rp_inst(struct kretprobe_instance *ri)
 {
 	/*
@@ -313,6 +285,7 @@ void __kprobes add_rp_inst(struct kretpr
 	hlist_add_head(&ri->uflist, &ri->rp->used_instances);
 }
 
+/* Called with kretprobe_lock held */
 void __kprobes recycle_rp_inst(struct kretprobe_instance *ri)
 {
 	/* remove rp inst off the rprobe_inst_table */
@@ -346,13 +319,13 @@ void __kprobes kprobe_flush_task(struct 
 	struct hlist_node *node, *tmp;
 	unsigned long flags = 0;
 
-	spin_lock_irqsave(&kprobe_lock, flags);
+	spin_lock_irqsave(&kretprobe_lock, flags);
         head = kretprobe_inst_table_head(current);
         hlist_for_each_entry_safe(ri, node, tmp, head, hlist) {
                 if (ri->task == tk)
                         recycle_rp_inst(ri);
         }
-	spin_unlock_irqrestore(&kprobe_lock, flags);
+	spin_unlock_irqrestore(&kretprobe_lock, flags);
 }
 
 /*
@@ -363,9 +336,12 @@ static int __kprobes pre_handler_kretpro
 					   struct pt_regs *regs)
 {
 	struct kretprobe *rp = container_of(p, struct kretprobe, kp);
+	unsigned long flags = 0;
 
 	/*TODO: consider to only swap the RA after the last pre_handler fired */
+	spin_lock_irqsave(&kretprobe_lock, flags);
 	arch_prepare_kretprobe(rp, regs);
+	spin_unlock_irqrestore(&kretprobe_lock, flags);
 	return 0;
 }
 
@@ -396,13 +372,13 @@ static int __kprobes add_new_kprobe(stru
         struct kprobe *kp;
 
 	if (p->break_handler) {
-		list_for_each_entry(kp, &old_p->list, list) {
+		list_for_each_entry_rcu(kp, &old_p->list, list) {
 			if (kp->break_handler)
 				return -EEXIST;
 		}
-		list_add_tail(&p->list, &old_p->list);
+		list_add_tail_rcu(&p->list, &old_p->list);
 	} else
-		list_add(&p->list, &old_p->list);
+		list_add_rcu(&p->list, &old_p->list);
 	return 0;
 }
 
@@ -420,18 +396,18 @@ static inline void add_aggr_kprobe(struc
 	ap->break_handler = aggr_break_handler;
 
 	INIT_LIST_HEAD(&ap->list);
-	list_add(&p->list, &ap->list);
+	list_add_rcu(&p->list, &ap->list);
 
 	INIT_HLIST_NODE(&ap->hlist);
-	hlist_del(&p->hlist);
-	hlist_add_head(&ap->hlist,
+	hlist_del_rcu(&p->hlist);
+	hlist_add_head_rcu(&ap->hlist,
 		&kprobe_table[hash_ptr(ap->addr, KPROBE_HASH_BITS)]);
 }
 
 /*
  * This is the second or subsequent kprobe at the address - handle
  * the intricacies
- * TODO: Move kcalloc outside the spinlock
+ * TODO: Move kcalloc outside the spin_lock
  */
 static int __kprobes register_aggr_kprobe(struct kprobe *old_p,
 					  struct kprobe *p)
@@ -457,7 +433,7 @@ static int __kprobes register_aggr_kprob
 static inline void cleanup_kprobe(struct kprobe *p, unsigned long flags)
 {
 	arch_disarm_kprobe(p);
-	hlist_del(&p->hlist);
+	hlist_del_rcu(&p->hlist);
 	spin_unlock_irqrestore(&kprobe_lock, flags);
 	arch_remove_kprobe(p);
 }
@@ -465,11 +441,10 @@ static inline void cleanup_kprobe(struct
 static inline void cleanup_aggr_kprobe(struct kprobe *old_p,
 		struct kprobe *p, unsigned long flags)
 {
-	list_del(&p->list);
-	if (list_empty(&old_p->list)) {
+	list_del_rcu(&p->list);
+	if (list_empty(&old_p->list))
 		cleanup_kprobe(old_p, flags);
-		kfree(old_p);
-	} else
+	else
 		spin_unlock_irqrestore(&kprobe_lock, flags);
 }
 
@@ -492,9 +467,9 @@ int __kprobes register_kprobe(struct kpr
 	if ((ret = arch_prepare_kprobe(p)) != 0)
 		goto rm_kprobe;
 
+	p->nmissed = 0;
 	spin_lock_irqsave(&kprobe_lock, flags);
 	old_p = get_kprobe(p->addr);
-	p->nmissed = 0;
 	if (old_p) {
 		ret = register_aggr_kprobe(old_p, p);
 		goto out;
@@ -502,7 +477,7 @@ int __kprobes register_kprobe(struct kpr
 
 	arch_copy_kprobe(p);
 	INIT_HLIST_NODE(&p->hlist);
-	hlist_add_head(&p->hlist,
+	hlist_add_head_rcu(&p->hlist,
 		       &kprobe_table[hash_ptr(p->addr, KPROBE_HASH_BITS)]);
 
   	arch_arm_kprobe(p);
@@ -523,10 +498,16 @@ void __kprobes unregister_kprobe(struct 
 	spin_lock_irqsave(&kprobe_lock, flags);
 	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);
+
+		synchronize_sched();
+		if (old_p->pre_handler == aggr_pre_handler &&
+				list_empty(&old_p->list))
+			kfree(old_p);
 	} else
 		spin_unlock_irqrestore(&kprobe_lock, flags);
 }
@@ -603,13 +584,13 @@ void __kprobes unregister_kretprobe(stru
 
 	unregister_kprobe(&rp->kp);
 	/* No race here */
-	spin_lock_irqsave(&kprobe_lock, flags);
+	spin_lock_irqsave(&kretprobe_lock, flags);
 	free_rp_inst(rp);
 	while ((ri = get_used_rp_inst(rp)) != NULL) {
 		ri->rp = NULL;
 		hlist_del(&ri->uflist);
 	}
-	spin_unlock_irqrestore(&kprobe_lock, flags);
+	spin_unlock_irqrestore(&kretprobe_lock, flags);
 }
 
 static int __init init_kprobes(void)

[PATCH 9/9] Kprobes: Use RCU for (un)register synchronization - arch changes

[Ananth N Mavinakayanahalli]

From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

Changes to the arch kprobes infrastructure to take advantage of the locking
changes introduced by usage of RCU for synchronization. All handlers are
now run without any locks held, so they have to be re-entrant or provide
their own synchronization.

Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
---

 arch/i386/kernel/kprobes.c    |   22 +++++++---------------
 arch/ia64/kernel/kprobes.c    |   16 ++++++----------
 arch/ppc64/kernel/kprobes.c   |   24 ++++++------------------
 arch/sparc64/kernel/kprobes.c |   14 ++------------
 arch/x86_64/kernel/kprobes.c  |   25 ++++++-------------------
 5 files changed, 27 insertions(+), 74 deletions(-)

Index: linux-2.6.14-rc3/arch/i386/kernel/kprobes.c
===================================================================
--- linux-2.6.14-rc3.orig/arch/i386/kernel/kprobes.c	2005-10-05 16:08:13.000000000 -0400
+++ linux-2.6.14-rc3/arch/i386/kernel/kprobes.c	2005-10-05 16:08:48.000000000 -0400
@@ -31,7 +31,6 @@
 #include <linux/config.h>
 #include <linux/kprobes.h>
 #include <linux/ptrace.h>
-#include <linux/spinlock.h>
 #include <linux/preempt.h>
 #include <asm/cacheflush.h>
 #include <asm/kdebug.h>
@@ -123,6 +122,7 @@ static inline void prepare_singlestep(st
 		regs->eip = (unsigned long)&p->ainsn.insn;
 }
 
+/* Called with kretprobe_lock held */
 void __kprobes arch_prepare_kretprobe(struct kretprobe *rp,
 				      struct pt_regs *regs)
 {
@@ -168,15 +168,12 @@ static int __kprobes kprobe_handler(stru
 	}
 	/* Check we're not actually recursing */
 	if (kprobe_running()) {
-		/* We *are* holding lock here, so this is safe.
-		   Disarm the probe we just hit, and ignore it. */
 		p = get_kprobe(addr);
 		if (p) {
 			if (kcb->kprobe_status == KPROBE_HIT_SS &&
 				*p->ainsn.insn == BREAKPOINT_INSTRUCTION) {
 				regs->eflags &= ~TF_MASK;
 				regs->eflags |= kcb->kprobe_saved_eflags;
-				unlock_kprobes();
 				goto no_kprobe;
 			}
 			/* We have reentered the kprobe_handler(), since
@@ -197,14 +194,11 @@ static int __kprobes kprobe_handler(stru
 				goto ss_probe;
 			}
 		}
-		/* If it's not ours, can't be delete race, (we hold lock). */
 		goto no_kprobe;
 	}
 
-	lock_kprobes();
 	p = get_kprobe(addr);
 	if (!p) {
-		unlock_kprobes();
 		if (regs->eflags & VM_MASK) {
 			/* We are in virtual-8086 mode. Return 0 */
 			goto no_kprobe;
@@ -268,9 +262,10 @@ int __kprobes trampoline_probe_handler(s
         struct kretprobe_instance *ri = NULL;
         struct hlist_head *head;
         struct hlist_node *node, *tmp;
-	unsigned long orig_ret_address = 0;
+	unsigned long flags, orig_ret_address = 0;
 	unsigned long trampoline_address =(unsigned long)&kretprobe_trampoline;
 
+	spin_lock_irqsave(&kretprobe_lock, flags);
         head = kretprobe_inst_table_head(current);
 
 	/*
@@ -310,7 +305,7 @@ int __kprobes trampoline_probe_handler(s
 	regs->eip = orig_ret_address;
 
 	reset_current_kprobe();
-	unlock_kprobes();
+	spin_unlock_irqrestore(&kretprobe_lock, flags);
 	preempt_enable_no_resched();
 
         /*
@@ -395,7 +390,7 @@ static void __kprobes resume_execution(s
 
 /*
  * Interrupts are disabled on entry as trap1 is an interrupt gate and they
- * remain disabled thoroughout this function.  And we hold kprobe lock.
+ * remain disabled thoroughout this function.
  */
 static inline int post_kprobe_handler(struct pt_regs *regs)
 {
@@ -419,7 +414,6 @@ static inline int post_kprobe_handler(st
 		goto out;
 	}
 	reset_current_kprobe();
-	unlock_kprobes();
 out:
 	preempt_enable_no_resched();
 
@@ -434,7 +428,6 @@ out:
 	return 1;
 }
 
-/* Interrupts disabled, kprobe_lock held. */
 static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
 {
 	struct kprobe *cur = kprobe_running();
@@ -448,7 +441,6 @@ static inline int kprobe_fault_handler(s
 		regs->eflags |= kcb->kprobe_old_eflags;
 
 		reset_current_kprobe();
-		unlock_kprobes();
 		preempt_enable_no_resched();
 	}
 	return 0;
@@ -463,7 +455,7 @@ int __kprobes kprobe_exceptions_notify(s
 	struct die_args *args = (struct die_args *)data;
 	int ret = NOTIFY_DONE;
 
-	preempt_disable();
+	rcu_read_lock();
 	switch (val) {
 	case DIE_INT3:
 		if (kprobe_handler(args->regs))
@@ -482,7 +474,7 @@ int __kprobes kprobe_exceptions_notify(s
 	default:
 		break;
 	}
-	preempt_enable();
+	rcu_read_unlock();
 	return ret;
 }
 
Index: linux-2.6.14-rc3/arch/ia64/kernel/kprobes.c
===================================================================
--- linux-2.6.14-rc3.orig/arch/ia64/kernel/kprobes.c	2005-10-05 16:08:14.000000000 -0400
+++ linux-2.6.14-rc3/arch/ia64/kernel/kprobes.c	2005-10-05 16:08:48.000000000 -0400
@@ -26,7 +26,6 @@
 #include <linux/config.h>
 #include <linux/kprobes.h>
 #include <linux/ptrace.h>
-#include <linux/spinlock.h>
 #include <linux/string.h>
 #include <linux/slab.h>
 #include <linux/preempt.h>
@@ -343,10 +342,11 @@ int __kprobes trampoline_probe_handler(s
 	struct kretprobe_instance *ri = NULL;
 	struct hlist_head *head;
 	struct hlist_node *node, *tmp;
-	unsigned long orig_ret_address = 0;
+	unsigned long flags, orig_ret_address = 0;
 	unsigned long trampoline_address =
 		((struct fnptr *)kretprobe_trampoline)->ip;
 
+	spin_lock_irqsave(&kretprobe_lock, flags);
         head = kretprobe_inst_table_head(current);
 
 	/*
@@ -386,7 +386,7 @@ int __kprobes trampoline_probe_handler(s
 	regs->cr_iip = orig_ret_address;
 
 	reset_current_kprobe();
-	unlock_kprobes();
+	spin_unlock_irqrestore(&kretprobe_lock, flags);
 	preempt_enable_no_resched();
 
         /*
@@ -397,6 +397,7 @@ int __kprobes trampoline_probe_handler(s
         return 1;
 }
 
+/* Called with kretprobe_lock held */
 void __kprobes arch_prepare_kretprobe(struct kretprobe *rp,
 				      struct pt_regs *regs)
 {
@@ -612,7 +613,6 @@ static int __kprobes pre_kprobes_handler
 			if ((kcb->kprobe_status == KPROBE_HIT_SS) &&
 	 		     (p->ainsn.inst_flag == INST_FLAG_BREAK_INST)) {
   				ia64_psr(regs)->ss = 0;
-				unlock_kprobes();
 				goto no_kprobe;
 			}
 			/* We have reentered the pre_kprobe_handler(), since
@@ -641,10 +641,8 @@ static int __kprobes pre_kprobes_handler
 		}
 	}
 
-	lock_kprobes();
 	p = get_kprobe(addr);
 	if (!p) {
-		unlock_kprobes();
 		if (!is_ia64_break_inst(regs)) {
 			/*
 			 * The breakpoint instruction was removed right
@@ -707,7 +705,6 @@ static int __kprobes post_kprobes_handle
 		goto out;
 	}
 	reset_current_kprobe();
-	unlock_kprobes();
 
 out:
 	preempt_enable_no_resched();
@@ -728,7 +725,6 @@ static int __kprobes kprobes_fault_handl
 	if (kcb->kprobe_status & KPROBE_HIT_SS) {
 		resume_execution(cur, regs);
 		reset_current_kprobe();
-		unlock_kprobes();
 		preempt_enable_no_resched();
 	}
 
@@ -741,7 +737,7 @@ int __kprobes kprobe_exceptions_notify(s
 	struct die_args *args = (struct die_args *)data;
 	int ret = NOTIFY_DONE;
 
-	preempt_disable();
+	rcu_read_lock();
 	switch(val) {
 	case DIE_BREAK:
 		if (pre_kprobes_handler(args))
@@ -757,7 +753,7 @@ int __kprobes kprobe_exceptions_notify(s
 	default:
 		break;
 	}
-	preempt_enable();
+	rcu_read_unlock();
 	return ret;
 }
 
Index: linux-2.6.14-rc3/arch/ppc64/kernel/kprobes.c
===================================================================
--- linux-2.6.14-rc3.orig/arch/ppc64/kernel/kprobes.c	2005-10-05 16:08:15.000000000 -0400
+++ linux-2.6.14-rc3/arch/ppc64/kernel/kprobes.c	2005-10-05 16:08:48.000000000 -0400
@@ -30,7 +30,6 @@
 #include <linux/config.h>
 #include <linux/kprobes.h>
 #include <linux/ptrace.h>
-#include <linux/spinlock.h>
 #include <linux/preempt.h>
 #include <asm/cacheflush.h>
 #include <asm/kdebug.h>
@@ -125,6 +124,7 @@ static inline void set_current_kprobe(st
 	kcb->kprobe_saved_msr = regs->msr;
 }
 
+/* Called with kretprobe_lock held */
 void __kprobes arch_prepare_kretprobe(struct kretprobe *rp,
 				      struct pt_regs *regs)
 {
@@ -152,8 +152,6 @@ static inline int kprobe_handler(struct 
 
 	/* Check we're not actually recursing */
 	if (kprobe_running()) {
-		/* We *are* holding lock here, so this is safe.
-		   Disarm the probe we just hit, and ignore it. */
 		p = get_kprobe(addr);
 		if (p) {
 			kprobe_opcode_t insn = *p->ainsn.insn;
@@ -161,7 +159,6 @@ static inline int kprobe_handler(struct 
 					is_trap(insn)) {
 				regs->msr &= ~MSR_SE;
 				regs->msr |= kcb->kprobe_saved_msr;
-				unlock_kprobes();
 				goto no_kprobe;
 			}
 			/* We have reentered the kprobe_handler(), since
@@ -183,14 +180,11 @@ static inline int kprobe_handler(struct 
 				goto ss_probe;
 			}
 		}
-		/* If it's not ours, can't be delete race, (we hold lock). */
 		goto no_kprobe;
 	}
 
-	lock_kprobes();
 	p = get_kprobe(addr);
 	if (!p) {
-		unlock_kprobes();
 		if (*addr != BREAKPOINT_INSTRUCTION) {
 			/*
 			 * PowerPC has multiple variants of the "trap"
@@ -254,9 +248,10 @@ int __kprobes trampoline_probe_handler(s
         struct kretprobe_instance *ri = NULL;
         struct hlist_head *head;
         struct hlist_node *node, *tmp;
-	unsigned long orig_ret_address = 0;
+	unsigned long flags, orig_ret_address = 0;
 	unsigned long trampoline_address =(unsigned long)&kretprobe_trampoline;
 
+	spin_lock_irqsave(&kretprobe_lock, flags);
         head = kretprobe_inst_table_head(current);
 
 	/*
@@ -296,7 +291,7 @@ int __kprobes trampoline_probe_handler(s
 	regs->nip = orig_ret_address;
 
 	reset_current_kprobe();
-	unlock_kprobes();
+	spin_unlock_irqrestore(&kretprobe_lock, flags);
 	preempt_enable_no_resched();
 
         /*
@@ -348,7 +343,6 @@ static inline int post_kprobe_handler(st
 		goto out;
 	}
 	reset_current_kprobe();
-	unlock_kprobes();
 out:
 	preempt_enable_no_resched();
 
@@ -363,7 +357,6 @@ out:
 	return 1;
 }
 
-/* Interrupts disabled, kprobe_lock held. */
 static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
 {
 	struct kprobe *cur = kprobe_running();
@@ -378,7 +371,6 @@ static inline int kprobe_fault_handler(s
 		regs->msr |= kcb->kprobe_saved_msr;
 
 		reset_current_kprobe();
-		unlock_kprobes();
 		preempt_enable_no_resched();
 	}
 	return 0;
@@ -393,11 +385,7 @@ int __kprobes kprobe_exceptions_notify(s
 	struct die_args *args = (struct die_args *)data;
 	int ret = NOTIFY_DONE;
 
-	/*
-	 * Interrupts are not disabled here.  We need to disable
-	 * preemption, because kprobe_running() uses smp_processor_id().
-	 */
-	preempt_disable();
+	rcu_read_lock();
 	switch (val) {
 	case DIE_BPT:
 		if (kprobe_handler(args->regs))
@@ -416,7 +404,7 @@ int __kprobes kprobe_exceptions_notify(s
 	default:
 		break;
 	}
-	preempt_enable_no_resched();
+	rcu_read_unlock();
 	return ret;
 }
 
Index: linux-2.6.14-rc3/arch/sparc64/kernel/kprobes.c
===================================================================
--- linux-2.6.14-rc3.orig/arch/sparc64/kernel/kprobes.c	2005-10-05 16:08:15.000000000 -0400
+++ linux-2.6.14-rc3/arch/sparc64/kernel/kprobes.c	2005-10-05 16:08:48.000000000 -0400
@@ -116,15 +116,11 @@ static int __kprobes kprobe_handler(stru
 	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 
 	if (kprobe_running()) {
-		/* We *are* holding lock here, so this is safe.
-		 * Disarm the probe we just hit, and ignore it.
-		 */
 		p = get_kprobe(addr);
 		if (p) {
 			if (kcb->kprobe_status == KPROBE_HIT_SS) {
 				regs->tstate = ((regs->tstate & ~TSTATE_PIL) |
 					kcb->kprobe_orig_tstate_pil);
-				unlock_kprobes();
 				goto no_kprobe;
 			}
 			/* We have reentered the kprobe_handler(), since
@@ -144,14 +140,11 @@ static int __kprobes kprobe_handler(stru
 			if (p->break_handler && p->break_handler(p, regs))
 				goto ss_probe;
 		}
-		/* If it's not ours, can't be delete race, (we hold lock). */
 		goto no_kprobe;
 	}
 
-	lock_kprobes();
 	p = get_kprobe(addr);
 	if (!p) {
-		unlock_kprobes();
 		if (*(u32 *)addr != BREAKPOINT_INSTRUCTION) {
 			/*
 			 * The breakpoint instruction was removed right
@@ -296,14 +289,12 @@ static inline int post_kprobe_handler(st
 		goto out;
 	}
 	reset_current_kprobe();
-	unlock_kprobes();
 out:
 	preempt_enable_no_resched();
 
 	return 1;
 }
 
-/* Interrupts disabled, kprobe_lock held. */
 static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
 {
 	struct kprobe *cur = kprobe_running();
@@ -316,7 +307,6 @@ static inline int kprobe_fault_handler(s
 		resume_execution(cur, regs, kcb);
 
 		reset_current_kprobe();
-		unlock_kprobes();
 		preempt_enable_no_resched();
 	}
 	return 0;
@@ -331,7 +321,7 @@ int __kprobes kprobe_exceptions_notify(s
 	struct die_args *args = (struct die_args *)data;
 	int ret = NOTIFY_DONE;
 
-	preempt_disable();
+	rcu_read_lock();
 	switch (val) {
 	case DIE_DEBUG:
 		if (kprobe_handler(args->regs))
@@ -350,7 +340,7 @@ int __kprobes kprobe_exceptions_notify(s
 	default:
 		break;
 	}
-	preempt_enable();
+	rcu_read_unlock();
 	return ret;
 }
 
Index: linux-2.6.14-rc3/arch/x86_64/kernel/kprobes.c
===================================================================
--- linux-2.6.14-rc3.orig/arch/x86_64/kernel/kprobes.c	2005-10-05 16:08:33.000000000 -0400
+++ linux-2.6.14-rc3/arch/x86_64/kernel/kprobes.c	2005-10-05 16:08:48.000000000 -0400
@@ -34,7 +34,6 @@
 #include <linux/config.h>
 #include <linux/kprobes.h>
 #include <linux/ptrace.h>
-#include <linux/spinlock.h>
 #include <linux/string.h>
 #include <linux/slab.h>
 #include <linux/preempt.h>
@@ -266,6 +265,7 @@ static void __kprobes prepare_singlestep
 		regs->rip = (unsigned long)p->ainsn.insn;
 }
 
+/* Called with kretprobe_lock held */
 void __kprobes arch_prepare_kretprobe(struct kretprobe *rp,
 				      struct pt_regs *regs)
 {
@@ -299,15 +299,12 @@ int __kprobes kprobe_handler(struct pt_r
 
 	/* Check we're not actually recursing */
 	if (kprobe_running()) {
-		/* We *are* holding lock here, so this is safe.
-		   Disarm the probe we just hit, and ignore it. */
 		p = get_kprobe(addr);
 		if (p) {
 			if (kcb->kprobe_status == KPROBE_HIT_SS &&
 				*p->ainsn.insn == BREAKPOINT_INSTRUCTION) {
 				regs->eflags &= ~TF_MASK;
 				regs->eflags |= kcb->kprobe_saved_rflags;
-				unlock_kprobes();
 				goto no_kprobe;
 			} else if (kcb->kprobe_status == KPROBE_HIT_SSDONE) {
 				/* TODO: Provide re-entrancy from
@@ -340,14 +337,11 @@ int __kprobes kprobe_handler(struct pt_r
 				goto ss_probe;
 			}
 		}
-		/* If it's not ours, can't be delete race, (we hold lock). */
 		goto no_kprobe;
 	}
 
-	lock_kprobes();
 	p = get_kprobe(addr);
 	if (!p) {
-		unlock_kprobes();
 		if (*addr != BREAKPOINT_INSTRUCTION) {
 			/*
 			 * The breakpoint instruction was removed right
@@ -406,9 +400,10 @@ int __kprobes trampoline_probe_handler(s
         struct kretprobe_instance *ri = NULL;
         struct hlist_head *head;
         struct hlist_node *node, *tmp;
-	unsigned long orig_ret_address = 0;
+	unsigned long flags, orig_ret_address = 0;
 	unsigned long trampoline_address =(unsigned long)&kretprobe_trampoline;
 
+	spin_lock_irqsave(&kretprobe_lock, flags);
         head = kretprobe_inst_table_head(current);
 
 	/*
@@ -448,7 +443,7 @@ int __kprobes trampoline_probe_handler(s
 	regs->rip = orig_ret_address;
 
 	reset_current_kprobe();
-	unlock_kprobes();
+	spin_unlock_irqrestore(&kretprobe_lock, flags);
 	preempt_enable_no_resched();
 
         /*
@@ -536,10 +531,6 @@ static void __kprobes resume_execution(s
 	}
 }
 
-/*
- * Interrupts are disabled on entry as trap1 is an interrupt gate and they
- * remain disabled thoroughout this function.  And we hold kprobe lock.
- */
 int __kprobes post_kprobe_handler(struct pt_regs *regs)
 {
 	struct kprobe *cur = kprobe_running();
@@ -560,8 +551,6 @@ int __kprobes post_kprobe_handler(struct
 	if (kcb->kprobe_status == KPROBE_REENTER) {
 		restore_previous_kprobe(kcb);
 		goto out;
-	} else {
-		unlock_kprobes();
 	}
 	reset_current_kprobe();
 out:
@@ -578,7 +567,6 @@ out:
 	return 1;
 }
 
-/* Interrupts disabled, kprobe_lock held. */
 int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr)
 {
 	struct kprobe *cur = kprobe_running();
@@ -592,7 +580,6 @@ int __kprobes kprobe_fault_handler(struc
 		regs->eflags |= kcb->kprobe_old_rflags;
 
 		reset_current_kprobe();
-		unlock_kprobes();
 		preempt_enable_no_resched();
 	}
 	return 0;
@@ -607,7 +594,7 @@ int __kprobes kprobe_exceptions_notify(s
 	struct die_args *args = (struct die_args *)data;
 	int ret = NOTIFY_DONE;
 
-	preempt_disable();
+	rcu_read_lock();
 	switch (val) {
 	case DIE_INT3:
 		if (kprobe_handler(args->regs))
@@ -626,7 +613,7 @@ int __kprobes kprobe_exceptions_notify(s
 	default:
 		break;
 	}
-	preempt_enable();
+	rcu_read_unlock();
 	return ret;
 }
 

Re: [PATCH 9/9] Kprobes: Use RCU for (un)register synchronization - arch changes

[Paul E. McKenney]

On Mon, Oct 10, 2005 at 10:48:13AM -0400, Ananth N Mavinakayanahalli wrote:
> From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> 
> Changes to the arch kprobes infrastructure to take advantage of the locking
> changes introduced by usage of RCU for synchronization. All handlers are
> now run without any locks held, so they have to be re-entrant or provide
> their own synchronization.

And a few very similar questions here as well...

							Thanx, Paul

> Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
> ---
> 
>  arch/i386/kernel/kprobes.c    |   22 +++++++---------------
>  arch/ia64/kernel/kprobes.c    |   16 ++++++----------
>  arch/ppc64/kernel/kprobes.c   |   24 ++++++------------------
>  arch/sparc64/kernel/kprobes.c |   14 ++------------
>  arch/x86_64/kernel/kprobes.c  |   25 ++++++-------------------
>  5 files changed, 27 insertions(+), 74 deletions(-)
> 
> Index: linux-2.6.14-rc3/arch/i386/kernel/kprobes.c
> ===================================================================
> --- linux-2.6.14-rc3.orig/arch/i386/kernel/kprobes.c	2005-10-05 16:08:13.000000000 -0400
> +++ linux-2.6.14-rc3/arch/i386/kernel/kprobes.c	2005-10-05 16:08:48.000000000 -0400
> @@ -31,7 +31,6 @@
>  #include <linux/config.h>
>  #include <linux/kprobes.h>
>  #include <linux/ptrace.h>
> -#include <linux/spinlock.h>
>  #include <linux/preempt.h>
>  #include <asm/cacheflush.h>
>  #include <asm/kdebug.h>
> @@ -123,6 +122,7 @@ static inline void prepare_singlestep(st
>  		regs->eip = (unsigned long)&p->ainsn.insn;
>  }
>  
> +/* Called with kretprobe_lock held */
>  void __kprobes arch_prepare_kretprobe(struct kretprobe *rp,
>  				      struct pt_regs *regs)
>  {
> @@ -168,15 +168,12 @@ static int __kprobes kprobe_handler(stru
>  	}
>  	/* Check we're not actually recursing */
>  	if (kprobe_running()) {
> -		/* We *are* holding lock here, so this is safe.
> -		   Disarm the probe we just hit, and ignore it. */
>  		p = get_kprobe(addr);
>  		if (p) {
>  			if (kcb->kprobe_status == KPROBE_HIT_SS &&
>  				*p->ainsn.insn == BREAKPOINT_INSTRUCTION) {
>  				regs->eflags &= ~TF_MASK;
>  				regs->eflags |= kcb->kprobe_saved_eflags;
> -				unlock_kprobes();
>  				goto no_kprobe;
>  			}
>  			/* We have reentered the kprobe_handler(), since
> @@ -197,14 +194,11 @@ static int __kprobes kprobe_handler(stru
>  				goto ss_probe;
>  			}
>  		}
> -		/* If it's not ours, can't be delete race, (we hold lock). */
>  		goto no_kprobe;
>  	}
>  
> -	lock_kprobes();
>  	p = get_kprobe(addr);
>  	if (!p) {
> -		unlock_kprobes();
>  		if (regs->eflags & VM_MASK) {
>  			/* We are in virtual-8086 mode. Return 0 */
>  			goto no_kprobe;
> @@ -268,9 +262,10 @@ int __kprobes trampoline_probe_handler(s
>          struct kretprobe_instance *ri = NULL;
>          struct hlist_head *head;
>          struct hlist_node *node, *tmp;
> -	unsigned long orig_ret_address = 0;
> +	unsigned long flags, orig_ret_address = 0;
>  	unsigned long trampoline_address =(unsigned long)&kretprobe_trampoline;
>  
> +	spin_lock_irqsave(&kretprobe_lock, flags);
>          head = kretprobe_inst_table_head(current);
>  
>  	/*
> @@ -310,7 +305,7 @@ int __kprobes trampoline_probe_handler(s
>  	regs->eip = orig_ret_address;
>  
>  	reset_current_kprobe();
> -	unlock_kprobes();
> +	spin_unlock_irqrestore(&kretprobe_lock, flags);
>  	preempt_enable_no_resched();
>  
>          /*
> @@ -395,7 +390,7 @@ static void __kprobes resume_execution(s
>  
>  /*
>   * Interrupts are disabled on entry as trap1 is an interrupt gate and they
> - * remain disabled thoroughout this function.  And we hold kprobe lock.
> + * remain disabled thoroughout this function.
>   */
>  static inline int post_kprobe_handler(struct pt_regs *regs)
>  {
> @@ -419,7 +414,6 @@ static inline int post_kprobe_handler(st
>  		goto out;
>  	}
>  	reset_current_kprobe();
> -	unlock_kprobes();
>  out:
>  	preempt_enable_no_resched();
>  
> @@ -434,7 +428,6 @@ out:
>  	return 1;
>  }
>  
> -/* Interrupts disabled, kprobe_lock held. */
>  static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
>  {
>  	struct kprobe *cur = kprobe_running();
> @@ -448,7 +441,6 @@ static inline int kprobe_fault_handler(s
>  		regs->eflags |= kcb->kprobe_old_eflags;
>  
>  		reset_current_kprobe();
> -		unlock_kprobes();
>  		preempt_enable_no_resched();
>  	}
>  	return 0;
> @@ -463,7 +455,7 @@ int __kprobes kprobe_exceptions_notify(s
>  	struct die_args *args = (struct die_args *)data;
>  	int ret = NOTIFY_DONE;
>  
> -	preempt_disable();
> +	rcu_read_lock();

If synchronize_sched() is used on the update side, this needs to
remain preempt_disable() rather than rcu_read_lock().

>  	switch (val) {
>  	case DIE_INT3:
>  		if (kprobe_handler(args->regs))
> @@ -482,7 +474,7 @@ int __kprobes kprobe_exceptions_notify(s
>  	default:
>  		break;
>  	}
> -	preempt_enable();
> +	rcu_read_unlock();
>  	return ret;
>  }
>  
> Index: linux-2.6.14-rc3/arch/ia64/kernel/kprobes.c
> ===================================================================
> --- linux-2.6.14-rc3.orig/arch/ia64/kernel/kprobes.c	2005-10-05 16:08:14.000000000 -0400
> +++ linux-2.6.14-rc3/arch/ia64/kernel/kprobes.c	2005-10-05 16:08:48.000000000 -0400
> @@ -26,7 +26,6 @@
>  #include <linux/config.h>
>  #include <linux/kprobes.h>
>  #include <linux/ptrace.h>
> -#include <linux/spinlock.h>
>  #include <linux/string.h>
>  #include <linux/slab.h>
>  #include <linux/preempt.h>
> @@ -343,10 +342,11 @@ int __kprobes trampoline_probe_handler(s
>  	struct kretprobe_instance *ri = NULL;
>  	struct hlist_head *head;
>  	struct hlist_node *node, *tmp;
> -	unsigned long orig_ret_address = 0;
> +	unsigned long flags, orig_ret_address = 0;
>  	unsigned long trampoline_address =
>  		((struct fnptr *)kretprobe_trampoline)->ip;
>  
> +	spin_lock_irqsave(&kretprobe_lock, flags);
>          head = kretprobe_inst_table_head(current);
>  
>  	/*
> @@ -386,7 +386,7 @@ int __kprobes trampoline_probe_handler(s
>  	regs->cr_iip = orig_ret_address;
>  
>  	reset_current_kprobe();
> -	unlock_kprobes();
> +	spin_unlock_irqrestore(&kretprobe_lock, flags);
>  	preempt_enable_no_resched();
>  
>          /*
> @@ -397,6 +397,7 @@ int __kprobes trampoline_probe_handler(s
>          return 1;
>  }
>  
> +/* Called with kretprobe_lock held */
>  void __kprobes arch_prepare_kretprobe(struct kretprobe *rp,
>  				      struct pt_regs *regs)
>  {
> @@ -612,7 +613,6 @@ static int __kprobes pre_kprobes_handler
>  			if ((kcb->kprobe_status == KPROBE_HIT_SS) &&
>  	 		     (p->ainsn.inst_flag == INST_FLAG_BREAK_INST)) {
>    				ia64_psr(regs)->ss = 0;
> -				unlock_kprobes();
>  				goto no_kprobe;
>  			}
>  			/* We have reentered the pre_kprobe_handler(), since
> @@ -641,10 +641,8 @@ static int __kprobes pre_kprobes_handler
>  		}
>  	}
>  
> -	lock_kprobes();
>  	p = get_kprobe(addr);
>  	if (!p) {
> -		unlock_kprobes();
>  		if (!is_ia64_break_inst(regs)) {
>  			/*
>  			 * The breakpoint instruction was removed right
> @@ -707,7 +705,6 @@ static int __kprobes post_kprobes_handle
>  		goto out;
>  	}
>  	reset_current_kprobe();
> -	unlock_kprobes();
>  
>  out:
>  	preempt_enable_no_resched();
> @@ -728,7 +725,6 @@ static int __kprobes kprobes_fault_handl
>  	if (kcb->kprobe_status & KPROBE_HIT_SS) {
>  		resume_execution(cur, regs);
>  		reset_current_kprobe();
> -		unlock_kprobes();
>  		preempt_enable_no_resched();
>  	}
>  
> @@ -741,7 +737,7 @@ int __kprobes kprobe_exceptions_notify(s
>  	struct die_args *args = (struct die_args *)data;
>  	int ret = NOTIFY_DONE;
>  
> -	preempt_disable();
> +	rcu_read_lock();

Ditto here...

>  	switch(val) {
>  	case DIE_BREAK:
>  		if (pre_kprobes_handler(args))
> @@ -757,7 +753,7 @@ int __kprobes kprobe_exceptions_notify(s
>  	default:
>  		break;
>  	}
> -	preempt_enable();
> +	rcu_read_unlock();
>  	return ret;
>  }
>  
> Index: linux-2.6.14-rc3/arch/ppc64/kernel/kprobes.c
> ===================================================================
> --- linux-2.6.14-rc3.orig/arch/ppc64/kernel/kprobes.c	2005-10-05 16:08:15.000000000 -0400
> +++ linux-2.6.14-rc3/arch/ppc64/kernel/kprobes.c	2005-10-05 16:08:48.000000000 -0400
> @@ -30,7 +30,6 @@
>  #include <linux/config.h>
>  #include <linux/kprobes.h>
>  #include <linux/ptrace.h>
> -#include <linux/spinlock.h>
>  #include <linux/preempt.h>
>  #include <asm/cacheflush.h>
>  #include <asm/kdebug.h>
> @@ -125,6 +124,7 @@ static inline void set_current_kprobe(st
>  	kcb->kprobe_saved_msr = regs->msr;
>  }
>  
> +/* Called with kretprobe_lock held */
>  void __kprobes arch_prepare_kretprobe(struct kretprobe *rp,
>  				      struct pt_regs *regs)
>  {
> @@ -152,8 +152,6 @@ static inline int kprobe_handler(struct 
>  
>  	/* Check we're not actually recursing */
>  	if (kprobe_running()) {
> -		/* We *are* holding lock here, so this is safe.
> -		   Disarm the probe we just hit, and ignore it. */
>  		p = get_kprobe(addr);
>  		if (p) {
>  			kprobe_opcode_t insn = *p->ainsn.insn;
> @@ -161,7 +159,6 @@ static inline int kprobe_handler(struct 
>  					is_trap(insn)) {
>  				regs->msr &= ~MSR_SE;
>  				regs->msr |= kcb->kprobe_saved_msr;
> -				unlock_kprobes();
>  				goto no_kprobe;
>  			}
>  			/* We have reentered the kprobe_handler(), since
> @@ -183,14 +180,11 @@ static inline int kprobe_handler(struct 
>  				goto ss_probe;
>  			}
>  		}
> -		/* If it's not ours, can't be delete race, (we hold lock). */
>  		goto no_kprobe;
>  	}
>  
> -	lock_kprobes();
>  	p = get_kprobe(addr);
>  	if (!p) {
> -		unlock_kprobes();
>  		if (*addr != BREAKPOINT_INSTRUCTION) {
>  			/*
>  			 * PowerPC has multiple variants of the "trap"
> @@ -254,9 +248,10 @@ int __kprobes trampoline_probe_handler(s
>          struct kretprobe_instance *ri = NULL;
>          struct hlist_head *head;
>          struct hlist_node *node, *tmp;
> -	unsigned long orig_ret_address = 0;
> +	unsigned long flags, orig_ret_address = 0;
>  	unsigned long trampoline_address =(unsigned long)&kretprobe_trampoline;
>  
> +	spin_lock_irqsave(&kretprobe_lock, flags);
>          head = kretprobe_inst_table_head(current);
>  
>  	/*
> @@ -296,7 +291,7 @@ int __kprobes trampoline_probe_handler(s
>  	regs->nip = orig_ret_address;
>  
>  	reset_current_kprobe();
> -	unlock_kprobes();
> +	spin_unlock_irqrestore(&kretprobe_lock, flags);
>  	preempt_enable_no_resched();
>  
>          /*
> @@ -348,7 +343,6 @@ static inline int post_kprobe_handler(st
>  		goto out;
>  	}
>  	reset_current_kprobe();
> -	unlock_kprobes();
>  out:
>  	preempt_enable_no_resched();
>  
> @@ -363,7 +357,6 @@ out:
>  	return 1;
>  }
>  
> -/* Interrupts disabled, kprobe_lock held. */
>  static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
>  {
>  	struct kprobe *cur = kprobe_running();
> @@ -378,7 +371,6 @@ static inline int kprobe_fault_handler(s
>  		regs->msr |= kcb->kprobe_saved_msr;
>  
>  		reset_current_kprobe();
> -		unlock_kprobes();
>  		preempt_enable_no_resched();
>  	}
>  	return 0;
> @@ -393,11 +385,7 @@ int __kprobes kprobe_exceptions_notify(s
>  	struct die_args *args = (struct die_args *)data;
>  	int ret = NOTIFY_DONE;
>  
> -	/*
> -	 * Interrupts are not disabled here.  We need to disable
> -	 * preemption, because kprobe_running() uses smp_processor_id().
> -	 */
> -	preempt_disable();
> +	rcu_read_lock();

And here...

>  	switch (val) {
>  	case DIE_BPT:
>  		if (kprobe_handler(args->regs))
> @@ -416,7 +404,7 @@ int __kprobes kprobe_exceptions_notify(s
>  	default:
>  		break;
>  	}
> -	preempt_enable_no_resched();
> +	rcu_read_unlock();
>  	return ret;
>  }
>  
> Index: linux-2.6.14-rc3/arch/sparc64/kernel/kprobes.c
> ===================================================================
> --- linux-2.6.14-rc3.orig/arch/sparc64/kernel/kprobes.c	2005-10-05 16:08:15.000000000 -0400
> +++ linux-2.6.14-rc3/arch/sparc64/kernel/kprobes.c	2005-10-05 16:08:48.000000000 -0400
> @@ -116,15 +116,11 @@ static int __kprobes kprobe_handler(stru
>  	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
>  
>  	if (kprobe_running()) {
> -		/* We *are* holding lock here, so this is safe.
> -		 * Disarm the probe we just hit, and ignore it.
> -		 */
>  		p = get_kprobe(addr);
>  		if (p) {
>  			if (kcb->kprobe_status == KPROBE_HIT_SS) {
>  				regs->tstate = ((regs->tstate & ~TSTATE_PIL) |
>  					kcb->kprobe_orig_tstate_pil);
> -				unlock_kprobes();
>  				goto no_kprobe;
>  			}
>  			/* We have reentered the kprobe_handler(), since
> @@ -144,14 +140,11 @@ static int __kprobes kprobe_handler(stru
>  			if (p->break_handler && p->break_handler(p, regs))
>  				goto ss_probe;
>  		}
> -		/* If it's not ours, can't be delete race, (we hold lock). */
>  		goto no_kprobe;
>  	}
>  
> -	lock_kprobes();
>  	p = get_kprobe(addr);
>  	if (!p) {
> -		unlock_kprobes();
>  		if (*(u32 *)addr != BREAKPOINT_INSTRUCTION) {
>  			/*
>  			 * The breakpoint instruction was removed right
> @@ -296,14 +289,12 @@ static inline int post_kprobe_handler(st
>  		goto out;
>  	}
>  	reset_current_kprobe();
> -	unlock_kprobes();
>  out:
>  	preempt_enable_no_resched();
>  
>  	return 1;
>  }
>  
> -/* Interrupts disabled, kprobe_lock held. */
>  static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
>  {
>  	struct kprobe *cur = kprobe_running();
> @@ -316,7 +307,6 @@ static inline int kprobe_fault_handler(s
>  		resume_execution(cur, regs, kcb);
>  
>  		reset_current_kprobe();
> -		unlock_kprobes();
>  		preempt_enable_no_resched();
>  	}
>  	return 0;
> @@ -331,7 +321,7 @@ int __kprobes kprobe_exceptions_notify(s
>  	struct die_args *args = (struct die_args *)data;
>  	int ret = NOTIFY_DONE;
>  
> -	preempt_disable();
> +	rcu_read_lock();

As well as here...

>  	switch (val) {
>  	case DIE_DEBUG:
>  		if (kprobe_handler(args->regs))
> @@ -350,7 +340,7 @@ int __kprobes kprobe_exceptions_notify(s
>  	default:
>  		break;
>  	}
> -	preempt_enable();
> +	rcu_read_unlock();
>  	return ret;
>  }
>  
> Index: linux-2.6.14-rc3/arch/x86_64/kernel/kprobes.c
> ===================================================================
> --- linux-2.6.14-rc3.orig/arch/x86_64/kernel/kprobes.c	2005-10-05 16:08:33.000000000 -0400
> +++ linux-2.6.14-rc3/arch/x86_64/kernel/kprobes.c	2005-10-05 16:08:48.000000000 -0400
> @@ -34,7 +34,6 @@
>  #include <linux/config.h>
>  #include <linux/kprobes.h>
>  #include <linux/ptrace.h>
> -#include <linux/spinlock.h>
>  #include <linux/string.h>
>  #include <linux/slab.h>
>  #include <linux/preempt.h>
> @@ -266,6 +265,7 @@ static void __kprobes prepare_singlestep
>  		regs->rip = (unsigned long)p->ainsn.insn;
>  }
>  
> +/* Called with kretprobe_lock held */
>  void __kprobes arch_prepare_kretprobe(struct kretprobe *rp,
>  				      struct pt_regs *regs)
>  {
> @@ -299,15 +299,12 @@ int __kprobes kprobe_handler(struct pt_r
>  
>  	/* Check we're not actually recursing */
>  	if (kprobe_running()) {
> -		/* We *are* holding lock here, so this is safe.
> -		   Disarm the probe we just hit, and ignore it. */
>  		p = get_kprobe(addr);
>  		if (p) {
>  			if (kcb->kprobe_status == KPROBE_HIT_SS &&
>  				*p->ainsn.insn == BREAKPOINT_INSTRUCTION) {
>  				regs->eflags &= ~TF_MASK;
>  				regs->eflags |= kcb->kprobe_saved_rflags;
> -				unlock_kprobes();
>  				goto no_kprobe;
>  			} else if (kcb->kprobe_status == KPROBE_HIT_SSDONE) {
>  				/* TODO: Provide re-entrancy from
> @@ -340,14 +337,11 @@ int __kprobes kprobe_handler(struct pt_r
>  				goto ss_probe;
>  			}
>  		}
> -		/* If it's not ours, can't be delete race, (we hold lock). */
>  		goto no_kprobe;
>  	}
>  
> -	lock_kprobes();
>  	p = get_kprobe(addr);
>  	if (!p) {
> -		unlock_kprobes();
>  		if (*addr != BREAKPOINT_INSTRUCTION) {
>  			/*
>  			 * The breakpoint instruction was removed right
> @@ -406,9 +400,10 @@ int __kprobes trampoline_probe_handler(s
>          struct kretprobe_instance *ri = NULL;
>          struct hlist_head *head;
>          struct hlist_node *node, *tmp;
> -	unsigned long orig_ret_address = 0;
> +	unsigned long flags, orig_ret_address = 0;
>  	unsigned long trampoline_address =(unsigned long)&kretprobe_trampoline;
>  
> +	spin_lock_irqsave(&kretprobe_lock, flags);
>          head = kretprobe_inst_table_head(current);
>  
>  	/*
> @@ -448,7 +443,7 @@ int __kprobes trampoline_probe_handler(s
>  	regs->rip = orig_ret_address;
>  
>  	reset_current_kprobe();
> -	unlock_kprobes();
> +	spin_unlock_irqrestore(&kretprobe_lock, flags);
>  	preempt_enable_no_resched();
>  
>          /*
> @@ -536,10 +531,6 @@ static void __kprobes resume_execution(s
>  	}
>  }
>  
> -/*
> - * Interrupts are disabled on entry as trap1 is an interrupt gate and they
> - * remain disabled thoroughout this function.  And we hold kprobe lock.
> - */
>  int __kprobes post_kprobe_handler(struct pt_regs *regs)
>  {
>  	struct kprobe *cur = kprobe_running();
> @@ -560,8 +551,6 @@ int __kprobes post_kprobe_handler(struct
>  	if (kcb->kprobe_status == KPROBE_REENTER) {
>  		restore_previous_kprobe(kcb);
>  		goto out;
> -	} else {
> -		unlock_kprobes();
>  	}
>  	reset_current_kprobe();
>  out:
> @@ -578,7 +567,6 @@ out:
>  	return 1;
>  }
>  
> -/* Interrupts disabled, kprobe_lock held. */
>  int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr)
>  {
>  	struct kprobe *cur = kprobe_running();
> @@ -592,7 +580,6 @@ int __kprobes kprobe_fault_handler(struc
>  		regs->eflags |= kcb->kprobe_old_rflags;
>  
>  		reset_current_kprobe();
> -		unlock_kprobes();
>  		preempt_enable_no_resched();
>  	}
>  	return 0;
> @@ -607,7 +594,7 @@ int __kprobes kprobe_exceptions_notify(s
>  	struct die_args *args = (struct die_args *)data;
>  	int ret = NOTIFY_DONE;
>  
> -	preempt_disable();
> +	rcu_read_lock();

As well as here yet again...

>  	switch (val) {
>  	case DIE_INT3:
>  		if (kprobe_handler(args->regs))
> @@ -626,7 +613,7 @@ int __kprobes kprobe_exceptions_notify(s
>  	default:
>  		break;
>  	}
> -	preempt_enable();
> +	rcu_read_unlock();
>  	return ret;
>  }
>  
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> 

Re: [PATCH 9/9] Kprobes: Use RCU for (un)register synchronization - arch changes

[Ananth N Mavinakayanahalli]

On Mon, Oct 17, 2005 at 10:49:30PM -0700, Paul E. McKenney wrote:
> On Mon, Oct 10, 2005 at 10:48:13AM -0400, Ananth N Mavinakayanahalli wrote:
> > From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> > 
> > Changes to the arch kprobes infrastructure to take advantage of the locking
> > changes introduced by usage of RCU for synchronization. All handlers are
> > now run without any locks held, so they have to be re-entrant or provide
> > their own synchronization.
> 
> And a few very similar questions here as well...

Replies inline...

Ananth
 
> 							Thanx, Paul
> 
> > Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> > Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
> > ---
> > 
> >  arch/i386/kernel/kprobes.c    |   22 +++++++---------------
> >  arch/ia64/kernel/kprobes.c    |   16 ++++++----------
> >  arch/ppc64/kernel/kprobes.c   |   24 ++++++------------------
> >  arch/sparc64/kernel/kprobes.c |   14 ++------------
> >  arch/x86_64/kernel/kprobes.c  |   25 ++++++-------------------
> >  5 files changed, 27 insertions(+), 74 deletions(-)
> > 
> > Index: linux-2.6.14-rc3/arch/i386/kernel/kprobes.c
> > ===================================================================
> > --- linux-2.6.14-rc3.orig/arch/i386/kernel/kprobes.c	2005-10-05 16:08:13.000000000 -0400
> > +++ linux-2.6.14-rc3/arch/i386/kernel/kprobes.c	2005-10-05 16:08:48.000000000 -0400
> > @@ -31,7 +31,6 @@
> >  #include <linux/config.h>
> >  #include <linux/kprobes.h>
> >  #include <linux/ptrace.h>
> > -#include <linux/spinlock.h>
> >  #include <linux/preempt.h>
> >  #include <asm/cacheflush.h>
> >  #include <asm/kdebug.h>
> > @@ -123,6 +122,7 @@ static inline void prepare_singlestep(st
> >  		regs->eip = (unsigned long)&p->ainsn.insn;
> >  }
> >  
> > +/* Called with kretprobe_lock held */
> >  void __kprobes arch_prepare_kretprobe(struct kretprobe *rp,
> >  				      struct pt_regs *regs)
> >  {
> > @@ -168,15 +168,12 @@ static int __kprobes kprobe_handler(stru
> >  	}
> >  	/* Check we're not actually recursing */
> >  	if (kprobe_running()) {
> > -		/* We *are* holding lock here, so this is safe.
> > -		   Disarm the probe we just hit, and ignore it. */
> >  		p = get_kprobe(addr);
> >  		if (p) {
> >  			if (kcb->kprobe_status == KPROBE_HIT_SS &&
> >  				*p->ainsn.insn == BREAKPOINT_INSTRUCTION) {
> >  				regs->eflags &= ~TF_MASK;
> >  				regs->eflags |= kcb->kprobe_saved_eflags;
> > -				unlock_kprobes();
> >  				goto no_kprobe;
> >  			}
> >  			/* We have reentered the kprobe_handler(), since
> > @@ -197,14 +194,11 @@ static int __kprobes kprobe_handler(stru
> >  				goto ss_probe;
> >  			}
> >  		}
> > -		/* If it's not ours, can't be delete race, (we hold lock). */
> >  		goto no_kprobe;
> >  	}
> >  
> > -	lock_kprobes();
> >  	p = get_kprobe(addr);
> >  	if (!p) {
> > -		unlock_kprobes();
> >  		if (regs->eflags & VM_MASK) {
> >  			/* We are in virtual-8086 mode. Return 0 */
> >  			goto no_kprobe;
> > @@ -268,9 +262,10 @@ int __kprobes trampoline_probe_handler(s
> >          struct kretprobe_instance *ri = NULL;
> >          struct hlist_head *head;
> >          struct hlist_node *node, *tmp;
> > -	unsigned long orig_ret_address = 0;
> > +	unsigned long flags, orig_ret_address = 0;
> >  	unsigned long trampoline_address =(unsigned long)&kretprobe_trampoline;
> >  
> > +	spin_lock_irqsave(&kretprobe_lock, flags);
> >          head = kretprobe_inst_table_head(current);
> >  
> >  	/*
> > @@ -310,7 +305,7 @@ int __kprobes trampoline_probe_handler(s
> >  	regs->eip = orig_ret_address;
> >  
> >  	reset_current_kprobe();
> > -	unlock_kprobes();
> > +	spin_unlock_irqrestore(&kretprobe_lock, flags);
> >  	preempt_enable_no_resched();
> >  
> >          /*
> > @@ -395,7 +390,7 @@ static void __kprobes resume_execution(s
> >  
> >  /*
> >   * Interrupts are disabled on entry as trap1 is an interrupt gate and they
> > - * remain disabled thoroughout this function.  And we hold kprobe lock.
> > + * remain disabled thoroughout this function.
> >   */
> >  static inline int post_kprobe_handler(struct pt_regs *regs)
> >  {
> > @@ -419,7 +414,6 @@ static inline int post_kprobe_handler(st
> >  		goto out;
> >  	}
> >  	reset_current_kprobe();
> > -	unlock_kprobes();
> >  out:
> >  	preempt_enable_no_resched();
> >  
> > @@ -434,7 +428,6 @@ out:
> >  	return 1;
> >  }
> >  
> > -/* Interrupts disabled, kprobe_lock held. */
> >  static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
> >  {
> >  	struct kprobe *cur = kprobe_running();
> > @@ -448,7 +441,6 @@ static inline int kprobe_fault_handler(s
> >  		regs->eflags |= kcb->kprobe_old_eflags;
> >  
> >  		reset_current_kprobe();
> > -		unlock_kprobes();
> >  		preempt_enable_no_resched();
> >  	}
> >  	return 0;
> > @@ -463,7 +455,7 @@ int __kprobes kprobe_exceptions_notify(s
> >  	struct die_args *args = (struct die_args *)data;
> >  	int ret = NOTIFY_DONE;
> >  
> > -	preempt_disable();
> > +	rcu_read_lock();
> 
> If synchronize_sched() is used on the update side, this needs to
> remain preempt_disable() rather than rcu_read_lock().

Kprobe handlers can't block/sleep. So the idea is to depend on a
schedule() event to ensure handlers have executed. This and the others
you have pointed out can surely be preempt_disable().

> >  	switch (val) {
> >  	case DIE_INT3:
> >  		if (kprobe_handler(args->regs))
> > @@ -482,7 +474,7 @@ int __kprobes kprobe_exceptions_notify(s
> >  	default:
> >  		break;
> >  	}
> > -	preempt_enable();
> > +	rcu_read_unlock();
> >  	return ret;
> >  }
> >  
> > Index: linux-2.6.14-rc3/arch/ia64/kernel/kprobes.c
> > ===================================================================
> > --- linux-2.6.14-rc3.orig/arch/ia64/kernel/kprobes.c	2005-10-05 16:08:14.000000000 -0400
> > +++ linux-2.6.14-rc3/arch/ia64/kernel/kprobes.c	2005-10-05 16:08:48.000000000 -0400
> > @@ -26,7 +26,6 @@
> >  #include <linux/config.h>
> >  #include <linux/kprobes.h>
> >  #include <linux/ptrace.h>
> > -#include <linux/spinlock.h>
> >  #include <linux/string.h>
> >  #include <linux/slab.h>
> >  #include <linux/preempt.h>
> > @@ -343,10 +342,11 @@ int __kprobes trampoline_probe_handler(s
> >  	struct kretprobe_instance *ri = NULL;
> >  	struct hlist_head *head;
> >  	struct hlist_node *node, *tmp;
> > -	unsigned long orig_ret_address = 0;
> > +	unsigned long flags, orig_ret_address = 0;
> >  	unsigned long trampoline_address =
> >  		((struct fnptr *)kretprobe_trampoline)->ip;
> >  
> > +	spin_lock_irqsave(&kretprobe_lock, flags);
> >          head = kretprobe_inst_table_head(current);
> >  
> >  	/*
> > @@ -386,7 +386,7 @@ int __kprobes trampoline_probe_handler(s
> >  	regs->cr_iip = orig_ret_address;
> >  
> >  	reset_current_kprobe();
> > -	unlock_kprobes();
> > +	spin_unlock_irqrestore(&kretprobe_lock, flags);
> >  	preempt_enable_no_resched();
> >  
> >          /*
> > @@ -397,6 +397,7 @@ int __kprobes trampoline_probe_handler(s
> >          return 1;
> >  }
> >  
> > +/* Called with kretprobe_lock held */
> >  void __kprobes arch_prepare_kretprobe(struct kretprobe *rp,
> >  				      struct pt_regs *regs)
> >  {
> > @@ -612,7 +613,6 @@ static int __kprobes pre_kprobes_handler
> >  			if ((kcb->kprobe_status == KPROBE_HIT_SS) &&
> >  	 		     (p->ainsn.inst_flag == INST_FLAG_BREAK_INST)) {
> >    				ia64_psr(regs)->ss = 0;
> > -				unlock_kprobes();
> >  				goto no_kprobe;
> >  			}
> >  			/* We have reentered the pre_kprobe_handler(), since
> > @@ -641,10 +641,8 @@ static int __kprobes pre_kprobes_handler
> >  		}
> >  	}
> >  
> > -	lock_kprobes();
> >  	p = get_kprobe(addr);
> >  	if (!p) {
> > -		unlock_kprobes();
> >  		if (!is_ia64_break_inst(regs)) {
> >  			/*
> >  			 * The breakpoint instruction was removed right
> > @@ -707,7 +705,6 @@ static int __kprobes post_kprobes_handle
> >  		goto out;
> >  	}
> >  	reset_current_kprobe();
> > -	unlock_kprobes();
> >  
> >  out:
> >  	preempt_enable_no_resched();
> > @@ -728,7 +725,6 @@ static int __kprobes kprobes_fault_handl
> >  	if (kcb->kprobe_status & KPROBE_HIT_SS) {
> >  		resume_execution(cur, regs);
> >  		reset_current_kprobe();
> > -		unlock_kprobes();
> >  		preempt_enable_no_resched();
> >  	}
> >  
> > @@ -741,7 +737,7 @@ int __kprobes kprobe_exceptions_notify(s
> >  	struct die_args *args = (struct die_args *)data;
> >  	int ret = NOTIFY_DONE;
> >  
> > -	preempt_disable();
> > +	rcu_read_lock();
> 
> Ditto here...
> 
> >  	switch(val) {
> >  	case DIE_BREAK:
> >  		if (pre_kprobes_handler(args))
> > @@ -757,7 +753,7 @@ int __kprobes kprobe_exceptions_notify(s
> >  	default:
> >  		break;
> >  	}
> > -	preempt_enable();
> > +	rcu_read_unlock();
> >  	return ret;
> >  }
> >  
> > Index: linux-2.6.14-rc3/arch/ppc64/kernel/kprobes.c
> > ===================================================================
> > --- linux-2.6.14-rc3.orig/arch/ppc64/kernel/kprobes.c	2005-10-05 16:08:15.000000000 -0400
> > +++ linux-2.6.14-rc3/arch/ppc64/kernel/kprobes.c	2005-10-05 16:08:48.000000000 -0400
> > @@ -30,7 +30,6 @@
> >  #include <linux/config.h>
> >  #include <linux/kprobes.h>
> >  #include <linux/ptrace.h>
> > -#include <linux/spinlock.h>
> >  #include <linux/preempt.h>
> >  #include <asm/cacheflush.h>
> >  #include <asm/kdebug.h>
> > @@ -125,6 +124,7 @@ static inline void set_current_kprobe(st
> >  	kcb->kprobe_saved_msr = regs->msr;
> >  }
> >  
> > +/* Called with kretprobe_lock held */
> >  void __kprobes arch_prepare_kretprobe(struct kretprobe *rp,
> >  				      struct pt_regs *regs)
> >  {
> > @@ -152,8 +152,6 @@ static inline int kprobe_handler(struct 
> >  
> >  	/* Check we're not actually recursing */
> >  	if (kprobe_running()) {
> > -		/* We *are* holding lock here, so this is safe.
> > -		   Disarm the probe we just hit, and ignore it. */
> >  		p = get_kprobe(addr);
> >  		if (p) {
> >  			kprobe_opcode_t insn = *p->ainsn.insn;
> > @@ -161,7 +159,6 @@ static inline int kprobe_handler(struct 
> >  					is_trap(insn)) {
> >  				regs->msr &= ~MSR_SE;
> >  				regs->msr |= kcb->kprobe_saved_msr;
> > -				unlock_kprobes();
> >  				goto no_kprobe;
> >  			}
> >  			/* We have reentered the kprobe_handler(), since
> > @@ -183,14 +180,11 @@ static inline int kprobe_handler(struct 
> >  				goto ss_probe;
> >  			}
> >  		}
> > -		/* If it's not ours, can't be delete race, (we hold lock). */
> >  		goto no_kprobe;
> >  	}
> >  
> > -	lock_kprobes();
> >  	p = get_kprobe(addr);
> >  	if (!p) {
> > -		unlock_kprobes();
> >  		if (*addr != BREAKPOINT_INSTRUCTION) {
> >  			/*
> >  			 * PowerPC has multiple variants of the "trap"
> > @@ -254,9 +248,10 @@ int __kprobes trampoline_probe_handler(s
> >          struct kretprobe_instance *ri = NULL;
> >          struct hlist_head *head;
> >          struct hlist_node *node, *tmp;
> > -	unsigned long orig_ret_address = 0;
> > +	unsigned long flags, orig_ret_address = 0;
> >  	unsigned long trampoline_address =(unsigned long)&kretprobe_trampoline;
> >  
> > +	spin_lock_irqsave(&kretprobe_lock, flags);
> >          head = kretprobe_inst_table_head(current);
> >  
> >  	/*
> > @@ -296,7 +291,7 @@ int __kprobes trampoline_probe_handler(s
> >  	regs->nip = orig_ret_address;
> >  
> >  	reset_current_kprobe();
> > -	unlock_kprobes();
> > +	spin_unlock_irqrestore(&kretprobe_lock, flags);
> >  	preempt_enable_no_resched();
> >  
> >          /*
> > @@ -348,7 +343,6 @@ static inline int post_kprobe_handler(st
> >  		goto out;
> >  	}
> >  	reset_current_kprobe();
> > -	unlock_kprobes();
> >  out:
> >  	preempt_enable_no_resched();
> >  
> > @@ -363,7 +357,6 @@ out:
> >  	return 1;
> >  }
> >  
> > -/* Interrupts disabled, kprobe_lock held. */
> >  static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
> >  {
> >  	struct kprobe *cur = kprobe_running();
> > @@ -378,7 +371,6 @@ static inline int kprobe_fault_handler(s
> >  		regs->msr |= kcb->kprobe_saved_msr;
> >  
> >  		reset_current_kprobe();
> > -		unlock_kprobes();
> >  		preempt_enable_no_resched();
> >  	}
> >  	return 0;
> > @@ -393,11 +385,7 @@ int __kprobes kprobe_exceptions_notify(s
> >  	struct die_args *args = (struct die_args *)data;
> >  	int ret = NOTIFY_DONE;
> >  
> > -	/*
> > -	 * Interrupts are not disabled here.  We need to disable
> > -	 * preemption, because kprobe_running() uses smp_processor_id().
> > -	 */
> > -	preempt_disable();
> > +	rcu_read_lock();
> 
> And here...
> 
> >  	switch (val) {
> >  	case DIE_BPT:
> >  		if (kprobe_handler(args->regs))
> > @@ -416,7 +404,7 @@ int __kprobes kprobe_exceptions_notify(s
> >  	default:
> >  		break;
> >  	}
> > -	preempt_enable_no_resched();
> > +	rcu_read_unlock();
> >  	return ret;
> >  }
> >  
> > Index: linux-2.6.14-rc3/arch/sparc64/kernel/kprobes.c
> > ===================================================================
> > --- linux-2.6.14-rc3.orig/arch/sparc64/kernel/kprobes.c	2005-10-05 16:08:15.000000000 -0400
> > +++ linux-2.6.14-rc3/arch/sparc64/kernel/kprobes.c	2005-10-05 16:08:48.000000000 -0400
> > @@ -116,15 +116,11 @@ static int __kprobes kprobe_handler(stru
> >  	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
> >  
> >  	if (kprobe_running()) {
> > -		/* We *are* holding lock here, so this is safe.
> > -		 * Disarm the probe we just hit, and ignore it.
> > -		 */
> >  		p = get_kprobe(addr);
> >  		if (p) {
> >  			if (kcb->kprobe_status == KPROBE_HIT_SS) {
> >  				regs->tstate = ((regs->tstate & ~TSTATE_PIL) |
> >  					kcb->kprobe_orig_tstate_pil);
> > -				unlock_kprobes();
> >  				goto no_kprobe;
> >  			}
> >  			/* We have reentered the kprobe_handler(), since
> > @@ -144,14 +140,11 @@ static int __kprobes kprobe_handler(stru
> >  			if (p->break_handler && p->break_handler(p, regs))
> >  				goto ss_probe;
> >  		}
> > -		/* If it's not ours, can't be delete race, (we hold lock). */
> >  		goto no_kprobe;
> >  	}
> >  
> > -	lock_kprobes();
> >  	p = get_kprobe(addr);
> >  	if (!p) {
> > -		unlock_kprobes();
> >  		if (*(u32 *)addr != BREAKPOINT_INSTRUCTION) {
> >  			/*
> >  			 * The breakpoint instruction was removed right
> > @@ -296,14 +289,12 @@ static inline int post_kprobe_handler(st
> >  		goto out;
> >  	}
> >  	reset_current_kprobe();
> > -	unlock_kprobes();
> >  out:
> >  	preempt_enable_no_resched();
> >  
> >  	return 1;
> >  }
> >  
> > -/* Interrupts disabled, kprobe_lock held. */
> >  static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
> >  {
> >  	struct kprobe *cur = kprobe_running();
> > @@ -316,7 +307,6 @@ static inline int kprobe_fault_handler(s
> >  		resume_execution(cur, regs, kcb);
> >  
> >  		reset_current_kprobe();
> > -		unlock_kprobes();
> >  		preempt_enable_no_resched();
> >  	}
> >  	return 0;
> > @@ -331,7 +321,7 @@ int __kprobes kprobe_exceptions_notify(s
> >  	struct die_args *args = (struct die_args *)data;
> >  	int ret = NOTIFY_DONE;
> >  
> > -	preempt_disable();
> > +	rcu_read_lock();
> 
> As well as here...
> 
> >  	switch (val) {
> >  	case DIE_DEBUG:
> >  		if (kprobe_handler(args->regs))
> > @@ -350,7 +340,7 @@ int __kprobes kprobe_exceptions_notify(s
> >  	default:
> >  		break;
> >  	}
> > -	preempt_enable();
> > +	rcu_read_unlock();
> >  	return ret;
> >  }
> >  
> > Index: linux-2.6.14-rc3/arch/x86_64/kernel/kprobes.c
> > ===================================================================
> > --- linux-2.6.14-rc3.orig/arch/x86_64/kernel/kprobes.c	2005-10-05 16:08:33.000000000 -0400
> > +++ linux-2.6.14-rc3/arch/x86_64/kernel/kprobes.c	2005-10-05 16:08:48.000000000 -0400
> > @@ -34,7 +34,6 @@
> >  #include <linux/config.h>
> >  #include <linux/kprobes.h>
> >  #include <linux/ptrace.h>
> > -#include <linux/spinlock.h>
> >  #include <linux/string.h>
> >  #include <linux/slab.h>
> >  #include <linux/preempt.h>
> > @@ -266,6 +265,7 @@ static void __kprobes prepare_singlestep
> >  		regs->rip = (unsigned long)p->ainsn.insn;
> >  }
> >  
> > +/* Called with kretprobe_lock held */
> >  void __kprobes arch_prepare_kretprobe(struct kretprobe *rp,
> >  				      struct pt_regs *regs)
> >  {
> > @@ -299,15 +299,12 @@ int __kprobes kprobe_handler(struct pt_r
> >  
> >  	/* Check we're not actually recursing */
> >  	if (kprobe_running()) {
> > -		/* We *are* holding lock here, so this is safe.
> > -		   Disarm the probe we just hit, and ignore it. */
> >  		p = get_kprobe(addr);
> >  		if (p) {
> >  			if (kcb->kprobe_status == KPROBE_HIT_SS &&
> >  				*p->ainsn.insn == BREAKPOINT_INSTRUCTION) {
> >  				regs->eflags &= ~TF_MASK;
> >  				regs->eflags |= kcb->kprobe_saved_rflags;
> > -				unlock_kprobes();
> >  				goto no_kprobe;
> >  			} else if (kcb->kprobe_status == KPROBE_HIT_SSDONE) {
> >  				/* TODO: Provide re-entrancy from
> > @@ -340,14 +337,11 @@ int __kprobes kprobe_handler(struct pt_r
> >  				goto ss_probe;
> >  			}
> >  		}
> > -		/* If it's not ours, can't be delete race, (we hold lock). */
> >  		goto no_kprobe;
> >  	}
> >  
> > -	lock_kprobes();
> >  	p = get_kprobe(addr);
> >  	if (!p) {
> > -		unlock_kprobes();
> >  		if (*addr != BREAKPOINT_INSTRUCTION) {
> >  			/*
> >  			 * The breakpoint instruction was removed right
> > @@ -406,9 +400,10 @@ int __kprobes trampoline_probe_handler(s
> >          struct kretprobe_instance *ri = NULL;
> >          struct hlist_head *head;
> >          struct hlist_node *node, *tmp;
> > -	unsigned long orig_ret_address = 0;
> > +	unsigned long flags, orig_ret_address = 0;
> >  	unsigned long trampoline_address =(unsigned long)&kretprobe_trampoline;
> >  
> > +	spin_lock_irqsave(&kretprobe_lock, flags);
> >          head = kretprobe_inst_table_head(current);
> >  
> >  	/*
> > @@ -448,7 +443,7 @@ int __kprobes trampoline_probe_handler(s
> >  	regs->rip = orig_ret_address;
> >  
> >  	reset_current_kprobe();
> > -	unlock_kprobes();
> > +	spin_unlock_irqrestore(&kretprobe_lock, flags);
> >  	preempt_enable_no_resched();
> >  
> >          /*
> > @@ -536,10 +531,6 @@ static void __kprobes resume_execution(s
> >  	}
> >  }
> >  
> > -/*
> > - * Interrupts are disabled on entry as trap1 is an interrupt gate and they
> > - * remain disabled thoroughout this function.  And we hold kprobe lock.
> > - */
> >  int __kprobes post_kprobe_handler(struct pt_regs *regs)
> >  {
> >  	struct kprobe *cur = kprobe_running();
> > @@ -560,8 +551,6 @@ int __kprobes post_kprobe_handler(struct
> >  	if (kcb->kprobe_status == KPROBE_REENTER) {
> >  		restore_previous_kprobe(kcb);
> >  		goto out;
> > -	} else {
> > -		unlock_kprobes();
> >  	}
> >  	reset_current_kprobe();
> >  out:
> > @@ -578,7 +567,6 @@ out:
> >  	return 1;
> >  }
> >  
> > -/* Interrupts disabled, kprobe_lock held. */
> >  int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr)
> >  {
> >  	struct kprobe *cur = kprobe_running();
> > @@ -592,7 +580,6 @@ int __kprobes kprobe_fault_handler(struc
> >  		regs->eflags |= kcb->kprobe_old_rflags;
> >  
> >  		reset_current_kprobe();
> > -		unlock_kprobes();
> >  		preempt_enable_no_resched();
> >  	}
> >  	return 0;
> > @@ -607,7 +594,7 @@ int __kprobes kprobe_exceptions_notify(s
> >  	struct die_args *args = (struct die_args *)data;
> >  	int ret = NOTIFY_DONE;
> >  
> > -	preempt_disable();
> > +	rcu_read_lock();
> 
> As well as here yet again...
> 
> >  	switch (val) {
> >  	case DIE_INT3:
> >  		if (kprobe_handler(args->regs))
> > @@ -626,7 +613,7 @@ int __kprobes kprobe_exceptions_notify(s
> >  	default:
> >  		break;
> >  	}
> > -	preempt_enable();
> > +	rcu_read_unlock();
> >  	return ret;
> >  }
> >  
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> > 
> > 

Re: [PATCH 9/9] Kprobes: Use RCU for (un)register synchronization - arch changes

[Paul E. McKenney]

On Tue, Oct 18, 2005 at 10:45:26AM -0400, Ananth N Mavinakayanahalli wrote:
> On Mon, Oct 17, 2005 at 10:49:30PM -0700, Paul E. McKenney wrote:
> > On Mon, Oct 10, 2005 at 10:48:13AM -0400, Ananth N Mavinakayanahalli wrote:
> > > From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> > > 
> > > Changes to the arch kprobes infrastructure to take advantage of the locking
> > > changes introduced by usage of RCU for synchronization. All handlers are
> > > now run without any locks held, so they have to be re-entrant or provide
> > > their own synchronization.
> > 
> > And a few very similar questions here as well...
> 
> Replies inline...
> 
> Ananth

Rereplies also inline!

> > 							Thanx, Paul
> > 
> > > Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> > > Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
> > > ---
> > > 
> > >  arch/i386/kernel/kprobes.c    |   22 +++++++---------------
> > >  arch/ia64/kernel/kprobes.c    |   16 ++++++----------
> > >  arch/ppc64/kernel/kprobes.c   |   24 ++++++------------------
> > >  arch/sparc64/kernel/kprobes.c |   14 ++------------
> > >  arch/x86_64/kernel/kprobes.c  |   25 ++++++-------------------
> > >  5 files changed, 27 insertions(+), 74 deletions(-)
> > > 
> > > Index: linux-2.6.14-rc3/arch/i386/kernel/kprobes.c
> > > ===================================================================
> > > --- linux-2.6.14-rc3.orig/arch/i386/kernel/kprobes.c	2005-10-05 16:08:13.000000000 -0400
> > > +++ linux-2.6.14-rc3/arch/i386/kernel/kprobes.c	2005-10-05 16:08:48.000000000 -0400
> > > @@ -31,7 +31,6 @@
> > >  #include <linux/config.h>
> > >  #include <linux/kprobes.h>
> > >  #include <linux/ptrace.h>
> > > -#include <linux/spinlock.h>
> > >  #include <linux/preempt.h>
> > >  #include <asm/cacheflush.h>
> > >  #include <asm/kdebug.h>
> > > @@ -123,6 +122,7 @@ static inline void prepare_singlestep(st
> > >  		regs->eip = (unsigned long)&p->ainsn.insn;
> > >  }
> > >  
> > > +/* Called with kretprobe_lock held */
> > >  void __kprobes arch_prepare_kretprobe(struct kretprobe *rp,
> > >  				      struct pt_regs *regs)
> > >  {
> > > @@ -168,15 +168,12 @@ static int __kprobes kprobe_handler(stru
> > >  	}
> > >  	/* Check we're not actually recursing */
> > >  	if (kprobe_running()) {
> > > -		/* We *are* holding lock here, so this is safe.
> > > -		   Disarm the probe we just hit, and ignore it. */
> > >  		p = get_kprobe(addr);
> > >  		if (p) {
> > >  			if (kcb->kprobe_status == KPROBE_HIT_SS &&
> > >  				*p->ainsn.insn == BREAKPOINT_INSTRUCTION) {
> > >  				regs->eflags &= ~TF_MASK;
> > >  				regs->eflags |= kcb->kprobe_saved_eflags;
> > > -				unlock_kprobes();
> > >  				goto no_kprobe;
> > >  			}
> > >  			/* We have reentered the kprobe_handler(), since
> > > @@ -197,14 +194,11 @@ static int __kprobes kprobe_handler(stru
> > >  				goto ss_probe;
> > >  			}
> > >  		}
> > > -		/* If it's not ours, can't be delete race, (we hold lock). */
> > >  		goto no_kprobe;
> > >  	}
> > >  
> > > -	lock_kprobes();
> > >  	p = get_kprobe(addr);
> > >  	if (!p) {
> > > -		unlock_kprobes();
> > >  		if (regs->eflags & VM_MASK) {
> > >  			/* We are in virtual-8086 mode. Return 0 */
> > >  			goto no_kprobe;
> > > @@ -268,9 +262,10 @@ int __kprobes trampoline_probe_handler(s
> > >          struct kretprobe_instance *ri = NULL;
> > >          struct hlist_head *head;
> > >          struct hlist_node *node, *tmp;
> > > -	unsigned long orig_ret_address = 0;
> > > +	unsigned long flags, orig_ret_address = 0;
> > >  	unsigned long trampoline_address =(unsigned long)&kretprobe_trampoline;
> > >  
> > > +	spin_lock_irqsave(&kretprobe_lock, flags);
> > >          head = kretprobe_inst_table_head(current);
> > >  
> > >  	/*
> > > @@ -310,7 +305,7 @@ int __kprobes trampoline_probe_handler(s
> > >  	regs->eip = orig_ret_address;
> > >  
> > >  	reset_current_kprobe();
> > > -	unlock_kprobes();
> > > +	spin_unlock_irqrestore(&kretprobe_lock, flags);
> > >  	preempt_enable_no_resched();
> > >  
> > >          /*
> > > @@ -395,7 +390,7 @@ static void __kprobes resume_execution(s
> > >  
> > >  /*
> > >   * Interrupts are disabled on entry as trap1 is an interrupt gate and they
> > > - * remain disabled thoroughout this function.  And we hold kprobe lock.
> > > + * remain disabled thoroughout this function.
> > >   */
> > >  static inline int post_kprobe_handler(struct pt_regs *regs)
> > >  {
> > > @@ -419,7 +414,6 @@ static inline int post_kprobe_handler(st
> > >  		goto out;
> > >  	}
> > >  	reset_current_kprobe();
> > > -	unlock_kprobes();
> > >  out:
> > >  	preempt_enable_no_resched();
> > >  
> > > @@ -434,7 +428,6 @@ out:
> > >  	return 1;
> > >  }
> > >  
> > > -/* Interrupts disabled, kprobe_lock held. */
> > >  static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
> > >  {
> > >  	struct kprobe *cur = kprobe_running();
> > > @@ -448,7 +441,6 @@ static inline int kprobe_fault_handler(s
> > >  		regs->eflags |= kcb->kprobe_old_eflags;
> > >  
> > >  		reset_current_kprobe();
> > > -		unlock_kprobes();
> > >  		preempt_enable_no_resched();
> > >  	}
> > >  	return 0;
> > > @@ -463,7 +455,7 @@ int __kprobes kprobe_exceptions_notify(s
> > >  	struct die_args *args = (struct die_args *)data;
> > >  	int ret = NOTIFY_DONE;
> > >  
> > > -	preempt_disable();
> > > +	rcu_read_lock();
> > 
> > If synchronize_sched() is used on the update side, this needs to
> > remain preempt_disable() rather than rcu_read_lock().
> 
> Kprobe handlers can't block/sleep. So the idea is to depend on a
> schedule() event to ensure handlers have executed. This and the others
> you have pointed out can surely be preempt_disable().

Yep!  Or maybe you can rely on the preempt_disable() that guards the
single-step operation?

> > >  	switch (val) {
> > >  	case DIE_INT3:
> > >  		if (kprobe_handler(args->regs))
> > > @@ -482,7 +474,7 @@ int __kprobes kprobe_exceptions_notify(s
> > >  	default:
> > >  		break;
> > >  	}
> > > -	preempt_enable();
> > > +	rcu_read_unlock();
> > >  	return ret;
> > >  }
> > >  
> > > Index: linux-2.6.14-rc3/arch/ia64/kernel/kprobes.c
> > > ===================================================================
> > > --- linux-2.6.14-rc3.orig/arch/ia64/kernel/kprobes.c	2005-10-05 16:08:14.000000000 -0400
> > > +++ linux-2.6.14-rc3/arch/ia64/kernel/kprobes.c	2005-10-05 16:08:48.000000000 -0400
> > > @@ -26,7 +26,6 @@
> > >  #include <linux/config.h>
> > >  #include <linux/kprobes.h>
> > >  #include <linux/ptrace.h>
> > > -#include <linux/spinlock.h>
> > >  #include <linux/string.h>
> > >  #include <linux/slab.h>
> > >  #include <linux/preempt.h>
> > > @@ -343,10 +342,11 @@ int __kprobes trampoline_probe_handler(s
> > >  	struct kretprobe_instance *ri = NULL;
> > >  	struct hlist_head *head;
> > >  	struct hlist_node *node, *tmp;
> > > -	unsigned long orig_ret_address = 0;
> > > +	unsigned long flags, orig_ret_address = 0;
> > >  	unsigned long trampoline_address =
> > >  		((struct fnptr *)kretprobe_trampoline)->ip;
> > >  
> > > +	spin_lock_irqsave(&kretprobe_lock, flags);
> > >          head = kretprobe_inst_table_head(current);
> > >  
> > >  	/*
> > > @@ -386,7 +386,7 @@ int __kprobes trampoline_probe_handler(s
> > >  	regs->cr_iip = orig_ret_address;
> > >  
> > >  	reset_current_kprobe();
> > > -	unlock_kprobes();
> > > +	spin_unlock_irqrestore(&kretprobe_lock, flags);
> > >  	preempt_enable_no_resched();
> > >  
> > >          /*
> > > @@ -397,6 +397,7 @@ int __kprobes trampoline_probe_handler(s
> > >          return 1;
> > >  }
> > >  
> > > +/* Called with kretprobe_lock held */
> > >  void __kprobes arch_prepare_kretprobe(struct kretprobe *rp,
> > >  				      struct pt_regs *regs)
> > >  {
> > > @@ -612,7 +613,6 @@ static int __kprobes pre_kprobes_handler
> > >  			if ((kcb->kprobe_status == KPROBE_HIT_SS) &&
> > >  	 		     (p->ainsn.inst_flag == INST_FLAG_BREAK_INST)) {
> > >    				ia64_psr(regs)->ss = 0;
> > > -				unlock_kprobes();
> > >  				goto no_kprobe;
> > >  			}
> > >  			/* We have reentered the pre_kprobe_handler(), since
> > > @@ -641,10 +641,8 @@ static int __kprobes pre_kprobes_handler
> > >  		}
> > >  	}
> > >  
> > > -	lock_kprobes();
> > >  	p = get_kprobe(addr);
> > >  	if (!p) {
> > > -		unlock_kprobes();
> > >  		if (!is_ia64_break_inst(regs)) {
> > >  			/*
> > >  			 * The breakpoint instruction was removed right
> > > @@ -707,7 +705,6 @@ static int __kprobes post_kprobes_handle
> > >  		goto out;
> > >  	}
> > >  	reset_current_kprobe();
> > > -	unlock_kprobes();
> > >  
> > >  out:
> > >  	preempt_enable_no_resched();
> > > @@ -728,7 +725,6 @@ static int __kprobes kprobes_fault_handl
> > >  	if (kcb->kprobe_status & KPROBE_HIT_SS) {
> > >  		resume_execution(cur, regs);
> > >  		reset_current_kprobe();
> > > -		unlock_kprobes();
> > >  		preempt_enable_no_resched();
> > >  	}
> > >  
> > > @@ -741,7 +737,7 @@ int __kprobes kprobe_exceptions_notify(s
> > >  	struct die_args *args = (struct die_args *)data;
> > >  	int ret = NOTIFY_DONE;
> > >  
> > > -	preempt_disable();
> > > +	rcu_read_lock();
> > 
> > Ditto here...
> > 
> > >  	switch(val) {
> > >  	case DIE_BREAK:
> > >  		if (pre_kprobes_handler(args))
> > > @@ -757,7 +753,7 @@ int __kprobes kprobe_exceptions_notify(s
> > >  	default:
> > >  		break;
> > >  	}
> > > -	preempt_enable();
> > > +	rcu_read_unlock();
> > >  	return ret;
> > >  }
> > >  
> > > Index: linux-2.6.14-rc3/arch/ppc64/kernel/kprobes.c
> > > ===================================================================
> > > --- linux-2.6.14-rc3.orig/arch/ppc64/kernel/kprobes.c	2005-10-05 16:08:15.000000000 -0400
> > > +++ linux-2.6.14-rc3/arch/ppc64/kernel/kprobes.c	2005-10-05 16:08:48.000000000 -0400
> > > @@ -30,7 +30,6 @@
> > >  #include <linux/config.h>
> > >  #include <linux/kprobes.h>
> > >  #include <linux/ptrace.h>
> > > -#include <linux/spinlock.h>
> > >  #include <linux/preempt.h>
> > >  #include <asm/cacheflush.h>
> > >  #include <asm/kdebug.h>
> > > @@ -125,6 +124,7 @@ static inline void set_current_kprobe(st
> > >  	kcb->kprobe_saved_msr = regs->msr;
> > >  }
> > >  
> > > +/* Called with kretprobe_lock held */
> > >  void __kprobes arch_prepare_kretprobe(struct kretprobe *rp,
> > >  				      struct pt_regs *regs)
> > >  {
> > > @@ -152,8 +152,6 @@ static inline int kprobe_handler(struct 
> > >  
> > >  	/* Check we're not actually recursing */
> > >  	if (kprobe_running()) {
> > > -		/* We *are* holding lock here, so this is safe.
> > > -		   Disarm the probe we just hit, and ignore it. */
> > >  		p = get_kprobe(addr);
> > >  		if (p) {
> > >  			kprobe_opcode_t insn = *p->ainsn.insn;
> > > @@ -161,7 +159,6 @@ static inline int kprobe_handler(struct 
> > >  					is_trap(insn)) {
> > >  				regs->msr &= ~MSR_SE;
> > >  				regs->msr |= kcb->kprobe_saved_msr;
> > > -				unlock_kprobes();
> > >  				goto no_kprobe;
> > >  			}
> > >  			/* We have reentered the kprobe_handler(), since
> > > @@ -183,14 +180,11 @@ static inline int kprobe_handler(struct 
> > >  				goto ss_probe;
> > >  			}
> > >  		}
> > > -		/* If it's not ours, can't be delete race, (we hold lock). */
> > >  		goto no_kprobe;
> > >  	}
> > >  
> > > -	lock_kprobes();
> > >  	p = get_kprobe(addr);
> > >  	if (!p) {
> > > -		unlock_kprobes();
> > >  		if (*addr != BREAKPOINT_INSTRUCTION) {
> > >  			/*
> > >  			 * PowerPC has multiple variants of the "trap"
> > > @@ -254,9 +248,10 @@ int __kprobes trampoline_probe_handler(s
> > >          struct kretprobe_instance *ri = NULL;
> > >          struct hlist_head *head;
> > >          struct hlist_node *node, *tmp;
> > > -	unsigned long orig_ret_address = 0;
> > > +	unsigned long flags, orig_ret_address = 0;
> > >  	unsigned long trampoline_address =(unsigned long)&kretprobe_trampoline;
> > >  
> > > +	spin_lock_irqsave(&kretprobe_lock, flags);
> > >          head = kretprobe_inst_table_head(current);
> > >  
> > >  	/*
> > > @@ -296,7 +291,7 @@ int __kprobes trampoline_probe_handler(s
> > >  	regs->nip = orig_ret_address;
> > >  
> > >  	reset_current_kprobe();
> > > -	unlock_kprobes();
> > > +	spin_unlock_irqrestore(&kretprobe_lock, flags);
> > >  	preempt_enable_no_resched();
> > >  
> > >          /*
> > > @@ -348,7 +343,6 @@ static inline int post_kprobe_handler(st
> > >  		goto out;
> > >  	}
> > >  	reset_current_kprobe();
> > > -	unlock_kprobes();
> > >  out:
> > >  	preempt_enable_no_resched();
> > >  
> > > @@ -363,7 +357,6 @@ out:
> > >  	return 1;
> > >  }
> > >  
> > > -/* Interrupts disabled, kprobe_lock held. */
> > >  static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
> > >  {
> > >  	struct kprobe *cur = kprobe_running();
> > > @@ -378,7 +371,6 @@ static inline int kprobe_fault_handler(s
> > >  		regs->msr |= kcb->kprobe_saved_msr;
> > >  
> > >  		reset_current_kprobe();
> > > -		unlock_kprobes();
> > >  		preempt_enable_no_resched();
> > >  	}
> > >  	return 0;
> > > @@ -393,11 +385,7 @@ int __kprobes kprobe_exceptions_notify(s
> > >  	struct die_args *args = (struct die_args *)data;
> > >  	int ret = NOTIFY_DONE;
> > >  
> > > -	/*
> > > -	 * Interrupts are not disabled here.  We need to disable
> > > -	 * preemption, because kprobe_running() uses smp_processor_id().
> > > -	 */
> > > -	preempt_disable();
> > > +	rcu_read_lock();
> > 
> > And here...
> > 
> > >  	switch (val) {
> > >  	case DIE_BPT:
> > >  		if (kprobe_handler(args->regs))
> > > @@ -416,7 +404,7 @@ int __kprobes kprobe_exceptions_notify(s
> > >  	default:
> > >  		break;
> > >  	}
> > > -	preempt_enable_no_resched();
> > > +	rcu_read_unlock();
> > >  	return ret;
> > >  }
> > >  
> > > Index: linux-2.6.14-rc3/arch/sparc64/kernel/kprobes.c
> > > ===================================================================
> > > --- linux-2.6.14-rc3.orig/arch/sparc64/kernel/kprobes.c	2005-10-05 16:08:15.000000000 -0400
> > > +++ linux-2.6.14-rc3/arch/sparc64/kernel/kprobes.c	2005-10-05 16:08:48.000000000 -0400
> > > @@ -116,15 +116,11 @@ static int __kprobes kprobe_handler(stru
> > >  	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
> > >  
> > >  	if (kprobe_running()) {
> > > -		/* We *are* holding lock here, so this is safe.
> > > -		 * Disarm the probe we just hit, and ignore it.
> > > -		 */
> > >  		p = get_kprobe(addr);
> > >  		if (p) {
> > >  			if (kcb->kprobe_status == KPROBE_HIT_SS) {
> > >  				regs->tstate = ((regs->tstate & ~TSTATE_PIL) |
> > >  					kcb->kprobe_orig_tstate_pil);
> > > -				unlock_kprobes();
> > >  				goto no_kprobe;
> > >  			}
> > >  			/* We have reentered the kprobe_handler(), since
> > > @@ -144,14 +140,11 @@ static int __kprobes kprobe_handler(stru
> > >  			if (p->break_handler && p->break_handler(p, regs))
> > >  				goto ss_probe;
> > >  		}
> > > -		/* If it's not ours, can't be delete race, (we hold lock). */
> > >  		goto no_kprobe;
> > >  	}
> > >  
> > > -	lock_kprobes();
> > >  	p = get_kprobe(addr);
> > >  	if (!p) {
> > > -		unlock_kprobes();
> > >  		if (*(u32 *)addr != BREAKPOINT_INSTRUCTION) {
> > >  			/*
> > >  			 * The breakpoint instruction was removed right
> > > @@ -296,14 +289,12 @@ static inline int post_kprobe_handler(st
> > >  		goto out;
> > >  	}
> > >  	reset_current_kprobe();
> > > -	unlock_kprobes();
> > >  out:
> > >  	preempt_enable_no_resched();
> > >  
> > >  	return 1;
> > >  }
> > >  
> > > -/* Interrupts disabled, kprobe_lock held. */
> > >  static inline int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
> > >  {
> > >  	struct kprobe *cur = kprobe_running();
> > > @@ -316,7 +307,6 @@ static inline int kprobe_fault_handler(s
> > >  		resume_execution(cur, regs, kcb);
> > >  
> > >  		reset_current_kprobe();
> > > -		unlock_kprobes();
> > >  		preempt_enable_no_resched();
> > >  	}
> > >  	return 0;
> > > @@ -331,7 +321,7 @@ int __kprobes kprobe_exceptions_notify(s
> > >  	struct die_args *args = (struct die_args *)data;
> > >  	int ret = NOTIFY_DONE;
> > >  
> > > -	preempt_disable();
> > > +	rcu_read_lock();
> > 
> > As well as here...
> > 
> > >  	switch (val) {
> > >  	case DIE_DEBUG:
> > >  		if (kprobe_handler(args->regs))
> > > @@ -350,7 +340,7 @@ int __kprobes kprobe_exceptions_notify(s
> > >  	default:
> > >  		break;
> > >  	}
> > > -	preempt_enable();
> > > +	rcu_read_unlock();
> > >  	return ret;
> > >  }
> > >  
> > > Index: linux-2.6.14-rc3/arch/x86_64/kernel/kprobes.c
> > > ===================================================================
> > > --- linux-2.6.14-rc3.orig/arch/x86_64/kernel/kprobes.c	2005-10-05 16:08:33.000000000 -0400
> > > +++ linux-2.6.14-rc3/arch/x86_64/kernel/kprobes.c	2005-10-05 16:08:48.000000000 -0400
> > > @@ -34,7 +34,6 @@
> > >  #include <linux/config.h>
> > >  #include <linux/kprobes.h>
> > >  #include <linux/ptrace.h>
> > > -#include <linux/spinlock.h>
> > >  #include <linux/string.h>
> > >  #include <linux/slab.h>
> > >  #include <linux/preempt.h>
> > > @@ -266,6 +265,7 @@ static void __kprobes prepare_singlestep
> > >  		regs->rip = (unsigned long)p->ainsn.insn;
> > >  }
> > >  
> > > +/* Called with kretprobe_lock held */
> > >  void __kprobes arch_prepare_kretprobe(struct kretprobe *rp,
> > >  				      struct pt_regs *regs)
> > >  {
> > > @@ -299,15 +299,12 @@ int __kprobes kprobe_handler(struct pt_r
> > >  
> > >  	/* Check we're not actually recursing */
> > >  	if (kprobe_running()) {
> > > -		/* We *are* holding lock here, so this is safe.
> > > -		   Disarm the probe we just hit, and ignore it. */
> > >  		p = get_kprobe(addr);
> > >  		if (p) {
> > >  			if (kcb->kprobe_status == KPROBE_HIT_SS &&
> > >  				*p->ainsn.insn == BREAKPOINT_INSTRUCTION) {
> > >  				regs->eflags &= ~TF_MASK;
> > >  				regs->eflags |= kcb->kprobe_saved_rflags;
> > > -				unlock_kprobes();
> > >  				goto no_kprobe;
> > >  			} else if (kcb->kprobe_status == KPROBE_HIT_SSDONE) {
> > >  				/* TODO: Provide re-entrancy from
> > > @@ -340,14 +337,11 @@ int __kprobes kprobe_handler(struct pt_r
> > >  				goto ss_probe;
> > >  			}
> > >  		}
> > > -		/* If it's not ours, can't be delete race, (we hold lock). */
> > >  		goto no_kprobe;
> > >  	}
> > >  
> > > -	lock_kprobes();
> > >  	p = get_kprobe(addr);
> > >  	if (!p) {
> > > -		unlock_kprobes();
> > >  		if (*addr != BREAKPOINT_INSTRUCTION) {
> > >  			/*
> > >  			 * The breakpoint instruction was removed right
> > > @@ -406,9 +400,10 @@ int __kprobes trampoline_probe_handler(s
> > >          struct kretprobe_instance *ri = NULL;
> > >          struct hlist_head *head;
> > >          struct hlist_node *node, *tmp;
> > > -	unsigned long orig_ret_address = 0;
> > > +	unsigned long flags, orig_ret_address = 0;
> > >  	unsigned long trampoline_address =(unsigned long)&kretprobe_trampoline;
> > >  
> > > +	spin_lock_irqsave(&kretprobe_lock, flags);
> > >          head = kretprobe_inst_table_head(current);
> > >  
> > >  	/*
> > > @@ -448,7 +443,7 @@ int __kprobes trampoline_probe_handler(s
> > >  	regs->rip = orig_ret_address;
> > >  
> > >  	reset_current_kprobe();
> > > -	unlock_kprobes();
> > > +	spin_unlock_irqrestore(&kretprobe_lock, flags);
> > >  	preempt_enable_no_resched();
> > >  
> > >          /*
> > > @@ -536,10 +531,6 @@ static void __kprobes resume_execution(s
> > >  	}
> > >  }
> > >  
> > > -/*
> > > - * Interrupts are disabled on entry as trap1 is an interrupt gate and they
> > > - * remain disabled thoroughout this function.  And we hold kprobe lock.
> > > - */
> > >  int __kprobes post_kprobe_handler(struct pt_regs *regs)
> > >  {
> > >  	struct kprobe *cur = kprobe_running();
> > > @@ -560,8 +551,6 @@ int __kprobes post_kprobe_handler(struct
> > >  	if (kcb->kprobe_status == KPROBE_REENTER) {
> > >  		restore_previous_kprobe(kcb);
> > >  		goto out;
> > > -	} else {
> > > -		unlock_kprobes();
> > >  	}
> > >  	reset_current_kprobe();
> > >  out:
> > > @@ -578,7 +567,6 @@ out:
> > >  	return 1;
> > >  }
> > >  
> > > -/* Interrupts disabled, kprobe_lock held. */
> > >  int __kprobes kprobe_fault_handler(struct pt_regs *regs, int trapnr)
> > >  {
> > >  	struct kprobe *cur = kprobe_running();
> > > @@ -592,7 +580,6 @@ int __kprobes kprobe_fault_handler(struc
> > >  		regs->eflags |= kcb->kprobe_old_rflags;
> > >  
> > >  		reset_current_kprobe();
> > > -		unlock_kprobes();
> > >  		preempt_enable_no_resched();
> > >  	}
> > >  	return 0;
> > > @@ -607,7 +594,7 @@ int __kprobes kprobe_exceptions_notify(s
> > >  	struct die_args *args = (struct die_args *)data;
> > >  	int ret = NOTIFY_DONE;
> > >  
> > > -	preempt_disable();
> > > +	rcu_read_lock();
> > 
> > As well as here yet again...
> > 
> > >  	switch (val) {
> > >  	case DIE_INT3:
> > >  		if (kprobe_handler(args->regs))
> > > @@ -626,7 +613,7 @@ int __kprobes kprobe_exceptions_notify(s
> > >  	default:
> > >  		break;
> > >  	}
> > > -	preempt_enable();
> > > +	rcu_read_unlock();
> > >  	return ret;
> > >  }
> > >  
> > > -
> > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > Please read the FAQ at  http://www.tux.org/lkml/
> > > 
> > > 
> 

Re: [PATCH 8/9] Kprobes: Use RCU for (un)register synchronization - base changes

[Paul E. McKenney]

On Mon, Oct 10, 2005 at 10:47:20AM -0400, Ananth N Mavinakayanahalli wrote:
> From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> 
> Changes to the base kprobes infrastructure to use RCU for synchronization
> during kprobe registration and unregistration. These changes coupled with
> the arch kprobe changes (next in series):
> 
> a. serialize registration and unregistration of kprobes.
> b. enable lockless execution of handlers. Handlers can now run in parallel.

A couple of questions interspersed...  Probably my limited knowledge of
kprobes, but I have to ask...  Search for empty lines to find them.

						Thanx, Paul

> Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
> ---
> 
>  include/linux/kprobes.h |    9 +---
>  kernel/kprobes.c        |  101 +++++++++++++++++++-----------------------------
>  2 files changed, 45 insertions(+), 65 deletions(-)
> 
> Index: linux-2.6.14-rc3/include/linux/kprobes.h
> ===================================================================
> --- linux-2.6.14-rc3.orig/include/linux/kprobes.h	2005-10-07 21:40:43.000000000 -0400
> +++ linux-2.6.14-rc3/include/linux/kprobes.h	2005-10-07 21:40:49.000000000 -0400
> @@ -34,6 +34,8 @@
>  #include <linux/notifier.h>
>  #include <linux/smp.h>
>  #include <linux/percpu.h>
> +#include <linux/spinlock.h>
> +#include <linux/rcupdate.h>
>  
>  #include <asm/kprobes.h>
>  
> @@ -146,10 +148,7 @@ struct kretprobe_instance {
>  };
>  
>  #ifdef CONFIG_KPROBES
> -/* Locks kprobe: irq must be disabled */
> -void lock_kprobes(void);
> -void unlock_kprobes(void);
> -
> +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);
> @@ -160,7 +159,7 @@ extern void show_registers(struct pt_reg
>  extern kprobe_opcode_t *get_insn_slot(void);
>  extern void free_insn_slot(kprobe_opcode_t *slot);
>  
> -/* Get the kprobe at this addr (if any).  Must have called lock_kprobes */
> +/* Get the kprobe at this addr (if any) - called under a rcu_read_lock() */

Since the update side uses synchronize_kernel() (in patch 9/9, right?),
shouldn't the above "rcu_read_lock()" instead by preempt_disable()?

Also, some of the code in the earlier patches seems to do preempt_disable.
For example, kprobe_handler() in arch/i386/kernel/kprobes.c.

In realtime kernels, synchronize_sched() does -not- guarantee that all
rcu_read_lock() critical sections have finished, only that any
preempt_disable() code sequences (explicit or implicit) have completed.

>  struct kprobe *get_kprobe(void *addr);
>  struct hlist_head * kretprobe_inst_table_head(struct task_struct *tsk);
>  
> Index: linux-2.6.14-rc3/kernel/kprobes.c
> ===================================================================
> --- linux-2.6.14-rc3.orig/kernel/kprobes.c	2005-10-07 21:40:43.000000000 -0400
> +++ linux-2.6.14-rc3/kernel/kprobes.c	2005-10-07 21:41:11.000000000 -0400
> @@ -32,7 +32,6 @@
>   *		<prasanna@in.ibm.com> added function-return probes.
>   */
>  #include <linux/kprobes.h>
> -#include <linux/spinlock.h>
>  #include <linux/hash.h>
>  #include <linux/init.h>
>  #include <linux/module.h>
> @@ -48,8 +47,8 @@
>  static struct hlist_head kprobe_table[KPROBE_TABLE_SIZE];
>  static struct hlist_head kretprobe_inst_table[KPROBE_TABLE_SIZE];
>  
> -unsigned int kprobe_cpu = NR_CPUS;
> -static DEFINE_SPINLOCK(kprobe_lock);
> +static DEFINE_SPINLOCK(kprobe_lock);	/* Protects kprobe_table */
> +DEFINE_SPINLOCK(kretprobe_lock);	/* Protects kretprobe_inst_table */
>  static DEFINE_PER_CPU(struct kprobe *, kprobe_instance) = NULL;
>  
>  /*
> @@ -152,41 +151,6 @@ void __kprobes free_insn_slot(kprobe_opc
>  	}
>  }
>  
> -/* Locks kprobe: irqs must be disabled */
> -void __kprobes lock_kprobes(void)
> -{
> -	unsigned long flags = 0;
> -
> -	/* Avoiding local interrupts to happen right after we take the kprobe_lock
> -	 * and before we get a chance to update kprobe_cpu, this to prevent
> -	 * deadlock when we have a kprobe on ISR routine and a kprobe on task
> -	 * routine
> -	 */
> -	local_irq_save(flags);
> -
> -	spin_lock(&kprobe_lock);
> -	kprobe_cpu = smp_processor_id();
> -
> - 	local_irq_restore(flags);
> -}
> -
> -void __kprobes unlock_kprobes(void)
> -{
> -	unsigned long flags = 0;
> -
> -	/* Avoiding local interrupts to happen right after we update
> -	 * kprobe_cpu and before we get a a chance to release kprobe_lock,
> -	 * this to prevent deadlock when we have a kprobe on ISR routine and
> -	 * a kprobe on task routine
> -	 */
> -	local_irq_save(flags);
> -
> -	kprobe_cpu = NR_CPUS;
> -	spin_unlock(&kprobe_lock);
> -
> - 	local_irq_restore(flags);
> -}
> -
>  /* We have preemption disabled.. so it is safe to use __ versions */
>  static inline void set_kprobe_instance(struct kprobe *kp)
>  {
> @@ -198,14 +162,19 @@ static inline void reset_kprobe_instance
>  	__get_cpu_var(kprobe_instance) = NULL;
>  }
>  
> -/* You have to be holding the kprobe_lock */
> +/*
> + * This routine is called either:
> + * 	- under the kprobe_lock spinlock - during kprobe_[un]register()
> + * 				OR
> + * 	- under an rcu_read_lock() - from arch/xxx/kernel/kprobes.c

Same here -- shouldn't the rcu_read_lock() be preempt_disable()?

> + */
>  struct kprobe __kprobes *get_kprobe(void *addr)
>  {
>  	struct hlist_head *head;
>  	struct hlist_node *node;
>  
>  	head = &kprobe_table[hash_ptr(addr, KPROBE_HASH_BITS)];
> -	hlist_for_each(node, head) {
> +	hlist_for_each_rcu(node, head) {
>  		struct kprobe *p = hlist_entry(node, struct kprobe, hlist);
>  		if (p->addr == addr)
>  			return p;
> @@ -221,7 +190,7 @@ static int __kprobes aggr_pre_handler(st
>  {
>  	struct kprobe *kp;
>  
> -	list_for_each_entry(kp, &p->list, list) {
> +	list_for_each_entry_rcu(kp, &p->list, list) {
>  		if (kp->pre_handler) {
>  			set_kprobe_instance(kp);
>  			if (kp->pre_handler(kp, regs))
> @@ -237,7 +206,7 @@ static void __kprobes aggr_post_handler(
>  {
>  	struct kprobe *kp;
>  
> -	list_for_each_entry(kp, &p->list, list) {
> +	list_for_each_entry_rcu(kp, &p->list, list) {
>  		if (kp->post_handler) {
>  			set_kprobe_instance(kp);
>  			kp->post_handler(kp, regs, flags);
> @@ -276,6 +245,7 @@ static int __kprobes aggr_break_handler(
>  	return ret;
>  }
>  
> +/* Called with kretprobe_lock held */
>  struct kretprobe_instance __kprobes *get_free_rp_inst(struct kretprobe *rp)
>  {
>  	struct hlist_node *node;
> @@ -285,6 +255,7 @@ struct kretprobe_instance __kprobes *get
>  	return NULL;
>  }
>  
> +/* Called with kretprobe_lock held */
>  static struct kretprobe_instance __kprobes *get_used_rp_inst(struct kretprobe
>  							      *rp)
>  {
> @@ -295,6 +266,7 @@ static struct kretprobe_instance __kprob
>  	return NULL;
>  }
>  
> +/* Called with kretprobe_lock held */
>  void __kprobes add_rp_inst(struct kretprobe_instance *ri)
>  {
>  	/*
> @@ -313,6 +285,7 @@ void __kprobes add_rp_inst(struct kretpr
>  	hlist_add_head(&ri->uflist, &ri->rp->used_instances);
>  }
>  
> +/* Called with kretprobe_lock held */
>  void __kprobes recycle_rp_inst(struct kretprobe_instance *ri)
>  {
>  	/* remove rp inst off the rprobe_inst_table */
> @@ -346,13 +319,13 @@ void __kprobes kprobe_flush_task(struct 
>  	struct hlist_node *node, *tmp;
>  	unsigned long flags = 0;
>  
> -	spin_lock_irqsave(&kprobe_lock, flags);
> +	spin_lock_irqsave(&kretprobe_lock, flags);
>          head = kretprobe_inst_table_head(current);
>          hlist_for_each_entry_safe(ri, node, tmp, head, hlist) {
>                  if (ri->task == tk)
>                          recycle_rp_inst(ri);
>          }
> -	spin_unlock_irqrestore(&kprobe_lock, flags);
> +	spin_unlock_irqrestore(&kretprobe_lock, flags);
>  }
>  
>  /*
> @@ -363,9 +336,12 @@ static int __kprobes pre_handler_kretpro
>  					   struct pt_regs *regs)
>  {
>  	struct kretprobe *rp = container_of(p, struct kretprobe, kp);
> +	unsigned long flags = 0;
>  
>  	/*TODO: consider to only swap the RA after the last pre_handler fired */
> +	spin_lock_irqsave(&kretprobe_lock, flags);
>  	arch_prepare_kretprobe(rp, regs);
> +	spin_unlock_irqrestore(&kretprobe_lock, flags);
>  	return 0;
>  }
>  
> @@ -396,13 +372,13 @@ static int __kprobes add_new_kprobe(stru
>          struct kprobe *kp;
>  
>  	if (p->break_handler) {
> -		list_for_each_entry(kp, &old_p->list, list) {
> +		list_for_each_entry_rcu(kp, &old_p->list, list) {
>  			if (kp->break_handler)
>  				return -EEXIST;
>  		}
> -		list_add_tail(&p->list, &old_p->list);
> +		list_add_tail_rcu(&p->list, &old_p->list);
>  	} else
> -		list_add(&p->list, &old_p->list);
> +		list_add_rcu(&p->list, &old_p->list);
>  	return 0;
>  }
>  
> @@ -420,18 +396,18 @@ static inline void add_aggr_kprobe(struc
>  	ap->break_handler = aggr_break_handler;
>  
>  	INIT_LIST_HEAD(&ap->list);
> -	list_add(&p->list, &ap->list);
> +	list_add_rcu(&p->list, &ap->list);
>  
>  	INIT_HLIST_NODE(&ap->hlist);
> -	hlist_del(&p->hlist);
> -	hlist_add_head(&ap->hlist,
> +	hlist_del_rcu(&p->hlist);
> +	hlist_add_head_rcu(&ap->hlist,
>  		&kprobe_table[hash_ptr(ap->addr, KPROBE_HASH_BITS)]);
>  }
>  
>  /*
>   * This is the second or subsequent kprobe at the address - handle
>   * the intricacies
> - * TODO: Move kcalloc outside the spinlock
> + * TODO: Move kcalloc outside the spin_lock
>   */
>  static int __kprobes register_aggr_kprobe(struct kprobe *old_p,
>  					  struct kprobe *p)
> @@ -457,7 +433,7 @@ static int __kprobes register_aggr_kprob
>  static inline void cleanup_kprobe(struct kprobe *p, unsigned long flags)
>  {
>  	arch_disarm_kprobe(p);
> -	hlist_del(&p->hlist);
> +	hlist_del_rcu(&p->hlist);
>  	spin_unlock_irqrestore(&kprobe_lock, flags);
>  	arch_remove_kprobe(p);
>  }
> @@ -465,11 +441,10 @@ static inline void cleanup_kprobe(struct
>  static inline void cleanup_aggr_kprobe(struct kprobe *old_p,
>  		struct kprobe *p, unsigned long flags)
>  {
> -	list_del(&p->list);
> -	if (list_empty(&old_p->list)) {
> +	list_del_rcu(&p->list);
> +	if (list_empty(&old_p->list))
>  		cleanup_kprobe(old_p, flags);
> -		kfree(old_p);
> -	} else
> +	else
>  		spin_unlock_irqrestore(&kprobe_lock, flags);
>  }
>  
> @@ -492,9 +467,9 @@ int __kprobes register_kprobe(struct kpr
>  	if ((ret = arch_prepare_kprobe(p)) != 0)
>  		goto rm_kprobe;
>  
> +	p->nmissed = 0;
>  	spin_lock_irqsave(&kprobe_lock, flags);
>  	old_p = get_kprobe(p->addr);
> -	p->nmissed = 0;
>  	if (old_p) {
>  		ret = register_aggr_kprobe(old_p, p);
>  		goto out;
> @@ -502,7 +477,7 @@ int __kprobes register_kprobe(struct kpr
>  
>  	arch_copy_kprobe(p);
>  	INIT_HLIST_NODE(&p->hlist);
> -	hlist_add_head(&p->hlist,
> +	hlist_add_head_rcu(&p->hlist,
>  		       &kprobe_table[hash_ptr(p->addr, KPROBE_HASH_BITS)]);
>  
>    	arch_arm_kprobe(p);
> @@ -523,10 +498,16 @@ void __kprobes unregister_kprobe(struct 
>  	spin_lock_irqsave(&kprobe_lock, flags);
>  	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);
> +
> +		synchronize_sched();
> +		if (old_p->pre_handler == aggr_pre_handler &&
> +				list_empty(&old_p->list))
> +			kfree(old_p);
>  	} else
>  		spin_unlock_irqrestore(&kprobe_lock, flags);
>  }
> @@ -603,13 +584,13 @@ void __kprobes unregister_kretprobe(stru
>  
>  	unregister_kprobe(&rp->kp);
>  	/* No race here */
> -	spin_lock_irqsave(&kprobe_lock, flags);
> +	spin_lock_irqsave(&kretprobe_lock, flags);
>  	free_rp_inst(rp);
>  	while ((ri = get_used_rp_inst(rp)) != NULL) {
>  		ri->rp = NULL;
>  		hlist_del(&ri->uflist);
>  	}
> -	spin_unlock_irqrestore(&kprobe_lock, flags);
> +	spin_unlock_irqrestore(&kretprobe_lock, flags);
>  }
>  
>  static int __init init_kprobes(void)
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> 

Re: [PATCH 8/9] Kprobes: Use RCU for (un)register synchronization - base changes

[Ananth N Mavinakayanahalli]

On Mon, Oct 17, 2005 at 10:44:36PM -0700, Paul E. McKenney wrote:
> On Mon, Oct 10, 2005 at 10:47:20AM -0400, Ananth N Mavinakayanahalli wrote:
> > From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> > 
> > Changes to the base kprobes infrastructure to use RCU for synchronization
> > during kprobe registration and unregistration. These changes coupled with
> > the arch kprobe changes (next in series):
> > 
> > a. serialize registration and unregistration of kprobes.
> > b. enable lockless execution of handlers. Handlers can now run in parallel.
> 
> A couple of questions interspersed...  Probably my limited knowledge of
> kprobes, but I have to ask...  Search for empty lines to find them.

Paul,

Thanks for the review... replies inlined...

Ananth

> 
> 						Thanx, Paul
> 
> > Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> > Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
> > ---
> > 
> >  include/linux/kprobes.h |    9 +---
> >  kernel/kprobes.c        |  101 +++++++++++++++++++-----------------------------
> >  2 files changed, 45 insertions(+), 65 deletions(-)
> > 
> > Index: linux-2.6.14-rc3/include/linux/kprobes.h
> > ===================================================================
> > --- linux-2.6.14-rc3.orig/include/linux/kprobes.h	2005-10-07 21:40:43.000000000 -0400
> > +++ linux-2.6.14-rc3/include/linux/kprobes.h	2005-10-07 21:40:49.000000000 -0400
> > @@ -34,6 +34,8 @@
> >  #include <linux/notifier.h>
> >  #include <linux/smp.h>
> >  #include <linux/percpu.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/rcupdate.h>
> >  
> >  #include <asm/kprobes.h>
> >  
> > @@ -146,10 +148,7 @@ struct kretprobe_instance {
> >  };
> >  
> >  #ifdef CONFIG_KPROBES
> > -/* Locks kprobe: irq must be disabled */
> > -void lock_kprobes(void);
> > -void unlock_kprobes(void);
> > -
> > +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);
> > @@ -160,7 +159,7 @@ extern void show_registers(struct pt_reg
> >  extern kprobe_opcode_t *get_insn_slot(void);
> >  extern void free_insn_slot(kprobe_opcode_t *slot);
> >  
> > -/* Get the kprobe at this addr (if any).  Must have called lock_kprobes */
> > +/* Get the kprobe at this addr (if any) - called under a rcu_read_lock() */
> 
> Since the update side uses synchronize_kernel() (in patch 9/9, right?),

Yes, synchronize_sched() to be precise.

> shouldn't the above "rcu_read_lock()" instead by preempt_disable()?

My reasoning was that since rcu_read_(un)lock is #defined to
preempt_(en)disable. But, I agree, since we use synchronize_sched() this
can just be preempt_disable()...
Suggestions?

> Also, some of the code in the earlier patches seems to do preempt_disable.
> For example, kprobe_handler() in arch/i386/kernel/kprobes.c.

Well, the convolution is due to the way kprobes work: 
- Breakpoint hit; execute pre_handler
- single-step on a copy of the original instruction; execute the
post_handler

We don't want to be preempted from the time of the breakpoint exception
until after the post_handler is run. I've taken care to ensure the
preempt_ calls from the various codepaths are balanced.

> In realtime kernels, synchronize_sched() does -not- guarantee that all
> rcu_read_lock() critical sections have finished, only that any
> preempt_disable() code sequences (explicit or implicit) have completed.

Are we assured that the preempt_ depth is also taken care of? If that is
the case, I think we are safe, since the matching preempt_enable() calls
are _after_ the post_handler is executed.

> >  struct kprobe *get_kprobe(void *addr);
> >  struct hlist_head * kretprobe_inst_table_head(struct task_struct *tsk);
> >  
> > Index: linux-2.6.14-rc3/kernel/kprobes.c
> > ===================================================================
> > --- linux-2.6.14-rc3.orig/kernel/kprobes.c	2005-10-07 21:40:43.000000000 -0400
> > +++ linux-2.6.14-rc3/kernel/kprobes.c	2005-10-07 21:41:11.000000000 -0400
> > @@ -32,7 +32,6 @@
> >   *		<prasanna@in.ibm.com> added function-return probes.
> >   */
> >  #include <linux/kprobes.h>
> > -#include <linux/spinlock.h>
> >  #include <linux/hash.h>
> >  #include <linux/init.h>
> >  #include <linux/module.h>
> > @@ -48,8 +47,8 @@
> >  static struct hlist_head kprobe_table[KPROBE_TABLE_SIZE];
> >  static struct hlist_head kretprobe_inst_table[KPROBE_TABLE_SIZE];
> >  
> > -unsigned int kprobe_cpu = NR_CPUS;
> > -static DEFINE_SPINLOCK(kprobe_lock);
> > +static DEFINE_SPINLOCK(kprobe_lock);	/* Protects kprobe_table */
> > +DEFINE_SPINLOCK(kretprobe_lock);	/* Protects kretprobe_inst_table */
> >  static DEFINE_PER_CPU(struct kprobe *, kprobe_instance) = NULL;
> >  
> >  /*
> > @@ -152,41 +151,6 @@ void __kprobes free_insn_slot(kprobe_opc
> >  	}
> >  }
> >  
> > -/* Locks kprobe: irqs must be disabled */
> > -void __kprobes lock_kprobes(void)
> > -{
> > -	unsigned long flags = 0;
> > -
> > -	/* Avoiding local interrupts to happen right after we take the kprobe_lock
> > -	 * and before we get a chance to update kprobe_cpu, this to prevent
> > -	 * deadlock when we have a kprobe on ISR routine and a kprobe on task
> > -	 * routine
> > -	 */
> > -	local_irq_save(flags);
> > -
> > -	spin_lock(&kprobe_lock);
> > -	kprobe_cpu = smp_processor_id();
> > -
> > - 	local_irq_restore(flags);
> > -}
> > -
> > -void __kprobes unlock_kprobes(void)
> > -{
> > -	unsigned long flags = 0;
> > -
> > -	/* Avoiding local interrupts to happen right after we update
> > -	 * kprobe_cpu and before we get a a chance to release kprobe_lock,
> > -	 * this to prevent deadlock when we have a kprobe on ISR routine and
> > -	 * a kprobe on task routine
> > -	 */
> > -	local_irq_save(flags);
> > -
> > -	kprobe_cpu = NR_CPUS;
> > -	spin_unlock(&kprobe_lock);
> > -
> > - 	local_irq_restore(flags);
> > -}
> > -
> >  /* We have preemption disabled.. so it is safe to use __ versions */
> >  static inline void set_kprobe_instance(struct kprobe *kp)
> >  {
> > @@ -198,14 +162,19 @@ static inline void reset_kprobe_instance
> >  	__get_cpu_var(kprobe_instance) = NULL;
> >  }
> >  
> > -/* You have to be holding the kprobe_lock */
> > +/*
> > + * This routine is called either:
> > + * 	- under the kprobe_lock spinlock - during kprobe_[un]register()
> > + * 				OR
> > + * 	- under an rcu_read_lock() - from arch/xxx/kernel/kprobes.c
> 
> Same here -- shouldn't the rcu_read_lock() be preempt_disable()?

This is the same routine referred to in the comment above.

> > + */
> >  struct kprobe __kprobes *get_kprobe(void *addr)
> >  {
> >  	struct hlist_head *head;
> >  	struct hlist_node *node;
> >  
> >  	head = &kprobe_table[hash_ptr(addr, KPROBE_HASH_BITS)];
> > -	hlist_for_each(node, head) {
> > +	hlist_for_each_rcu(node, head) {
> >  		struct kprobe *p = hlist_entry(node, struct kprobe, hlist);
> >  		if (p->addr == addr)
> >  			return p;
> > @@ -221,7 +190,7 @@ static int __kprobes aggr_pre_handler(st
> >  {
> >  	struct kprobe *kp;
> >  
> > -	list_for_each_entry(kp, &p->list, list) {
> > +	list_for_each_entry_rcu(kp, &p->list, list) {
> >  		if (kp->pre_handler) {
> >  			set_kprobe_instance(kp);
> >  			if (kp->pre_handler(kp, regs))
> > @@ -237,7 +206,7 @@ static void __kprobes aggr_post_handler(
> >  {
> >  	struct kprobe *kp;
> >  
> > -	list_for_each_entry(kp, &p->list, list) {
> > +	list_for_each_entry_rcu(kp, &p->list, list) {
> >  		if (kp->post_handler) {
> >  			set_kprobe_instance(kp);
> >  			kp->post_handler(kp, regs, flags);
> > @@ -276,6 +245,7 @@ static int __kprobes aggr_break_handler(
> >  	return ret;
> >  }
> >  
> > +/* Called with kretprobe_lock held */
> >  struct kretprobe_instance __kprobes *get_free_rp_inst(struct kretprobe *rp)
> >  {
> >  	struct hlist_node *node;
> > @@ -285,6 +255,7 @@ struct kretprobe_instance __kprobes *get
> >  	return NULL;
> >  }
> >  
> > +/* Called with kretprobe_lock held */
> >  static struct kretprobe_instance __kprobes *get_used_rp_inst(struct kretprobe
> >  							      *rp)
> >  {
> > @@ -295,6 +266,7 @@ static struct kretprobe_instance __kprob
> >  	return NULL;
> >  }
> >  
> > +/* Called with kretprobe_lock held */
> >  void __kprobes add_rp_inst(struct kretprobe_instance *ri)
> >  {
> >  	/*
> > @@ -313,6 +285,7 @@ void __kprobes add_rp_inst(struct kretpr
> >  	hlist_add_head(&ri->uflist, &ri->rp->used_instances);
> >  }
> >  
> > +/* Called with kretprobe_lock held */
> >  void __kprobes recycle_rp_inst(struct kretprobe_instance *ri)
> >  {
> >  	/* remove rp inst off the rprobe_inst_table */
> > @@ -346,13 +319,13 @@ void __kprobes kprobe_flush_task(struct 
> >  	struct hlist_node *node, *tmp;
> >  	unsigned long flags = 0;
> >  
> > -	spin_lock_irqsave(&kprobe_lock, flags);
> > +	spin_lock_irqsave(&kretprobe_lock, flags);
> >          head = kretprobe_inst_table_head(current);
> >          hlist_for_each_entry_safe(ri, node, tmp, head, hlist) {
> >                  if (ri->task == tk)
> >                          recycle_rp_inst(ri);
> >          }
> > -	spin_unlock_irqrestore(&kprobe_lock, flags);
> > +	spin_unlock_irqrestore(&kretprobe_lock, flags);
> >  }
> >  
> >  /*
> > @@ -363,9 +336,12 @@ static int __kprobes pre_handler_kretpro
> >  					   struct pt_regs *regs)
> >  {
> >  	struct kretprobe *rp = container_of(p, struct kretprobe, kp);
> > +	unsigned long flags = 0;
> >  
> >  	/*TODO: consider to only swap the RA after the last pre_handler fired */
> > +	spin_lock_irqsave(&kretprobe_lock, flags);
> >  	arch_prepare_kretprobe(rp, regs);
> > +	spin_unlock_irqrestore(&kretprobe_lock, flags);
> >  	return 0;
> >  }
> >  
> > @@ -396,13 +372,13 @@ static int __kprobes add_new_kprobe(stru
> >          struct kprobe *kp;
> >  
> >  	if (p->break_handler) {
> > -		list_for_each_entry(kp, &old_p->list, list) {
> > +		list_for_each_entry_rcu(kp, &old_p->list, list) {
> >  			if (kp->break_handler)
> >  				return -EEXIST;
> >  		}
> > -		list_add_tail(&p->list, &old_p->list);
> > +		list_add_tail_rcu(&p->list, &old_p->list);
> >  	} else
> > -		list_add(&p->list, &old_p->list);
> > +		list_add_rcu(&p->list, &old_p->list);
> >  	return 0;
> >  }
> >  
> > @@ -420,18 +396,18 @@ static inline void add_aggr_kprobe(struc
> >  	ap->break_handler = aggr_break_handler;
> >  
> >  	INIT_LIST_HEAD(&ap->list);
> > -	list_add(&p->list, &ap->list);
> > +	list_add_rcu(&p->list, &ap->list);
> >  
> >  	INIT_HLIST_NODE(&ap->hlist);
> > -	hlist_del(&p->hlist);
> > -	hlist_add_head(&ap->hlist,
> > +	hlist_del_rcu(&p->hlist);
> > +	hlist_add_head_rcu(&ap->hlist,
> >  		&kprobe_table[hash_ptr(ap->addr, KPROBE_HASH_BITS)]);
> >  }
> >  
> >  /*
> >   * This is the second or subsequent kprobe at the address - handle
> >   * the intricacies
> > - * TODO: Move kcalloc outside the spinlock
> > + * TODO: Move kcalloc outside the spin_lock
> >   */
> >  static int __kprobes register_aggr_kprobe(struct kprobe *old_p,
> >  					  struct kprobe *p)
> > @@ -457,7 +433,7 @@ static int __kprobes register_aggr_kprob
> >  static inline void cleanup_kprobe(struct kprobe *p, unsigned long flags)
> >  {
> >  	arch_disarm_kprobe(p);
> > -	hlist_del(&p->hlist);
> > +	hlist_del_rcu(&p->hlist);
> >  	spin_unlock_irqrestore(&kprobe_lock, flags);
> >  	arch_remove_kprobe(p);
> >  }
> > @@ -465,11 +441,10 @@ static inline void cleanup_kprobe(struct
> >  static inline void cleanup_aggr_kprobe(struct kprobe *old_p,
> >  		struct kprobe *p, unsigned long flags)
> >  {
> > -	list_del(&p->list);
> > -	if (list_empty(&old_p->list)) {
> > +	list_del_rcu(&p->list);
> > +	if (list_empty(&old_p->list))
> >  		cleanup_kprobe(old_p, flags);
> > -		kfree(old_p);
> > -	} else
> > +	else
> >  		spin_unlock_irqrestore(&kprobe_lock, flags);
> >  }
> >  
> > @@ -492,9 +467,9 @@ int __kprobes register_kprobe(struct kpr
> >  	if ((ret = arch_prepare_kprobe(p)) != 0)
> >  		goto rm_kprobe;
> >  
> > +	p->nmissed = 0;
> >  	spin_lock_irqsave(&kprobe_lock, flags);
> >  	old_p = get_kprobe(p->addr);
> > -	p->nmissed = 0;
> >  	if (old_p) {
> >  		ret = register_aggr_kprobe(old_p, p);
> >  		goto out;
> > @@ -502,7 +477,7 @@ int __kprobes register_kprobe(struct kpr
> >  
> >  	arch_copy_kprobe(p);
> >  	INIT_HLIST_NODE(&p->hlist);
> > -	hlist_add_head(&p->hlist,
> > +	hlist_add_head_rcu(&p->hlist,
> >  		       &kprobe_table[hash_ptr(p->addr, KPROBE_HASH_BITS)]);
> >  
> >    	arch_arm_kprobe(p);
> > @@ -523,10 +498,16 @@ void __kprobes unregister_kprobe(struct 
> >  	spin_lock_irqsave(&kprobe_lock, flags);
> >  	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);
> > +
> > +		synchronize_sched();
> > +		if (old_p->pre_handler == aggr_pre_handler &&
> > +				list_empty(&old_p->list))
> > +			kfree(old_p);
> >  	} else
> >  		spin_unlock_irqrestore(&kprobe_lock, flags);
> >  }
> > @@ -603,13 +584,13 @@ void __kprobes unregister_kretprobe(stru
> >  
> >  	unregister_kprobe(&rp->kp);
> >  	/* No race here */
> > -	spin_lock_irqsave(&kprobe_lock, flags);
> > +	spin_lock_irqsave(&kretprobe_lock, flags);
> >  	free_rp_inst(rp);
> >  	while ((ri = get_used_rp_inst(rp)) != NULL) {
> >  		ri->rp = NULL;
> >  		hlist_del(&ri->uflist);
> >  	}
> > -	spin_unlock_irqrestore(&kprobe_lock, flags);
> > +	spin_unlock_irqrestore(&kretprobe_lock, flags);
> >  }
> >  
> >  static int __init init_kprobes(void)
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> > 
> > 

Re: [PATCH 8/9] Kprobes: Use RCU for (un)register synchronization - base changes

[Paul E. McKenney]

On Tue, Oct 18, 2005 at 10:43:23AM -0400, Ananth N Mavinakayanahalli wrote:
> On Mon, Oct 17, 2005 at 10:44:36PM -0700, Paul E. McKenney wrote:
> > On Mon, Oct 10, 2005 at 10:47:20AM -0400, Ananth N Mavinakayanahalli wrote:
> > > From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> > > 
> > > Changes to the base kprobes infrastructure to use RCU for synchronization
> > > during kprobe registration and unregistration. These changes coupled with
> > > the arch kprobe changes (next in series):
> > > 
> > > a. serialize registration and unregistration of kprobes.
> > > b. enable lockless execution of handlers. Handlers can now run in parallel.
> > 
> > A couple of questions interspersed...  Probably my limited knowledge of
> > kprobes, but I have to ask...  Search for empty lines to find them.
> 
> Paul,
> 
> Thanks for the review... replies inlined...
> 
> Ananth
> 
> > 
> > 						Thanx, Paul
> > 
> > > Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> > > Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
> > > ---
> > > 
> > >  include/linux/kprobes.h |    9 +---
> > >  kernel/kprobes.c        |  101 +++++++++++++++++++-----------------------------
> > >  2 files changed, 45 insertions(+), 65 deletions(-)
> > > 
> > > Index: linux-2.6.14-rc3/include/linux/kprobes.h
> > > ===================================================================
> > > --- linux-2.6.14-rc3.orig/include/linux/kprobes.h	2005-10-07 21:40:43.000000000 -0400
> > > +++ linux-2.6.14-rc3/include/linux/kprobes.h	2005-10-07 21:40:49.000000000 -0400
> > > @@ -34,6 +34,8 @@
> > >  #include <linux/notifier.h>
> > >  #include <linux/smp.h>
> > >  #include <linux/percpu.h>
> > > +#include <linux/spinlock.h>
> > > +#include <linux/rcupdate.h>
> > >  
> > >  #include <asm/kprobes.h>
> > >  
> > > @@ -146,10 +148,7 @@ struct kretprobe_instance {
> > >  };
> > >  
> > >  #ifdef CONFIG_KPROBES
> > > -/* Locks kprobe: irq must be disabled */
> > > -void lock_kprobes(void);
> > > -void unlock_kprobes(void);
> > > -
> > > +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);
> > > @@ -160,7 +159,7 @@ extern void show_registers(struct pt_reg
> > >  extern kprobe_opcode_t *get_insn_slot(void);
> > >  extern void free_insn_slot(kprobe_opcode_t *slot);
> > >  
> > > -/* Get the kprobe at this addr (if any).  Must have called lock_kprobes */
> > > +/* Get the kprobe at this addr (if any) - called under a rcu_read_lock() */
> > 
> > Since the update side uses synchronize_kernel() (in patch 9/9, right?),
> 
> Yes, synchronize_sched() to be precise.

Good, am not going blind yet.  ;-)

> > shouldn't the above "rcu_read_lock()" instead by preempt_disable()?
> 
> My reasoning was that since rcu_read_(un)lock is #defined to
> preempt_(en)disable. But, I agree, since we use synchronize_sched() this
> can just be preempt_disable()...
> Suggestions?

In some realtime-friendly RCU implementations, the RCU read-side critical
sections do not disable preemption.  So, if you are using synchronize_sched()
on the update side, you need to use preempt_disable() (or some other
primitive that disables preemption, for example, any of the primitives
that disables hardware interrupts) on the read side.

>From Documentation/RCU/whatisRCU.txt, the correspondence between the
update-side primitive and the read-side primitives needs to be as follows:

	Defer			Protect

a.	synchronize_rcu()	rcu_read_lock() / rcu_read_unlock()
	call_rcu()

b.	call_rcu_bh()		rcu_read_lock_bh() / rcu_read_unlock_bh()

c.	synchronize_sched()	preempt_disable() / preempt_enable()
				local_irq_save() / local_irq_restore()
				hardirq enter / hardirq exit
				NMI enter / NMI exit

> > Also, some of the code in the earlier patches seems to do preempt_disable.
> > For example, kprobe_handler() in arch/i386/kernel/kprobes.c.
> 
> Well, the convolution is due to the way kprobes work: 
> - Breakpoint hit; execute pre_handler
> - single-step on a copy of the original instruction; execute the
> post_handler
> 
> We don't want to be preempted from the time of the breakpoint exception
> until after the post_handler is run. I've taken care to ensure the
> preempt_ calls from the various codepaths are balanced.

OK, didn't follow this -- thank you for the explanation!  This sequence
of events happens on each execution of the kprobe handler, or is there
some optimization for subsequent invocations of the same kprobe handler?

If the former, then perhaps there is some way to make the preempt_disable()
that protects the single stepping also protect the probe lookup.

> > In realtime kernels, synchronize_sched() does -not- guarantee that all
> > rcu_read_lock() critical sections have finished, only that any
> > preempt_disable() code sequences (explicit or implicit) have completed.
> 
> Are we assured that the preempt_ depth is also taken care of? If that is
> the case, I think we are safe, since the matching preempt_enable() calls
> are _after_ the post_handler is executed.

Seems so to me -- though if I understand you correctly, you should then
be able to simply remove the rcu_read_lock() and rcu_read_unlock() calls,
since that code sequence would be covered by the preempt_disable().

Or am I missing something?

> > >  struct kprobe *get_kprobe(void *addr);
> > >  struct hlist_head * kretprobe_inst_table_head(struct task_struct *tsk);
> > >  
> > > Index: linux-2.6.14-rc3/kernel/kprobes.c
> > > ===================================================================
> > > --- linux-2.6.14-rc3.orig/kernel/kprobes.c	2005-10-07 21:40:43.000000000 -0400
> > > +++ linux-2.6.14-rc3/kernel/kprobes.c	2005-10-07 21:41:11.000000000 -0400
> > > @@ -32,7 +32,6 @@
> > >   *		<prasanna@in.ibm.com> added function-return probes.
> > >   */
> > >  #include <linux/kprobes.h>
> > > -#include <linux/spinlock.h>
> > >  #include <linux/hash.h>
> > >  #include <linux/init.h>
> > >  #include <linux/module.h>
> > > @@ -48,8 +47,8 @@
> > >  static struct hlist_head kprobe_table[KPROBE_TABLE_SIZE];
> > >  static struct hlist_head kretprobe_inst_table[KPROBE_TABLE_SIZE];
> > >  
> > > -unsigned int kprobe_cpu = NR_CPUS;
> > > -static DEFINE_SPINLOCK(kprobe_lock);
> > > +static DEFINE_SPINLOCK(kprobe_lock);	/* Protects kprobe_table */
> > > +DEFINE_SPINLOCK(kretprobe_lock);	/* Protects kretprobe_inst_table */
> > >  static DEFINE_PER_CPU(struct kprobe *, kprobe_instance) = NULL;
> > >  
> > >  /*
> > > @@ -152,41 +151,6 @@ void __kprobes free_insn_slot(kprobe_opc
> > >  	}
> > >  }
> > >  
> > > -/* Locks kprobe: irqs must be disabled */
> > > -void __kprobes lock_kprobes(void)
> > > -{
> > > -	unsigned long flags = 0;
> > > -
> > > -	/* Avoiding local interrupts to happen right after we take the kprobe_lock
> > > -	 * and before we get a chance to update kprobe_cpu, this to prevent
> > > -	 * deadlock when we have a kprobe on ISR routine and a kprobe on task
> > > -	 * routine
> > > -	 */
> > > -	local_irq_save(flags);
> > > -
> > > -	spin_lock(&kprobe_lock);
> > > -	kprobe_cpu = smp_processor_id();
> > > -
> > > - 	local_irq_restore(flags);
> > > -}
> > > -
> > > -void __kprobes unlock_kprobes(void)
> > > -{
> > > -	unsigned long flags = 0;
> > > -
> > > -	/* Avoiding local interrupts to happen right after we update
> > > -	 * kprobe_cpu and before we get a a chance to release kprobe_lock,
> > > -	 * this to prevent deadlock when we have a kprobe on ISR routine and
> > > -	 * a kprobe on task routine
> > > -	 */
> > > -	local_irq_save(flags);
> > > -
> > > -	kprobe_cpu = NR_CPUS;
> > > -	spin_unlock(&kprobe_lock);
> > > -
> > > - 	local_irq_restore(flags);
> > > -}
> > > -
> > >  /* We have preemption disabled.. so it is safe to use __ versions */
> > >  static inline void set_kprobe_instance(struct kprobe *kp)
> > >  {
> > > @@ -198,14 +162,19 @@ static inline void reset_kprobe_instance
> > >  	__get_cpu_var(kprobe_instance) = NULL;
> > >  }
> > >  
> > > -/* You have to be holding the kprobe_lock */
> > > +/*
> > > + * This routine is called either:
> > > + * 	- under the kprobe_lock spinlock - during kprobe_[un]register()
> > > + * 				OR
> > > + * 	- under an rcu_read_lock() - from arch/xxx/kernel/kprobes.c
> > 
> > Same here -- shouldn't the rcu_read_lock() be preempt_disable()?
> 
> This is the same routine referred to in the comment above.

OK, same responses apply.  ;-)

> > > + */
> > >  struct kprobe __kprobes *get_kprobe(void *addr)
> > >  {
> > >  	struct hlist_head *head;
> > >  	struct hlist_node *node;
> > >  
> > >  	head = &kprobe_table[hash_ptr(addr, KPROBE_HASH_BITS)];
> > > -	hlist_for_each(node, head) {
> > > +	hlist_for_each_rcu(node, head) {
> > >  		struct kprobe *p = hlist_entry(node, struct kprobe, hlist);
> > >  		if (p->addr == addr)
> > >  			return p;
> > > @@ -221,7 +190,7 @@ static int __kprobes aggr_pre_handler(st
> > >  {
> > >  	struct kprobe *kp;
> > >  
> > > -	list_for_each_entry(kp, &p->list, list) {
> > > +	list_for_each_entry_rcu(kp, &p->list, list) {
> > >  		if (kp->pre_handler) {
> > >  			set_kprobe_instance(kp);
> > >  			if (kp->pre_handler(kp, regs))
> > > @@ -237,7 +206,7 @@ static void __kprobes aggr_post_handler(
> > >  {
> > >  	struct kprobe *kp;
> > >  
> > > -	list_for_each_entry(kp, &p->list, list) {
> > > +	list_for_each_entry_rcu(kp, &p->list, list) {
> > >  		if (kp->post_handler) {
> > >  			set_kprobe_instance(kp);
> > >  			kp->post_handler(kp, regs, flags);
> > > @@ -276,6 +245,7 @@ static int __kprobes aggr_break_handler(
> > >  	return ret;
> > >  }
> > >  
> > > +/* Called with kretprobe_lock held */
> > >  struct kretprobe_instance __kprobes *get_free_rp_inst(struct kretprobe *rp)
> > >  {
> > >  	struct hlist_node *node;
> > > @@ -285,6 +255,7 @@ struct kretprobe_instance __kprobes *get
> > >  	return NULL;
> > >  }
> > >  
> > > +/* Called with kretprobe_lock held */
> > >  static struct kretprobe_instance __kprobes *get_used_rp_inst(struct kretprobe
> > >  							      *rp)
> > >  {
> > > @@ -295,6 +266,7 @@ static struct kretprobe_instance __kprob
> > >  	return NULL;
> > >  }
> > >  
> > > +/* Called with kretprobe_lock held */
> > >  void __kprobes add_rp_inst(struct kretprobe_instance *ri)
> > >  {
> > >  	/*
> > > @@ -313,6 +285,7 @@ void __kprobes add_rp_inst(struct kretpr
> > >  	hlist_add_head(&ri->uflist, &ri->rp->used_instances);
> > >  }
> > >  
> > > +/* Called with kretprobe_lock held */
> > >  void __kprobes recycle_rp_inst(struct kretprobe_instance *ri)
> > >  {
> > >  	/* remove rp inst off the rprobe_inst_table */
> > > @@ -346,13 +319,13 @@ void __kprobes kprobe_flush_task(struct 
> > >  	struct hlist_node *node, *tmp;
> > >  	unsigned long flags = 0;
> > >  
> > > -	spin_lock_irqsave(&kprobe_lock, flags);
> > > +	spin_lock_irqsave(&kretprobe_lock, flags);
> > >          head = kretprobe_inst_table_head(current);
> > >          hlist_for_each_entry_safe(ri, node, tmp, head, hlist) {
> > >                  if (ri->task == tk)
> > >                          recycle_rp_inst(ri);
> > >          }
> > > -	spin_unlock_irqrestore(&kprobe_lock, flags);
> > > +	spin_unlock_irqrestore(&kretprobe_lock, flags);
> > >  }
> > >  
> > >  /*
> > > @@ -363,9 +336,12 @@ static int __kprobes pre_handler_kretpro
> > >  					   struct pt_regs *regs)
> > >  {
> > >  	struct kretprobe *rp = container_of(p, struct kretprobe, kp);
> > > +	unsigned long flags = 0;
> > >  
> > >  	/*TODO: consider to only swap the RA after the last pre_handler fired */
> > > +	spin_lock_irqsave(&kretprobe_lock, flags);
> > >  	arch_prepare_kretprobe(rp, regs);
> > > +	spin_unlock_irqrestore(&kretprobe_lock, flags);
> > >  	return 0;
> > >  }
> > >  
> > > @@ -396,13 +372,13 @@ static int __kprobes add_new_kprobe(stru
> > >          struct kprobe *kp;
> > >  
> > >  	if (p->break_handler) {
> > > -		list_for_each_entry(kp, &old_p->list, list) {
> > > +		list_for_each_entry_rcu(kp, &old_p->list, list) {
> > >  			if (kp->break_handler)
> > >  				return -EEXIST;
> > >  		}
> > > -		list_add_tail(&p->list, &old_p->list);
> > > +		list_add_tail_rcu(&p->list, &old_p->list);
> > >  	} else
> > > -		list_add(&p->list, &old_p->list);
> > > +		list_add_rcu(&p->list, &old_p->list);
> > >  	return 0;
> > >  }
> > >  
> > > @@ -420,18 +396,18 @@ static inline void add_aggr_kprobe(struc
> > >  	ap->break_handler = aggr_break_handler;
> > >  
> > >  	INIT_LIST_HEAD(&ap->list);
> > > -	list_add(&p->list, &ap->list);
> > > +	list_add_rcu(&p->list, &ap->list);
> > >  
> > >  	INIT_HLIST_NODE(&ap->hlist);
> > > -	hlist_del(&p->hlist);
> > > -	hlist_add_head(&ap->hlist,
> > > +	hlist_del_rcu(&p->hlist);
> > > +	hlist_add_head_rcu(&ap->hlist,
> > >  		&kprobe_table[hash_ptr(ap->addr, KPROBE_HASH_BITS)]);
> > >  }
> > >  
> > >  /*
> > >   * This is the second or subsequent kprobe at the address - handle
> > >   * the intricacies
> > > - * TODO: Move kcalloc outside the spinlock
> > > + * TODO: Move kcalloc outside the spin_lock
> > >   */
> > >  static int __kprobes register_aggr_kprobe(struct kprobe *old_p,
> > >  					  struct kprobe *p)
> > > @@ -457,7 +433,7 @@ static int __kprobes register_aggr_kprob
> > >  static inline void cleanup_kprobe(struct kprobe *p, unsigned long flags)
> > >  {
> > >  	arch_disarm_kprobe(p);
> > > -	hlist_del(&p->hlist);
> > > +	hlist_del_rcu(&p->hlist);
> > >  	spin_unlock_irqrestore(&kprobe_lock, flags);
> > >  	arch_remove_kprobe(p);
> > >  }
> > > @@ -465,11 +441,10 @@ static inline void cleanup_kprobe(struct
> > >  static inline void cleanup_aggr_kprobe(struct kprobe *old_p,
> > >  		struct kprobe *p, unsigned long flags)
> > >  {
> > > -	list_del(&p->list);
> > > -	if (list_empty(&old_p->list)) {
> > > +	list_del_rcu(&p->list);
> > > +	if (list_empty(&old_p->list))
> > >  		cleanup_kprobe(old_p, flags);
> > > -		kfree(old_p);
> > > -	} else
> > > +	else
> > >  		spin_unlock_irqrestore(&kprobe_lock, flags);
> > >  }
> > >  
> > > @@ -492,9 +467,9 @@ int __kprobes register_kprobe(struct kpr
> > >  	if ((ret = arch_prepare_kprobe(p)) != 0)
> > >  		goto rm_kprobe;
> > >  
> > > +	p->nmissed = 0;
> > >  	spin_lock_irqsave(&kprobe_lock, flags);
> > >  	old_p = get_kprobe(p->addr);
> > > -	p->nmissed = 0;
> > >  	if (old_p) {
> > >  		ret = register_aggr_kprobe(old_p, p);
> > >  		goto out;
> > > @@ -502,7 +477,7 @@ int __kprobes register_kprobe(struct kpr
> > >  
> > >  	arch_copy_kprobe(p);
> > >  	INIT_HLIST_NODE(&p->hlist);
> > > -	hlist_add_head(&p->hlist,
> > > +	hlist_add_head_rcu(&p->hlist,
> > >  		       &kprobe_table[hash_ptr(p->addr, KPROBE_HASH_BITS)]);
> > >  
> > >    	arch_arm_kprobe(p);
> > > @@ -523,10 +498,16 @@ void __kprobes unregister_kprobe(struct 
> > >  	spin_lock_irqsave(&kprobe_lock, flags);
> > >  	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);
> > > +
> > > +		synchronize_sched();
> > > +		if (old_p->pre_handler == aggr_pre_handler &&
> > > +				list_empty(&old_p->list))
> > > +			kfree(old_p);
> > >  	} else
> > >  		spin_unlock_irqrestore(&kprobe_lock, flags);
> > >  }
> > > @@ -603,13 +584,13 @@ void __kprobes unregister_kretprobe(stru
> > >  
> > >  	unregister_kprobe(&rp->kp);
> > >  	/* No race here */
> > > -	spin_lock_irqsave(&kprobe_lock, flags);
> > > +	spin_lock_irqsave(&kretprobe_lock, flags);
> > >  	free_rp_inst(rp);
> > >  	while ((ri = get_used_rp_inst(rp)) != NULL) {
> > >  		ri->rp = NULL;
> > >  		hlist_del(&ri->uflist);
> > >  	}
> > > -	spin_unlock_irqrestore(&kprobe_lock, flags);
> > > +	spin_unlock_irqrestore(&kretprobe_lock, flags);
> > >  }
> > >  
> > >  static int __init init_kprobes(void)
> > > -
> > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > Please read the FAQ at  http://www.tux.org/lkml/
> > > 
> > > 
> 

Re: [PATCH 8/9] Kprobes: Use RCU for (un)register synchronization - base changes

[Ananth N Mavinakayanahalli]

On Tue, Oct 18, 2005 at 09:31:16AM -0700, Paul E. McKenney wrote:
> On Tue, Oct 18, 2005 at 10:43:23AM -0400, Ananth N Mavinakayanahalli wrote:
> > On Mon, Oct 17, 2005 at 10:44:36PM -0700, Paul E. McKenney wrote:
> > > On Mon, Oct 10, 2005 at 10:47:20AM -0400, Ananth N Mavinakayanahalli wrote:
> > > > From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

<snip>

> > > > -/* Get the kprobe at this addr (if any).  Must have called lock_kprobes */
> > > > +/* Get the kprobe at this addr (if any) - called under a rcu_read_lock() */
> > > 
> > > Since the update side uses synchronize_kernel() (in patch 9/9, right?),
> > 
> > Yes, synchronize_sched() to be precise.
> 
> Good, am not going blind yet.  ;-)

:)

> > > shouldn't the above "rcu_read_lock()" instead by preempt_disable()?
> > 
> > My reasoning was that since rcu_read_(un)lock is #defined to
> > preempt_(en)disable. But, I agree, since we use synchronize_sched() this
> > can just be preempt_disable()...
> > Suggestions?
> 
> In some realtime-friendly RCU implementations, the RCU read-side critical
> sections do not disable preemption.  So, if you are using synchronize_sched()
> on the update side, you need to use preempt_disable() (or some other
> primitive that disables preemption, for example, any of the primitives
> that disables hardware interrupts) on the read side.
> 
> >From Documentation/RCU/whatisRCU.txt, the correspondence between the
> update-side primitive and the read-side primitives needs to be as follows:
> 
> 	Defer			Protect
> 
> a.	synchronize_rcu()	rcu_read_lock() / rcu_read_unlock()
> 	call_rcu()
> 
> b.	call_rcu_bh()		rcu_read_lock_bh() / rcu_read_unlock_bh()
> 
> c.	synchronize_sched()	preempt_disable() / preempt_enable()
> 				local_irq_save() / local_irq_restore()
> 				hardirq enter / hardirq exit
> 				NMI enter / NMI exit

Thanks for the pointers.

> > > Also, some of the code in the earlier patches seems to do preempt_disable.
> > > For example, kprobe_handler() in arch/i386/kernel/kprobes.c.
> > 
> > Well, the convolution is due to the way kprobes work: 
> > - Breakpoint hit; execute pre_handler
> > - single-step on a copy of the original instruction; execute the
> > post_handler
> > 
> > We don't want to be preempted from the time of the breakpoint exception
> > until after the post_handler is run. I've taken care to ensure the
> > preempt_ calls from the various codepaths are balanced.
> 
> OK, didn't follow this -- thank you for the explanation!  This sequence
> of events happens on each execution of the kprobe handler, or is there
> some optimization for subsequent invocations of the same kprobe handler?
> 
> If the former, then perhaps there is some way to make the preempt_disable()
> that protects the single stepping also protect the probe lookup.

It is the former.

> > > In realtime kernels, synchronize_sched() does -not- guarantee that all
> > > rcu_read_lock() critical sections have finished, only that any
> > > preempt_disable() code sequences (explicit or implicit) have completed.
> > 
> > Are we assured that the preempt_ depth is also taken care of? If that is
> > the case, I think we are safe, since the matching preempt_enable() calls
> > are _after_ the post_handler is executed.
> 
> Seems so to me -- though if I understand you correctly, you should then
> be able to simply remove the rcu_read_lock() and rcu_read_unlock() calls,
> since that code sequence would be covered by the preempt_disable().
> 
> Or am I missing something?

Nope, you are right. The extra depth was introduced so the older kprobes
code wouldn't throw warnings on every smp_processor_id() invocation
without preemption disabled. With the current changes, we can get rid of
the extra depth with a little bit of code rearrangement. I am on it now.

Thanks,
Ananth