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

[Maran]

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/