All of lore.kernel.org
 help / color / mirror / Atom feed
From: Harvey Harrison <harvey.harrison@gmail.com>
To: Masami Hiramatsu <mhiramat@redhat.com>
Cc: LKML <linux-kernel@vger.kernel.org>, Ingo Molnar <mingo@elte.hu>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: [PATCH] x86: kprobes change kprobe_handler flow
Date: Thu, 27 Dec 2007 18:08:22 -0800	[thread overview]
Message-ID: <1198807702.6323.37.camel@brick> (raw)

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.

Create two jump targets:
	preempt_out: re-enables preemption before returning ret
	out: only returns ret

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
Masami, noticed a small bug in the previous version in the !p
case when the breakpoint was the kernel's.  Please review this
version.

 arch/x86/kernel/kprobes.c |   60 +++++++++++++++++++++------------------------
 1 files changed, 28 insertions(+), 32 deletions(-)

diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
index 4e33329..f8c7ac1 100644
--- a/arch/x86/kernel/kprobes.c
+++ b/arch/x86/kernel/kprobes.c
@@ -480,32 +480,28 @@ 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;
+			{
+				ret = 1;
+				goto out;
+			}
+			goto preempt_out;
 		} 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;
+			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 */
 				ret = 1;
-				goto no_kprobe;
+				goto out;
 			}
-			p = __get_cpu_var(current_kprobe);
-			if (p->break_handler && p->break_handler(p, regs))
-				goto ss_probe;
 		}
-		goto no_kprobe;
-	}
-
-	p = get_kprobe(addr);
-	if (!p) {
+	} else {
 		if (*addr != BREAKPOINT_INSTRUCTION) {
 			/*
 			 * The breakpoint instruction was removed right
@@ -518,34 +514,34 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
 			 */
 			regs->ip = (unsigned long)addr;
 			ret = 1;
+			goto preempt_out;
+		}
+		if (kprobe_running()) {
+			p = __get_cpu_var(current_kprobe);
+			if (p->break_handler && p->break_handler(p, regs))
+				goto ss_probe;
 		}
 		/* Not one of ours: let kernel handle it */
-		goto no_kprobe;
+		goto preempt_out;
 	}
 
-	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:
+	ret = 1;
 #if !defined(CONFIG_PREEMPT) || defined(CONFIG_PM)
 	if (p->ainsn.boostable == 1 && !p->post_handler) {
 		/* Boost up -- we can execute copied instructions directly */
 		reset_current_kprobe();
 		regs->ip = (unsigned long)p->ainsn.insn;
-		preempt_enable_no_resched();
-		return 1;
+		goto preempt_out;
 	}
 #endif
 	prepare_singlestep(p, regs);
 	kcb->kprobe_status = KPROBE_HIT_SS;
-	return 1;
+	goto out;
 
-no_kprobe:
+preempt_out:
 	preempt_enable_no_resched();
+out:
 	return ret;
 }
 
-- 
1.5.4.rc2.1097.gb6e0d




             reply	other threads:[~2007-12-28  2:08 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-12-28  2:08 Harvey Harrison [this message]
2007-12-30  8:07 ` [PATCH] x86: kprobes change kprobe_handler flow Masami Hiramatsu
2007-12-30 13:47   ` Ingo Molnar
  -- strict thread matches above, loose matches on Subject: below --
2007-12-28  1:44 Harvey Harrison
2007-12-31 13:03 ` Abhishek Sagar
2008-01-01 15:35   ` Ingo Molnar
2008-01-01 19:40     ` Abhishek Sagar
2008-01-01 20:19       ` Harvey Harrison
2008-01-01 20:54         ` Abhishek Sagar
2008-01-02 18:09       ` Masami Hiramatsu
2008-01-02 19:31         ` Abhishek Sagar
2008-01-02 20:23           ` Ingo Molnar
2008-01-02 21:56           ` Masami Hiramatsu
2008-01-03 17:15             ` Masami Hiramatsu
2008-01-03 21:31               ` Masami Hiramatsu
2008-01-04  6:34                 ` Abhishek Sagar
2008-01-03 18:12             ` Abhishek Sagar
2008-01-03 20:11               ` Masami Hiramatsu
2008-01-04  6:43                 ` Abhishek Sagar
2008-01-01 17:49   ` Masami Hiramatsu
2008-01-01 20:24     ` Abhishek Sagar
2008-01-02 16:00       ` Masami Hiramatsu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1198807702.6323.37.camel@brick \
    --to=harvey.harrison@gmail.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@redhat.com \
    --cc=mingo@elte.hu \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.