lkml.org 
[lkml]   [2007]   [Dec]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Subject[PATCH] x86: kprobes change kprobe_handler flow
From
Date
Make the control flow of kprobe_handler more obvious.

Collapse the separate if blocks/gotos with if/else blocks
this unifies the duplication of the check for a breakpoint
instruction race with another cpu.

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
Masami, please have a look at this, I think it's much more obvious
written this way. The way the old code fell through or not was rather
non-obvious. Some further work eliminating the nested returns and
creating a out: and preempt_out: target at the end of the function
would make it easier to notice preempt imbalances in later changes.

arch/x86/kernel/kprobes.c | 48 ++++++++++++++++----------------------------
1 files changed, 18 insertions(+), 30 deletions(-)

diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
index 4e33329..d656215 100644
--- a/arch/x86/kernel/kprobes.c
+++ b/arch/x86/kernel/kprobes.c
@@ -480,32 +480,22 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
preempt_disable();
kcb = get_kprobe_ctlblk();

- /* Check we're not actually recursing */
- if (kprobe_running()) {
- p = get_kprobe(addr);
- if (p) {
+ p = get_kprobe(addr);
+ if (p) {
+ /* Check we're not actually recursing */
+ if (kprobe_running()) {
ret = reenter_kprobe(p, regs, kcb);
if (kcb->kprobe_status == KPROBE_REENTER)
return 1;
+ goto no_kprobe;
} else {
- if (*addr != BREAKPOINT_INSTRUCTION) {
- /* The breakpoint instruction was removed by
- * another cpu right after we hit, no further
- * handling of this interrupt is appropriate
- */
- regs->ip = (unsigned long)addr;
- ret = 1;
- goto no_kprobe;
- }
- p = __get_cpu_var(current_kprobe);
- if (p->break_handler && p->break_handler(p, regs))
- goto ss_probe;
+ set_current_kprobe(p, regs, kcb);
+ kcb->kprobe_status = KPROBE_HIT_ACTIVE;
+ if (p->pre_handler && p->pre_handler(p, regs))
+ /* handler set things up, skip ss setup */
+ return 1;
}
- goto no_kprobe;
- }
-
- p = get_kprobe(addr);
- if (!p) {
+ } else {
if (*addr != BREAKPOINT_INSTRUCTION) {
/*
* The breakpoint instruction was removed right
@@ -518,18 +508,16 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
*/
regs->ip = (unsigned long)addr;
ret = 1;
+ goto no_kprobe;
+ }
+ if (kprobe_running()) {
+ p = __get_cpu_var(current_kprobe);
+ if (p->break_handler && p->break_handler(p, regs))
+ goto ss_probe;
+ goto no_kprobe;
}
- /* Not one of ours: let kernel handle it */
- goto no_kprobe;
}

- 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 */
- return 1;
-
ss_probe:
#if !defined(CONFIG_PREEMPT) || defined(CONFIG_PM)
if (p->ainsn.boostable == 1 && !p->post_handler) {
--
1.5.4.rc2.1097.gb6e0d




\
 
 \ /
  Last update: 2007-12-28 02:47    [W:0.065 / U:0.472 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site