Re: [PATCH] stop_cpu() needs to use raw_smp_processor_id()

From: Maran
Date: Mon Jun 22 2009 - 10:35:03 EST


Heiko Carstens wrote:
On Wed, Jun 17, 2009 at 12:31:56PM +0530, Maran wrote:
Heiko Carstens wrote:
On Tue, Jun 16, 2009 at 04:49:41PM +0530, Ananth N Mavinakayanahalli wrote:
With DEBUG_PREEMPT on, on an s390 box, we are seeing:

BUG: using smp_processor_id() in preemptible [00000000] code: insmod/3006 caller is arch_arm_kprobe+0x2c/0x90 CPU: 0 Not tainted 2.6.30-rc8-tipjun10-02053-g39ae214 #7 Process insmod (pid: 3006, task: 000000007df31410, ksp: 000000007e087a28) 0000000000000000 000000007e087c38 0000000000000002 0000000000000000
000000007e087cd8 000000007e087c50 000000007e087c50 0000000000048156 00000000003e4de8 000000000002b830 0000000000000000 0000000000000000 0000000000000000 000000007e087c38 ffffffff0000000d 000000007e087ca8 000000000040a780 000000000001635c 000000007e087c38 000000007e087c80 Call Trace: ([<00000000000162e6>] show_trace+0x13a/0x148) [<00000000001f123a>] debug_smp_processor_id+0x136/0x13c [<00000000003ddbd8>] arch_arm_kprobe+0x2c/0x90 [<00000000003df8cc>] register_kprobe+0x630/0x684 [<000003e000241056>] kprobe_init+0x56/0xa8 [kprobe_example] [<00000000000120c2>] do_one_initcall+0x3e/0x16c [<000000000007cb4c>] SyS_init_module+0xc4/0x1fc [<0000000000027f42>] sysc_noemu+0x10/0x16 [<0000004a4493324e>] 0x4a4493324e

With the current linus tree (git 10), kprobe breaks. Even smoke tests are failed in s390x box.
With 2.6.30, the same warning is raised even with CONFIG_SMP enabled. I have pasted the config file used.

How about the patch below? Works for me. modprobe kprobe_example and the
kprobes selftest work for me.

Subject: [PATCH] kprobes: defer setting of ctlblk state

From: Heiko Carstens <heiko.carstens@xxxxxxxxxx>

get_krobe_ctlblk returns a per cpu kprobe control block which holds
the state of the current cpu wrt to kprobe.
When inserting/removing a kprobe the state of the cpu which replaces
the code is changed to KPROBE_SWAP_INST. This however is done when
preemption is still enabled. So the state of the current cpu doesn't
necessarily reflect the real state.
To fix this move the code that changes the state to non-preemptible
context.

Reported-by: Ananth N Mavinakayanahalli <ananth@xxxxxxxxxx>
Signed-off-by: Heiko Carstens <heiko.carstens@xxxxxxxxxx>
---
arch/s390/kernel/kprobes.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)

Index: linux-2.6/arch/s390/kernel/kprobes.c
===================================================================
--- linux-2.6.orig/arch/s390/kernel/kprobes.c
+++ linux-2.6/arch/s390/kernel/kprobes.c
@@ -154,39 +154,35 @@ void __kprobes get_instruction_type(stru

static int __kprobes swap_instruction(void *aref)
{
+ struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+ unsigned long status = kcb->kprobe_status;
struct ins_replace_args *args = aref;
+ int rc;

- return probe_kernel_write(args->ptr, &args->new, sizeof(args->new));
+ kcb->kprobe_status = KPROBE_SWAP_INST;
+ rc = probe_kernel_write(args->ptr, &args->new, sizeof(args->new));
+ kcb->kprobe_status = status;
+ return rc;
}

void __kprobes arch_arm_kprobe(struct kprobe *p)
{
- struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
- unsigned long status = kcb->kprobe_status;
struct ins_replace_args args;

args.ptr = p->addr;
args.old = p->opcode;
args.new = BREAKPOINT_INSTRUCTION;
-
- kcb->kprobe_status = KPROBE_SWAP_INST;
stop_machine(swap_instruction, &args, NULL);
- kcb->kprobe_status = status;
}

void __kprobes arch_disarm_kprobe(struct kprobe *p)
{
- struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
- unsigned long status = kcb->kprobe_status;
struct ins_replace_args args;

args.ptr = p->addr;
args.old = BREAKPOINT_INSTRUCTION;
args.new = p->opcode;
-
- kcb->kprobe_status = KPROBE_SWAP_INST;
stop_machine(swap_instruction, &args, NULL);
- kcb->kprobe_status = status;
}

void __kprobes arch_remove_kprobe(struct kprobe *p)
Hi Heiko,
modprobe kprobe_example and kprobe sanity test passed on applying the patch. Thanks!

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/