Re: [PATCH] parisc: ftrace: get_kprobe() must be called with preempt disabled

[Masami Hiramatsu <mhiramat@kernel.org>]

On Tue, 3 Nov 2020 08:42:44 +0100
Helge Deller <deller@gmx.de> wrote:

> As noticed by Masami Hiramatsu, get_kprobe() must be called with preempt
> disabled.

Doesn't parisc ftrace implementation preempt off? Then it is required.

Steve, can we expect that op->func() is called under preempt off always
on any arch or is it arch dependent?

Thank you,

> 
> Noticed-by: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Helge Deller <deller@gmx.de>
> 
> diff --git a/arch/parisc/kernel/ftrace.c b/arch/parisc/kernel/ftrace.c
> index 63e3ecb9da81..dd356ad18aca 100644
> --- a/arch/parisc/kernel/ftrace.c
> +++ b/arch/parisc/kernel/ftrace.c
> @@ -207,13 +212,22 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
>  			   struct ftrace_ops *ops, struct pt_regs *regs)
>  {
>  	struct kprobe_ctlblk *kcb;
> -	struct kprobe *p = get_kprobe((kprobe_opcode_t *)ip);
> +	struct kprobe *p;
> +
> +	/*
> +	 * We don't want to be preempted for the entire
> +	 * duration of kprobe processing
> +	 */
> +	preempt_disable();
> +	p = get_kprobe((kprobe_opcode_t *)ip);
> 
>  	if (unlikely(!p) || kprobe_disabled(p))
> +		preempt_enable_no_resched();
>  		return;
> 
>  	if (kprobe_running()) {
>  		kprobes_inc_nmissed_count(p);
> +		preempt_enable_no_resched();
>  		return;
>  	}
> 
> @@ -235,6 +249,8 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
>  		}
>  	}
>  	__this_cpu_write(current_kprobe, NULL);
> +
> +	preempt_enable_no_resched();
>  }
>  NOKPROBE_SYMBOL(kprobe_ftrace_handler);
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>