Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <09c35d21-67dc-bbe5-532a-a41797bfe0fc@loongson.cn>
Date:   Wed, 7 Sep 2022 18:40:34 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.10.0
Subject: Re: [PATCH v3 10/10] LoongArch/ftrace: Fix unwind state when option
 func_stack_trace
Content-Language: en-US
To:     Qing Zhang <zhangqing@loongson.cn>,
        Huacai Chen <chenhuacai@kernel.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Ingo Molnar <mingo@redhat.com>
Cc:     WANG Xuerui <kernel@xen0n.name>, loongarch@lists.linux.dev,
        linux-kernel@vger.kernel.org,
        Jiaxun Yang <jiaxun.yang@flygoat.com>,
        Jeff Xie <xiehuan09@gmail.com>
References: <20220907100626.32657-1-zhangqing@loongson.cn>
From:   Jinyang He <hejinyang@loongson.cn>
In-Reply-To: <20220907100626.32657-1-zhangqing@loongson.cn>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2022/9/7 18:06, Qing Zhang wrote:

> Ftrace plays like function head exception, prologue analysis will stop soon
> because PC is at entry.
>
>     90000000004c5a54 <callee>:
>     90000000004c5a54:	03400000  andi   $zero, $zero, 0x0   ==>move  t0, ra
>     90000000004c5a58:	03400000  andi   $zero, $zero, 0x0   ==>bl callsite
> ==>90000000004c5a5c:	02fcc063  addi.d $sp, $sp, -208(0xf30)
>     ...
>
> When encountering ftrace_call, save trace function ra at PT_ERA, save parent ra
> at PT_R1, At this time, pc is the position after the two nops of callee. There is
> no conventional prologue operation between this position and the function entry,
> so we need to resetthe first flag to make the caller continue to unwind.
>
> testing method:
>    echo path_openat > ./set_ftrace_filter
>    echo 1 > ./options/func_stack_trace
>    echo function > ./current_tracer
>
> Signed-off-by: Qing Zhang <zhangqing@loongson.cn>
> ---
>   arch/loongarch/include/asm/unwind.h     |  2 +-
>   arch/loongarch/kernel/unwind_prologue.c | 33 +++++++++++++++++++++----
>   2 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/arch/loongarch/include/asm/unwind.h b/arch/loongarch/include/asm/unwind.h
> index f66b07c3e6a1..f2b52b9ea93d 100644
> --- a/arch/loongarch/include/asm/unwind.h
> +++ b/arch/loongarch/include/asm/unwind.h
> @@ -20,7 +20,7 @@ struct unwind_state {
>   	char type; /* UNWINDER_XXX */
>   	struct stack_info stack_info;
>   	struct task_struct *task;
> -	bool first, error;
> +	bool first, error, is_ftrace;
>   	int graph_idx;
>   	unsigned long sp, pc, ra;
>   };
> diff --git a/arch/loongarch/kernel/unwind_prologue.c b/arch/loongarch/kernel/unwind_prologue.c
> index 03f8b31a90cc..0bb19d3ae8ba 100644
> --- a/arch/loongarch/kernel/unwind_prologue.c
> +++ b/arch/loongarch/kernel/unwind_prologue.c
> @@ -14,9 +14,7 @@ unsigned long unwind_get_return_address(struct unwind_state *state)
>   
>   	if (unwind_done(state))
>   		return 0;
> -	else if (state->type)
> -		return state->pc;
> -	else if (state->first)
> +	else if (state->type || state->first)
>   		return state->pc;
>   
>   	return *(unsigned long *)(state->sp);
> @@ -42,16 +40,39 @@ static bool unwind_by_guess(struct unwind_state *state)
>   	return false;
>   }
>   
> +static void unwind_state_fixup(struct unwind_state *state)
> +{
> +	static unsigned long ftrace_case = (unsigned long)ftrace_call + 4;
> +
> +	if (state->pc == ftrace_case)
> +		state->is_ftrace = true;
> +}
> +
>   static bool unwind_by_prologue(struct unwind_state *state)
>   {
>   	struct stack_info *info = &state->stack_info;
>   	union loongarch_instruction *ip, *ip_end;
>   	unsigned long frame_size = 0, frame_ra = -1;
>   	unsigned long size, offset, pc = state->pc;
> +	struct pt_regs *regs;
>   
>   	if (state->sp >= info->end || state->sp < info->begin)
>   		return false;
>   
> +	if (state->is_ftrace) {
> +		/*
> +		 * As we meet ftrace_regs_entry, reset first flag like first doing
> +		 * tracing, Prologue analysis will stop soon because PC is at entry.
> +		 */
> +		regs = (struct pt_regs *)state->sp;
> +		state->pc = regs->csr_era;
> +		state->ra = regs->regs[1];
> +		state->sp = regs->regs[3];
> +		state->first = true;
> +		state->is_ftrace = false;
> +		return true;
> +	}
> +
>   	if (!kallsyms_lookup_size_offset(pc, &size, &offset))
>   		return false;
>   
> @@ -97,7 +118,7 @@ static bool unwind_by_prologue(struct unwind_state *state)
>   
>   	state->pc = *(unsigned long *)(state->sp + frame_ra);
>   	state->sp = state->sp + frame_size;
> -	return !!__kernel_text_address(state->pc);
> +	goto out;
>   
>   first:
>   	state->first = false;
> @@ -106,7 +127,9 @@ static bool unwind_by_prologue(struct unwind_state *state)
>   
>   	state->pc = state->ra;
>   
> -	return !!__kernel_text_address(state->ra);
> +out:
> +	unwind_state_fixup(state);
> +	return !!__kernel_text_address(state->pc);
>   }
>   
>   void unwind_start(struct unwind_state *state, struct task_struct *task,

Hi, Qing

Additional 'Reported-by: Jeff Xie <xiehuan09@gmail.com>' is welcome.

For this patch, Tested-by: Jinyang He <hejinyang@loongson.cn>.


Thanks,

Jinyang