Re: [PATCH 1/2] powerpc/kprobes: Remove redundant code

From: Naveen N. Rao
Date: Tue Feb 18 2020 - 09:40:00 EST


Christophe Leroy wrote:
At the time being we have something like

if (something) {
p = get();
if (p) {
if (something_wrong)
goto out;
...
return;
} else if (a != b) {
if (some_error)
goto out;
...
}
goto out;
}
p = get();
if (!p) {
if (a != b) {
if (some_error)
goto out;
...
}
goto out;
}

This is similar to

p = get();
if (something) {
if (p) {
if (something_wrong)
goto out;
...
return;
}
}
if (!p) {
if (a != b) {
if (some_error)
goto out;
...
}
goto out;
}

Signed-off-by: Christophe Leroy <christophe.leroy@xxxxxx>
---
arch/powerpc/kernel/kprobes.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)

Good cleanup, thanks.


diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index f8b848aa65bd..7a925eb76ec0 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -276,8 +276,8 @@ int kprobe_handler(struct pt_regs *regs)
kcb = get_kprobe_ctlblk();
/* Check we're not actually recursing */
+ p = get_kprobe(addr);
if (kprobe_running()) {
- p = get_kprobe(addr);
if (p) {
kprobe_opcode_t insn = *p->ainsn.insn;
if (kcb->kprobe_status == KPROBE_HIT_SS &&
@@ -308,22 +308,9 @@ int kprobe_handler(struct pt_regs *regs)
}
prepare_singlestep(p, regs);
return 1;
- } else if (*addr != BREAKPOINT_INSTRUCTION) {
- /* If trap variant, then it belongs not to us */
- kprobe_opcode_t cur_insn = *addr;
-
- if (is_trap(cur_insn))
- goto no_kprobe;
- /* The breakpoint instruction was removed by
- * another cpu right after we hit, no further
- * handling of this interrupt is appropriate
- */
- ret = 1;
}
- goto no_kprobe;

A minot nit -- removing the above goto makes a slight change to the logic. But, see my comments for the next patch.

- Naveen

}
- p = get_kprobe(addr);
if (!p) {
if (*addr != BREAKPOINT_INSTRUCTION) {
/*
--
2.25.0