All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu <mhiramat@kernel.org>
To: Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@kernel.org>
Cc: x86@kernel.org, Masami Hiramatsu <mhiramat@kernel.org>,
	Ingo Molnar <mingo@redhat.com>, "H . Peter Anvin" <hpa@zytor.com>,
	linux-kernel@vger.kernel.org,
	Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Laura Abbott <labbott@redhat.com>, Josef Bacik <jbacik@fb.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>,
	Vineet Gupta <vgupta@synopsys.com>,
	Russell King <linux@armlinux.org.uk>,
	Arnd Bergmann <arnd@arndb.de>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Tony Luck <tony.luck@intel.com>,
	Fenghua Yu <fenghua.yu@intel.com>,
	Ralf Baechle <ralf@linux-mips.org>,
	James Hogan <jhogan@kernel.org>,
	Martin Schwidefsky <schwidefsky@de.ibm.com>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	Yoshinori Sato <ysato@users.sourceforge.jp>,
	Rich Felker <dalias@libc.org>,
	"David S . Miller" <davem@davemloft.net>,
	linux-arch@vger.kernel.org
Subject: [PATCH -tip v4 25/27] x86: kprobes: Do not disable preempt on int3 path
Date: Mon, 28 May 2018 16:12:09 +0900	[thread overview]
Message-ID: <152749152918.15132.3522241227611599185.stgit@devbox> (raw)
In-Reply-To: <152749074878.15132.16693721906742461289.stgit@devbox>

Since int3 and debug exception(for singlestep) are run with
IRQ disabled and while running single stepping we drop IF
from regs->flags, that path must not be preemptible. So we
can remove the preempt disable/enable calls from that path.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Suggested-by: Ingo Molnar <mingo@kernel.org>
---
 Changes in v3:
  - Split user-side changes to another patch
 Changes in v2:
  - Include user-side changes.
---
 Documentation/kprobes.txt      |   11 +++++------
 arch/x86/kernel/kprobes/core.c |   18 ++++--------------
 arch/x86/kernel/kprobes/opt.c  |    1 -
 3 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/Documentation/kprobes.txt b/Documentation/kprobes.txt
index 907a3017c0f2..3e9e99ea751b 100644
--- a/Documentation/kprobes.txt
+++ b/Documentation/kprobes.txt
@@ -566,12 +566,11 @@ the same handler) may run concurrently on different CPUs.
 Kprobes does not use mutexes or allocate memory except during
 registration and unregistration.
 
-Probe handlers are run with preemption disabled.  Depending on the
-architecture and optimization state, handlers may also run with
-interrupts disabled (e.g., kretprobe handlers and optimized kprobe
-handlers run without interrupt disabled on x86/x86-64).  In any case,
-your handler should not yield the CPU (e.g., by attempting to acquire
-a semaphore).
+Probe handlers are run with preemption disabled or interrupt disabled,
+which depends on the architecture and optimization state.  (e.g.,
+kretprobe handlers and optimized kprobe handlers run without interrupt
+disabled on x86/x86-64).  In any case, your handler should not yield
+the CPU (e.g., by attempting to acquire a semaphore, or waiting I/O).
 
 Since a return probe is implemented by replacing the return
 address with the trampoline's address, stack backtraces and calls
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 814e26b7c8a2..f7104b256de7 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -594,7 +594,6 @@ static void setup_singlestep(struct kprobe *p, struct pt_regs *regs,
 		 * stepping.
 		 */
 		regs->ip = (unsigned long)p->ainsn.insn;
-		preempt_enable_no_resched();
 		return;
 	}
 #endif
@@ -667,12 +666,10 @@ int kprobe_int3_handler(struct pt_regs *regs)
 
 	addr = (kprobe_opcode_t *)(regs->ip - sizeof(kprobe_opcode_t));
 	/*
-	 * We don't want to be preempted for the entire
-	 * duration of kprobe processing. We conditionally
-	 * re-enable preemption at the end of this function,
-	 * and also in reenter_kprobe() and setup_singlestep().
+	 * We don't want to be preempted for the entire duration of kprobe
+	 * processing. Since int3 and debug trap disables irqs and we clear
+	 * IF while singlestepping, it must be no preemptible.
 	 */
-	preempt_disable();
 
 	kcb = get_kprobe_ctlblk();
 	p = get_kprobe(addr);
@@ -694,10 +691,8 @@ int kprobe_int3_handler(struct pt_regs *regs)
 			 */
 			if (!p->pre_handler || !p->pre_handler(p, regs))
 				setup_singlestep(p, regs, kcb, 0);
-			else {
+			else
 				reset_current_kprobe();
-				preempt_enable_no_resched();
-			}
 			return 1;
 		}
 	} else if (*addr != BREAKPOINT_INSTRUCTION) {
@@ -711,11 +706,9 @@ int kprobe_int3_handler(struct pt_regs *regs)
 		 * the original instruction.
 		 */
 		regs->ip = (unsigned long)addr;
-		preempt_enable_no_resched();
 		return 1;
 	} /* else: not a kprobe fault; let the kernel handle it */
 
-	preempt_enable_no_resched();
 	return 0;
 }
 NOKPROBE_SYMBOL(kprobe_int3_handler);
@@ -966,8 +959,6 @@ int kprobe_debug_handler(struct pt_regs *regs)
 	}
 	reset_current_kprobe();
 out:
-	preempt_enable_no_resched();
-
 	/*
 	 * if somebody else is singlestepping across a probe point, flags
 	 * will have TF set, in which case, continue the remaining processing
@@ -1014,7 +1005,6 @@ int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
 			restore_previous_kprobe(kcb);
 		else
 			reset_current_kprobe();
-		preempt_enable_no_resched();
 	} else if (kcb->kprobe_status == KPROBE_HIT_ACTIVE ||
 		   kcb->kprobe_status == KPROBE_HIT_SSDONE) {
 		/*
diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 203d398802a3..eaf02f2e7300 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -491,7 +491,6 @@ int setup_detour_execution(struct kprobe *p, struct pt_regs *regs, int reenter)
 		regs->ip = (unsigned long)op->optinsn.insn + TMPL_END_IDX;
 		if (!reenter)
 			reset_current_kprobe();
-		preempt_enable_no_resched();
 		return 1;
 	}
 	return 0;

WARNING: multiple messages have this Message-ID (diff)
From: Masami Hiramatsu <mhiramat@kernel.org>
To: Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@kernel.org>
Cc: x86@kernel.org, Masami Hiramatsu <mhiramat@kernel.org>,
	Ingo Molnar <mingo@redhat.com>, "H . Peter Anvin" <hpa@zytor.com>,
	linux-kernel@vger.kernel.org,
	Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Laura Abbott <labbott@redhat.com>, Josef Bacik <jbacik@fb.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>,
	Vineet Gupta <vgupta@synopsys.com>,
	Russell King <linux@armlinux.org.uk>,
	Arnd Bergmann <arnd@arndb.de>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Tony Luck <tony.luck@intel.com>,
	Fenghua Yu <fenghua.yu@intel.com>,
	Ralf Baechle <ralf@linux-mips.org>,
	James Hogan <jhogan@kernel.org>
Subject: [PATCH -tip v4 25/27] x86: kprobes: Do not disable preempt on int3 path
Date: Mon, 28 May 2018 16:12:09 +0900	[thread overview]
Message-ID: <152749152918.15132.3522241227611599185.stgit@devbox> (raw)
In-Reply-To: <152749074878.15132.16693721906742461289.stgit@devbox>

Since int3 and debug exception(for singlestep) are run with
IRQ disabled and while running single stepping we drop IF
from regs->flags, that path must not be preemptible. So we
can remove the preempt disable/enable calls from that path.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Suggested-by: Ingo Molnar <mingo@kernel.org>
---
 Changes in v3:
  - Split user-side changes to another patch
 Changes in v2:
  - Include user-side changes.
---
 Documentation/kprobes.txt      |   11 +++++------
 arch/x86/kernel/kprobes/core.c |   18 ++++--------------
 arch/x86/kernel/kprobes/opt.c  |    1 -
 3 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/Documentation/kprobes.txt b/Documentation/kprobes.txt
index 907a3017c0f2..3e9e99ea751b 100644
--- a/Documentation/kprobes.txt
+++ b/Documentation/kprobes.txt
@@ -566,12 +566,11 @@ the same handler) may run concurrently on different CPUs.
 Kprobes does not use mutexes or allocate memory except during
 registration and unregistration.
 
-Probe handlers are run with preemption disabled.  Depending on the
-architecture and optimization state, handlers may also run with
-interrupts disabled (e.g., kretprobe handlers and optimized kprobe
-handlers run without interrupt disabled on x86/x86-64).  In any case,
-your handler should not yield the CPU (e.g., by attempting to acquire
-a semaphore).
+Probe handlers are run with preemption disabled or interrupt disabled,
+which depends on the architecture and optimization state.  (e.g.,
+kretprobe handlers and optimized kprobe handlers run without interrupt
+disabled on x86/x86-64).  In any case, your handler should not yield
+the CPU (e.g., by attempting to acquire a semaphore, or waiting I/O).
 
 Since a return probe is implemented by replacing the return
 address with the trampoline's address, stack backtraces and calls
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 814e26b7c8a2..f7104b256de7 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -594,7 +594,6 @@ static void setup_singlestep(struct kprobe *p, struct pt_regs *regs,
 		 * stepping.
 		 */
 		regs->ip = (unsigned long)p->ainsn.insn;
-		preempt_enable_no_resched();
 		return;
 	}
 #endif
@@ -667,12 +666,10 @@ int kprobe_int3_handler(struct pt_regs *regs)
 
 	addr = (kprobe_opcode_t *)(regs->ip - sizeof(kprobe_opcode_t));
 	/*
-	 * We don't want to be preempted for the entire
-	 * duration of kprobe processing. We conditionally
-	 * re-enable preemption at the end of this function,
-	 * and also in reenter_kprobe() and setup_singlestep().
+	 * We don't want to be preempted for the entire duration of kprobe
+	 * processing. Since int3 and debug trap disables irqs and we clear
+	 * IF while singlestepping, it must be no preemptible.
 	 */
-	preempt_disable();
 
 	kcb = get_kprobe_ctlblk();
 	p = get_kprobe(addr);
@@ -694,10 +691,8 @@ int kprobe_int3_handler(struct pt_regs *regs)
 			 */
 			if (!p->pre_handler || !p->pre_handler(p, regs))
 				setup_singlestep(p, regs, kcb, 0);
-			else {
+			else
 				reset_current_kprobe();
-				preempt_enable_no_resched();
-			}
 			return 1;
 		}
 	} else if (*addr != BREAKPOINT_INSTRUCTION) {
@@ -711,11 +706,9 @@ int kprobe_int3_handler(struct pt_regs *regs)
 		 * the original instruction.
 		 */
 		regs->ip = (unsigned long)addr;
-		preempt_enable_no_resched();
 		return 1;
 	} /* else: not a kprobe fault; let the kernel handle it */
 
-	preempt_enable_no_resched();
 	return 0;
 }
 NOKPROBE_SYMBOL(kprobe_int3_handler);
@@ -966,8 +959,6 @@ int kprobe_debug_handler(struct pt_regs *regs)
 	}
 	reset_current_kprobe();
 out:
-	preempt_enable_no_resched();
-
 	/*
 	 * if somebody else is singlestepping across a probe point, flags
 	 * will have TF set, in which case, continue the remaining processing
@@ -1014,7 +1005,6 @@ int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
 			restore_previous_kprobe(kcb);
 		else
 			reset_current_kprobe();
-		preempt_enable_no_resched();
 	} else if (kcb->kprobe_status == KPROBE_HIT_ACTIVE ||
 		   kcb->kprobe_status == KPROBE_HIT_SSDONE) {
 		/*
diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 203d398802a3..eaf02f2e7300 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -491,7 +491,6 @@ int setup_detour_execution(struct kprobe *p, struct pt_regs *regs, int reenter)
 		regs->ip = (unsigned long)op->optinsn.insn + TMPL_END_IDX;
 		if (!reenter)
 			reset_current_kprobe();
-		preempt_enable_no_resched();
 		return 1;
 	}
 	return 0;

  parent reply	other threads:[~2018-05-28  7:12 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-28  6:59 [PATCH -tip v4 00/27] kprobes: Cleanup jprobe implementation Masami Hiramatsu
2018-05-28  6:59 ` Masami Hiramatsu
2018-05-28  6:59 ` [PATCH -tip v4 01/27] Documentation/kprobes: Fix to remove remaining jprobe Masami Hiramatsu
2018-05-28  6:59   ` Masami Hiramatsu
2018-05-28  7:00 ` [PATCH -tip v4 02/27] kprobes: Remove jprobe API implementation Masami Hiramatsu
2018-05-28  7:00   ` Masami Hiramatsu
2018-05-28  7:00 ` [PATCH -tip v4 03/27] kprobes/x86: Remove jprobe implementation Masami Hiramatsu
2018-05-28  7:00   ` Masami Hiramatsu
2018-05-28  7:01 ` [PATCH -tip v4 04/27] ARC: kprobes: " Masami Hiramatsu
2018-05-28  7:01   ` Masami Hiramatsu
2018-05-28  7:01 ` [PATCH -tip v4 05/27] ARM: kprobes: Remove jprobe arm implementation Masami Hiramatsu
2018-05-28  7:01   ` Masami Hiramatsu
2018-05-28  7:02 ` [PATCH -tip v4 06/27] arm64: kprobes: Remove jprobe implementation Masami Hiramatsu
2018-05-28  7:02   ` Masami Hiramatsu
2018-05-28  7:02 ` [PATCH -tip v4 07/27] powerpc/kprobes: Remove jprobe powerpc implementation Masami Hiramatsu
2018-05-28  7:02   ` Masami Hiramatsu
2018-05-28  7:03 ` [PATCH -tip v4 08/27] ia64: kprobes: Remove jprobe implementation Masami Hiramatsu
2018-05-28  7:03   ` Masami Hiramatsu
2018-05-28  7:03 ` [PATCH -tip v4 09/27] MIPS: " Masami Hiramatsu
2018-05-28  7:03   ` Masami Hiramatsu
2018-05-28  7:04 ` [PATCH -tip v4 10/27] s390/kprobes: " Masami Hiramatsu
2018-05-28  7:04   ` Masami Hiramatsu
2018-05-28  7:04 ` [PATCH -tip v4 11/27] sh: kprobes: " Masami Hiramatsu
2018-05-28  7:04   ` Masami Hiramatsu
2018-05-28  7:05 ` [PATCH -tip v4 12/27] sparc64: " Masami Hiramatsu
2018-05-28  7:05   ` Masami Hiramatsu
2018-05-28  7:05 ` [PATCH -tip v4 13/27] kprobes: Don't check the ->break_handler() in generic kprobes code Masami Hiramatsu
2018-05-28  7:05   ` Masami Hiramatsu
2018-05-28  7:06 ` [PATCH -tip v4 14/27] kprobes/x86: Don't call ->break_handler() in x86 kprobes Masami Hiramatsu
2018-05-28  7:06   ` Masami Hiramatsu
2018-05-28  7:06 ` [PATCH -tip v4 15/27] ARC: kprobes: Don't call the ->break_handler() in ARC kprobes code Masami Hiramatsu
2018-05-28  7:06   ` Masami Hiramatsu
2018-05-28  7:07 ` [PATCH -tip v4 16/27] ARM: kprobes: Don't call the ->break_handler() in arm " Masami Hiramatsu
2018-05-28  7:07   ` Masami Hiramatsu
2018-05-28  7:08 ` [PATCH -tip v4 17/27] arm64: " Masami Hiramatsu
2018-05-28  7:08   ` Masami Hiramatsu
2018-05-28  7:08 ` [PATCH -tip v4 18/27] powerpc/kprobes: " Masami Hiramatsu
2018-05-28  7:08   ` Masami Hiramatsu
2018-05-28  7:09 ` [PATCH -tip v4 19/27] ia64: kprobes: Don't call the ->break_handler() in ia64 " Masami Hiramatsu
2018-05-28  7:09   ` Masami Hiramatsu
2018-05-28  7:09 ` [PATCH -tip v4 20/27] MIPS: kprobes: Don't call the ->break_handler() in MIPS " Masami Hiramatsu
2018-05-28  7:09   ` Masami Hiramatsu
2018-05-28  7:10 ` [PATCH -tip v4 21/27] s390/kprobes: Don't call the ->break_handler() in s390 " Masami Hiramatsu
2018-05-28  7:10   ` Masami Hiramatsu
2018-05-28  7:10 ` [PATCH -tip v4 22/27] sh: kprobes: Don't call the ->break_handler() in SH " Masami Hiramatsu
2018-05-28  7:10   ` Masami Hiramatsu
2018-05-28  7:11 ` [PATCH -tip v4 23/27] sparc64: kprobes: Don't call the ->break_handler() in sparc64 " Masami Hiramatsu
2018-05-28  7:11   ` Masami Hiramatsu
2018-05-28  7:11 ` [PATCH -tip v4 24/27] bpf: error-inject: kprobes: Clear current_kprobe and enable preempt in kprobe Masami Hiramatsu
2018-05-28  7:11   ` Masami Hiramatsu
2018-05-31 10:55   ` Naveen N. Rao
2018-06-01 23:36     ` Masami Hiramatsu
2018-06-02 11:58       ` Naveen N. Rao
2018-06-04  9:08         ` Masami Hiramatsu
2018-05-28  7:12 ` Masami Hiramatsu [this message]
2018-05-28  7:12   ` [PATCH -tip v4 25/27] x86: kprobes: Do not disable preempt on int3 path Masami Hiramatsu
2018-05-28  7:12 ` [PATCH -tip v4 26/27] Documentation: kprobes: Add how to change the execution path Masami Hiramatsu
2018-05-28  7:12   ` Masami Hiramatsu
2018-05-28  7:13 ` [PATCH -tip v4 27/27] kprobes: Remove jprobe stub API Masami Hiramatsu
2018-05-28  7:13   ` Masami Hiramatsu
2018-05-30  9:01 ` [PATCH -tip v4 00/27] kprobes: Cleanup jprobe implementation Masami Hiramatsu
2018-05-30  9:01   ` Masami Hiramatsu
2018-05-31 10:43   ` Naveen N. Rao
2018-05-31 10:43     ` Naveen N. Rao

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=152749152918.15132.3522241227611599185.stgit@devbox \
    --to=mhiramat@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=ananth@linux.vnet.ibm.com \
    --cc=arnd@arndb.de \
    --cc=ast@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=dalias@libc.org \
    --cc=davem@davemloft.net \
    --cc=fenghua.yu@intel.com \
    --cc=heiko.carstens@de.ibm.com \
    --cc=hpa@zytor.com \
    --cc=jbacik@fb.com \
    --cc=jhogan@kernel.org \
    --cc=labbott@redhat.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.com \
    --cc=ralf@linux-mips.org \
    --cc=ravi.bangoria@linux.vnet.ibm.com \
    --cc=rostedt@goodmis.org \
    --cc=schwidefsky@de.ibm.com \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=vgupta@synopsys.com \
    --cc=will.deacon@arm.com \
    --cc=x86@kernel.org \
    --cc=ysato@users.sourceforge.jp \
    /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.