[PATCH] Kprobes: rearrange preempt_disable/enable() calls
authorAnanth N Mavinakayanahalli <ananth@in.ibm.com>
Mon, 7 Nov 2005 09:00:07 +0000 (01:00 -0800)
committerLinus Torvalds <torvalds@g5.osdl.org>
Mon, 7 Nov 2005 15:53:45 +0000 (07:53 -0800)
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.

This patch:

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>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
arch/i386/kernel/kprobes.c
arch/ia64/kernel/kprobes.c
arch/ppc64/kernel/kprobes.c
arch/sparc64/kernel/kprobes.c
arch/x86_64/kernel/kprobes.c

index 6345b430b105fd913c1fdca36768910e2f7fd580..fd35039859e610d46559a7227789e6a23458681f 100644 (file)
@@ -158,8 +158,6 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
        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(struct pt_regs *regs)
                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;
 }
 
@@ -313,11 +315,11 @@ int __kprobes trampoline_probe_handler(struct kprobe *p, struct pt_regs *regs)
        unlock_kprobes();
        preempt_enable_no_resched();
 
-        /*
-         * By returning a non-zero value, we are telling
-         * kprobe_handler() that we have handled unlocking
-         * and re-enabling preemption.
-         */
+       /*
+        * By returning a non-zero value, we are telling
+        * kprobe_handler() that we have handled unlocking
+        * and re-enabling preemption
+        */
         return 1;
 }
 
@@ -453,29 +455,29 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
                                       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 kprobe *p, struct pt_regs *regs)
 
 void __kprobes jprobe_return(void)
 {
-       preempt_enable_no_resched();
        asm volatile ("       xchgl   %%ebx,%%esp     \n"
                      "       int3                      \n"
                      "       .globl jprobe_return_end  \n"
index 471086b808a4b942781aa19abe940fb08dc8533b..1e80ec80dd215afe725fe2f07d90c27325a25eb3 100644 (file)
@@ -395,7 +395,7 @@ int __kprobes trampoline_probe_handler(struct kprobe *p, struct pt_regs *regs)
         /*
          * 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 die_args *args)
        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(struct die_args *args)
                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(struct notifier_block *self,
                                       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 ed876a5178ae6bedf663488915c0dc27cdd0ff83..6071ee99f5cbb975beed8c8100d657b485468966 100644 (file)
@@ -209,6 +209,11 @@ static inline int kprobe_handler(struct pt_regs *regs)
                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 pt_regs *regs)
 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(struct kprobe *p, struct pt_regs *regs)
        regs->nip = orig_ret_address;
 
        unlock_kprobes();
+       preempt_enable_no_resched();
 
         /*
          * By returning a non-zero value, we are telling
index 0d66d07c8c6eead89fa6e5799ec6ba5fcf57873f..755a0d7d887fad22890e6cabf2a25de97fbb4d42 100644 (file)
@@ -118,8 +118,6 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
        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(struct pt_regs *regs)
                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(struct notifier_block *self,
                                       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 kprobe *p, struct pt_regs *regs)
 
 void __kprobes jprobe_return(void)
 {
-       preempt_enable_no_resched();
        __asm__ __volatile__(
                ".globl jprobe_return_trap_instruction\n"
 "jprobe_return_trap_instruction:\n\t"
index 76a28b007be95fcb17344fe48c7a1b93935a6bc0..ebfa2c9241ca906b7d7b03f267f15593fd37d1ad 100644 (file)
@@ -302,9 +302,6 @@ int __kprobes kprobe_handler(struct pt_regs *regs)
        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_regs *regs)
                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(struct kprobe *p, struct pt_regs *regs)
         /*
          * 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(struct notifier_block *self,
                                       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 kprobe *p, struct pt_regs *regs)
 
 void __kprobes jprobe_return(void)
 {
-       preempt_enable_no_resched();
        asm volatile ("       xchg   %%rbx,%%rsp     \n"
                      "       int3                      \n"
                      "       .globl jprobe_return_end  \n"