From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
To: linux-kernel@vger.kernel.org
Cc: akpm@osdl.org, anil.s.keshavamurthy@intel.com,
prasanna@in.ibm.com, paulmck@us.ibm.com, davem@davemloft.net
Subject: [PATCH] Kprobes: preempt_disable/enable() reordering redux
Date: Mon, 24 Oct 2005 11:57:19 -0400 [thread overview]
Message-ID: <20051024155719.GA6024@in.ibm.com> (raw)
From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
This patch (over 2.6.14-rc4-mm1) reorganizes the preempt_disable/enable
calls to eliminate the extra preempt depth. Changes based on Paul
McKenney's review suggestions for the kprobes RCU changeset.
Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Signed-off-by: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
---
arch/i386/kernel/kprobes.c | 25 +++++++++++++++----------
arch/ia64/kernel/kprobes.c | 37 ++++++++++++++++++++-----------------
arch/ppc64/kernel/kprobes.c | 25 +++++++++++++++----------
arch/sparc64/kernel/kprobes.c | 21 +++++++++++++--------
arch/x86_64/kernel/kprobes.c | 29 +++++++++++++++--------------
include/linux/kprobes.h | 2 +-
kernel/kprobes.c | 2 +-
7 files changed, 80 insertions(+), 61 deletions(-)
Index: linux-2.6.14-rc4/arch/i386/kernel/kprobes.c
===================================================================
--- linux-2.6.14-rc4.orig/arch/i386/kernel/kprobes.c 2005-10-17 18:59:51.000000000 -0400
+++ linux-2.6.14-rc4/arch/i386/kernel/kprobes.c 2005-10-21 11:02:52.000000000 -0400
@@ -153,7 +153,14 @@ static int __kprobes kprobe_handler(stru
int ret = 0;
kprobe_opcode_t *addr = NULL;
unsigned long *lp;
- struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+ struct kprobe_ctlblk *kcb;
+
+ /*
+ * We don't want to be preempted for the entire
+ * duration of kprobe processing
+ */
+ preempt_disable();
+ kcb = get_kprobe_ctlblk();
/* 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.
@@ -221,11 +228,6 @@ static int __kprobes kprobe_handler(stru
goto no_kprobe;
}
- /*
- * This preempt_disable() matches the preempt_enable_no_resched()
- * in post_kprobe_handler()
- */
- preempt_disable();
set_current_kprobe(p, regs, kcb);
kcb->kprobe_status = KPROBE_HIT_ACTIVE;
@@ -239,6 +241,7 @@ ss_probe:
return 1;
no_kprobe:
+ preempt_enable_no_resched();
return ret;
}
@@ -310,8 +313,8 @@ int __kprobes trampoline_probe_handler(s
/*
* By returning a non-zero value, we are telling
- * kprobe_handler() that we have handled unlocking
- * and re-enabling preemption
+ * kprobe_handler() that we don't want the post_handler
+ * to run (and have re-enabled preemption)
*/
return 1;
}
@@ -455,7 +458,6 @@ int __kprobes kprobe_exceptions_notify(s
struct die_args *args = (struct die_args *)data;
int ret = NOTIFY_DONE;
- rcu_read_lock();
switch (val) {
case DIE_INT3:
if (kprobe_handler(args->regs))
@@ -467,14 +469,16 @@ int __kprobes kprobe_exceptions_notify(s
break;
case DIE_GPF:
case DIE_PAGE_FAULT:
+ /* kprobe_running() needs smp_processor_id() */
+ preempt_disable();
if (kprobe_running() &&
kprobe_fault_handler(args->regs, args->trapnr))
ret = NOTIFY_STOP;
+ preempt_enable();
break;
default:
break;
}
- rcu_read_unlock();
return ret;
}
@@ -537,6 +541,7 @@ int __kprobes longjmp_break_handler(stru
*regs = kcb->jprobe_saved_regs;
memcpy((kprobe_opcode_t *) stack_addr, kcb->jprobes_stack,
MIN_STACK_SIZE(stack_addr));
+ preempt_enable_no_resched();
return 1;
}
return 0;
Index: linux-2.6.14-rc4/include/linux/kprobes.h
===================================================================
--- linux-2.6.14-rc4.orig/include/linux/kprobes.h 2005-10-17 19:00:01.000000000 -0400
+++ linux-2.6.14-rc4/include/linux/kprobes.h 2005-10-19 10:56:35.000000000 -0400
@@ -159,7 +159,7 @@ extern void show_registers(struct pt_reg
extern kprobe_opcode_t *get_insn_slot(void);
extern void free_insn_slot(kprobe_opcode_t *slot);
-/* Get the kprobe at this addr (if any) - called under a rcu_read_lock() */
+/* Get the kprobe at this addr (if any) - called with preemption disabled */
struct kprobe *get_kprobe(void *addr);
struct hlist_head * kretprobe_inst_table_head(struct task_struct *tsk);
Index: linux-2.6.14-rc4/kernel/kprobes.c
===================================================================
--- linux-2.6.14-rc4.orig/kernel/kprobes.c 2005-10-17 19:00:02.000000000 -0400
+++ linux-2.6.14-rc4/kernel/kprobes.c 2005-10-18 14:19:01.000000000 -0400
@@ -167,7 +167,7 @@ static inline void reset_kprobe_instance
* This routine is called either:
* - under the kprobe_lock spinlock - during kprobe_[un]register()
* OR
- * - under an rcu_read_lock() - from arch/xxx/kernel/kprobes.c
+ * - with preemption disabled - from arch/xxx/kernel/kprobes.c
*/
struct kprobe __kprobes *get_kprobe(void *addr)
{
Index: linux-2.6.14-rc4/arch/ia64/kernel/kprobes.c
===================================================================
--- linux-2.6.14-rc4.orig/arch/ia64/kernel/kprobes.c 2005-10-17 18:59:52.000000000 -0400
+++ linux-2.6.14-rc4/arch/ia64/kernel/kprobes.c 2005-10-21 13:50:10.000000000 -0400
@@ -389,11 +389,11 @@ int __kprobes trampoline_probe_handler(s
spin_unlock_irqrestore(&kretprobe_lock, flags);
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 don't want the post_handler
+ * to run (and have re-enabled preemption)
+ */
return 1;
}
@@ -604,7 +604,14 @@ static int __kprobes pre_kprobes_handler
int ret = 0;
struct pt_regs *regs = args->regs;
kprobe_opcode_t *addr = (kprobe_opcode_t *)instruction_pointer(regs);
- struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+ struct kprobe_ctlblk *kcb;
+
+ /*
+ * We don't want to be preempted for the entire
+ * duration of kprobe processing
+ */
+ preempt_disable();
+ kcb = get_kprobe_ctlblk();
/* Handle recursion cases */
if (kprobe_running()) {
@@ -659,11 +666,6 @@ static int __kprobes pre_kprobes_handler
goto no_kprobe;
}
- /*
- * This preempt_disable() matches the preempt_enable_no_resched()
- * in post_kprobes_handler()
- */
- preempt_disable();
set_current_kprobe(p, kcb);
kcb->kprobe_status = KPROBE_HIT_ACTIVE;
@@ -681,6 +683,7 @@ ss_probe:
return 1;
no_kprobe:
+ preempt_enable_no_resched();
return ret;
}
@@ -716,9 +719,6 @@ static int __kprobes kprobes_fault_handl
struct kprobe *cur = kprobe_running();
struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
- if (!cur)
- return 0;
-
if (cur->fault_handler && cur->fault_handler(cur, regs, trapnr))
return 1;
@@ -737,7 +737,6 @@ int __kprobes kprobe_exceptions_notify(s
struct die_args *args = (struct die_args *)data;
int ret = NOTIFY_DONE;
- rcu_read_lock();
switch(val) {
case DIE_BREAK:
if (pre_kprobes_handler(args))
@@ -748,12 +747,15 @@ int __kprobes kprobe_exceptions_notify(s
ret = NOTIFY_STOP;
break;
case DIE_PAGE_FAULT:
- if (kprobes_fault_handler(args->regs, args->trapnr))
+ /* kprobe_running() needs smp_processor_id() */
+ preempt_disable();
+ if (kprobe_running() &&
+ kprobes_fault_handler(args->regs, args->trapnr))
ret = NOTIFY_STOP;
+ preempt_enable();
default:
break;
}
- rcu_read_unlock();
return ret;
}
@@ -785,6 +787,7 @@ int __kprobes longjmp_break_handler(stru
struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
*regs = kcb->jprobe_saved_regs;
+ preempt_enable_no_resched();
return 1;
}
Index: linux-2.6.14-rc4/arch/ppc64/kernel/kprobes.c
===================================================================
--- linux-2.6.14-rc4.orig/arch/ppc64/kernel/kprobes.c 2005-10-17 18:59:52.000000000 -0400
+++ linux-2.6.14-rc4/arch/ppc64/kernel/kprobes.c 2005-10-19 11:00:02.000000000 -0400
@@ -148,7 +148,14 @@ static inline int kprobe_handler(struct
struct kprobe *p;
int ret = 0;
unsigned int *addr = (unsigned int *)regs->nip;
- struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+ struct kprobe_ctlblk *kcb;
+
+ /*
+ * We don't want to be preempted for the entire
+ * duration of kprobe processing
+ */
+ preempt_disable();
+ kcb = get_kprobe_ctlblk();
/* Check we're not actually recursing */
if (kprobe_running()) {
@@ -207,11 +214,6 @@ static inline int kprobe_handler(struct
goto no_kprobe;
}
- /*
- * This preempt_disable() matches the preempt_enable_no_resched()
- * in post_kprobe_handler().
- */
- preempt_disable();
kcb->kprobe_status = KPROBE_HIT_ACTIVE;
set_current_kprobe(p, regs, kcb);
if (p->pre_handler && p->pre_handler(p, regs))
@@ -224,6 +226,7 @@ ss_probe:
return 1;
no_kprobe:
+ preempt_enable_no_resched();
return ret;
}
@@ -296,8 +299,8 @@ int __kprobes trampoline_probe_handler(s
/*
* By returning a non-zero value, we are telling
- * kprobe_handler() that we have handled unlocking
- * and re-enabling preemption.
+ * kprobe_handler() that we don't want the post_handler
+ * to run (and have re-enabled preemption)
*/
return 1;
}
@@ -385,7 +388,6 @@ int __kprobes kprobe_exceptions_notify(s
struct die_args *args = (struct die_args *)data;
int ret = NOTIFY_DONE;
- rcu_read_lock();
switch (val) {
case DIE_BPT:
if (kprobe_handler(args->regs))
@@ -397,14 +399,16 @@ int __kprobes kprobe_exceptions_notify(s
break;
case DIE_GPF:
case DIE_PAGE_FAULT:
+ /* kprobe_running() needs smp_processor_id() */
+ preempt_disable();
if (kprobe_running() &&
kprobe_fault_handler(args->regs, args->trapnr))
ret = NOTIFY_STOP;
+ preempt_enable();
break;
default:
break;
}
- rcu_read_unlock();
return ret;
}
@@ -441,6 +445,7 @@ int __kprobes longjmp_break_handler(stru
* saved regs...
*/
memcpy(regs, &kcb->jprobe_saved_regs, sizeof(struct pt_regs));
+ preempt_enable_no_resched();
return 1;
}
Index: linux-2.6.14-rc4/arch/sparc64/kernel/kprobes.c
===================================================================
--- linux-2.6.14-rc4.orig/arch/sparc64/kernel/kprobes.c 2005-10-17 18:59:53.000000000 -0400
+++ linux-2.6.14-rc4/arch/sparc64/kernel/kprobes.c 2005-10-19 11:00:21.000000000 -0400
@@ -113,7 +113,14 @@ static int __kprobes kprobe_handler(stru
struct kprobe *p;
void *addr = (void *) regs->tpc;
int ret = 0;
- struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+ struct kprobe_ctlblk *kcb;
+
+ /*
+ * We don't want to be preempted for the entire
+ * duration of kprobe processing
+ */
+ preempt_disable();
+ kcb = get_kprobe_ctlblk();
if (kprobe_running()) {
p = get_kprobe(addr);
@@ -159,11 +166,6 @@ static int __kprobes kprobe_handler(stru
goto no_kprobe;
}
- /*
- * This preempt_disable() matches the preempt_enable_no_resched()
- * in post_kprobes_handler()
- */
- preempt_disable();
set_current_kprobe(p, regs, kcb);
kcb->kprobe_status = KPROBE_HIT_ACTIVE;
if (p->pre_handler && p->pre_handler(p, regs))
@@ -175,6 +177,7 @@ ss_probe:
return 1;
no_kprobe:
+ preempt_enable_no_resched();
return ret;
}
@@ -321,7 +324,6 @@ int __kprobes kprobe_exceptions_notify(s
struct die_args *args = (struct die_args *)data;
int ret = NOTIFY_DONE;
- rcu_read_lock();
switch (val) {
case DIE_DEBUG:
if (kprobe_handler(args->regs))
@@ -333,14 +335,16 @@ int __kprobes kprobe_exceptions_notify(s
break;
case DIE_GPF:
case DIE_PAGE_FAULT:
+ /* kprobe_running() needs smp_processor_id() */
+ preempt_disable();
if (kprobe_running() &&
kprobe_fault_handler(args->regs, args->trapnr))
ret = NOTIFY_STOP;
+ preempt_enable();
break;
default:
break;
}
- rcu_read_unlock();
return ret;
}
@@ -426,6 +430,7 @@ int __kprobes longjmp_break_handler(stru
&(kcb->jprobe_saved_stack),
sizeof(kcb->jprobe_saved_stack));
+ preempt_enable_no_resched();
return 1;
}
return 0;
Index: linux-2.6.14-rc4/arch/x86_64/kernel/kprobes.c
===================================================================
--- linux-2.6.14-rc4.orig/arch/x86_64/kernel/kprobes.c 2005-10-17 18:59:53.000000000 -0400
+++ linux-2.6.14-rc4/arch/x86_64/kernel/kprobes.c 2005-10-19 11:01:31.000000000 -0400
@@ -286,16 +286,19 @@ void __kprobes arch_prepare_kretprobe(st
}
}
-/*
- * Interrupts are disabled on entry as trap3 is an interrupt gate and they
- * remain disabled thorough out this function.
- */
int __kprobes kprobe_handler(struct pt_regs *regs)
{
struct kprobe *p;
int ret = 0;
kprobe_opcode_t *addr = (kprobe_opcode_t *)(regs->rip - sizeof(kprobe_opcode_t));
- struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+ struct kprobe_ctlblk *kcb;
+
+ /*
+ * We don't want to be preempted for the entire
+ * duration of kprobe processing
+ */
+ preempt_disable();
+ kcb = get_kprobe_ctlblk();
/* Check we're not actually recursing */
if (kprobe_running()) {
@@ -359,11 +362,6 @@ int __kprobes kprobe_handler(struct pt_r
goto no_kprobe;
}
- /*
- * This preempt_disable() matches the preempt_enable_no_resched()
- * in post_kprobe_handler()
- */
- preempt_disable();
set_current_kprobe(p, regs, kcb);
kcb->kprobe_status = KPROBE_HIT_ACTIVE;
@@ -377,6 +375,7 @@ ss_probe:
return 1;
no_kprobe:
+ preempt_enable_no_resched();
return ret;
}
@@ -448,8 +447,8 @@ int __kprobes trampoline_probe_handler(s
/*
* By returning a non-zero value, we are telling
- * kprobe_handler() that we have handled unlocking
- * and re-enabling preemption
+ * kprobe_handler() that we don't want the post_handler
+ * to run (and have re-enabled preemption)
*/
return 1;
}
@@ -594,7 +593,6 @@ int __kprobes kprobe_exceptions_notify(s
struct die_args *args = (struct die_args *)data;
int ret = NOTIFY_DONE;
- rcu_read_lock();
switch (val) {
case DIE_INT3:
if (kprobe_handler(args->regs))
@@ -606,14 +604,16 @@ int __kprobes kprobe_exceptions_notify(s
break;
case DIE_GPF:
case DIE_PAGE_FAULT:
+ /* kprobe_running() needs smp_processor_id() */
+ preempt_disable();
if (kprobe_running() &&
kprobe_fault_handler(args->regs, args->trapnr))
ret = NOTIFY_STOP;
+ preempt_enable();
break;
default:
break;
}
- rcu_read_unlock();
return ret;
}
@@ -675,6 +675,7 @@ int __kprobes longjmp_break_handler(stru
*regs = kcb->jprobe_saved_regs;
memcpy((kprobe_opcode_t *) stack_addr, kcb->jprobes_stack,
MIN_STACK_SIZE(stack_addr));
+ preempt_enable_no_resched();
return 1;
}
return 0;
next reply other threads:[~2005-10-24 15:58 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2005-10-24 15:57 Ananth N Mavinakayanahalli [this message]
2005-10-25 1:22 ` [PATCH] Kprobes: preempt_disable/enable() reordering redux Paul E. McKenney
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=20051024155719.GA6024@in.ibm.com \
--to=ananth@in.ibm.com \
--cc=akpm@osdl.org \
--cc=anil.s.keshavamurthy@intel.com \
--cc=davem@davemloft.net \
--cc=linux-kernel@vger.kernel.org \
--cc=paulmck@us.ibm.com \
--cc=prasanna@in.ibm.com \
/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.