function:stacktrace/mips: Fix function:stacktrace for mips
diff mbox series

Message ID 20200528123640.4285-1-yuanjunqing66@163.com
State New
Headers show
Series
  • function:stacktrace/mips: Fix function:stacktrace for mips
Related show

Commit Message

YuanJunQing May 28, 2020, 12:36 p.m. UTC
ftrace_call as global symbol in ftrace_caller(), this
will cause function:stacktrace can not work well.
i.e. echo do_IRQ:stacktrace > set_ftrace_filte

Signed-off-by: YuanJunQing <yuanjunqing66@163.com>
---
 arch/mips/kernel/mcount.S | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Comments

YuanJunQing May 29, 2020, 9:29 a.m. UTC | #1
May I ask if you have received this email.

在 2020/5/28 下午8:36, YuanJunQing 写道:
> ftrace_call as global symbol in ftrace_caller(), this
> will cause function:stacktrace can not work well.
> i.e. echo do_IRQ:stacktrace > set_ftrace_filte
>
> Signed-off-by: YuanJunQing <yuanjunqing66@163.com>
> ---
>  arch/mips/kernel/mcount.S | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/arch/mips/kernel/mcount.S b/arch/mips/kernel/mcount.S
> index cff52b283e03..cd5545764e5f 100644
> --- a/arch/mips/kernel/mcount.S
> +++ b/arch/mips/kernel/mcount.S
> @@ -87,8 +87,15 @@ EXPORT_SYMBOL(_mcount)
>  	PTR_LA   t1, _etext
>  	sltu     t3, t1, a0	/* t3 = (a0 > _etext) */
>  	or       t1, t2, t3
> +	PTR_LA	 t2, stlab-4 	/* t2: "function:stacktrace" return address */
> +	move	 a1, AT		/* arg2: parent's return address */
>  	beqz     t1, ftrace_call
> -	 nop
> +	 nop			/* "function:stacktrace" return address */
> +stlab:
> +	PTR_LA	t2, stlab-4
> +	/* ftrace_call_end: ftrace_call return address */
> +	beq	t2,ra, ftrace_call_end
> +	nop
>  #if defined(KBUILD_MCOUNT_RA_ADDRESS) && defined(CONFIG_32BIT)
>  	PTR_SUBU a0, a0, 16	/* arg1: adjust to module's recorded callsite */
>  #else
> @@ -98,7 +105,9 @@ EXPORT_SYMBOL(_mcount)
>  	.globl ftrace_call
>  ftrace_call:
>  	nop	/* a placeholder for the call to a real tracing function */
> -	 move	a1, AT		/* arg2: parent's return address */
> +	move	ra, t2		/* t2: "function:stacktrace" return address */
> +
> +ftrace_call_end:
>  
>  #ifdef CONFIG_FUNCTION_GRAPH_TRACER
>  	.globl ftrace_graph_call
WANG Xuerui May 29, 2020, 9:41 a.m. UTC | #2
On 2020/5/29 17:29, yuanjunqing wrote:

> May I ask if you have received this email.

At least I have received the complete thread in my inbox, so others may 
well have no problem.

It's not polite to ping maintainers just one day after sending your 
patch BTW; keep the patch submitting guide[1] under your pillow.

Lastly, don't top-post even though your message body is short.


[1]: 
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#don-t-get-discouraged-or-impatient


And a bit of review while we're at it...

>
> 在 2020/5/28 下午8:36, YuanJunQing 写道:
>> ftrace_call as global symbol in ftrace_caller(), this
>> will cause function:stacktrace can not work well.
Chinglish. I can't understand this despite being a Chinese myself...
>> i.e. echo do_IRQ:stacktrace > set_ftrace_filte
The commit message is truncated? I can't seem to make sense of this line.
>>
>> Signed-off-by: YuanJunQing <yuanjunqing66@163.com>
>> ---
>>   arch/mips/kernel/mcount.S | 13 +++++++++++--
>>   1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/mips/kernel/mcount.S b/arch/mips/kernel/mcount.S
>> index cff52b283e03..cd5545764e5f 100644
>> --- a/arch/mips/kernel/mcount.S
>> +++ b/arch/mips/kernel/mcount.S
>> @@ -87,8 +87,15 @@ EXPORT_SYMBOL(_mcount)
>>   	PTR_LA   t1, _etext
>>   	sltu     t3, t1, a0	/* t3 = (a0 > _etext) */
>>   	or       t1, t2, t3
>> +	PTR_LA	 t2, stlab-4 	/* t2: "function:stacktrace" return address */
>> +	move	 a1, AT		/* arg2: parent's return address */
>>   	beqz     t1, ftrace_call
>> -	 nop
>> +	 nop			/* "function:stacktrace" return address */
>> +stlab:
>> +	PTR_LA	t2, stlab-4
>> +	/* ftrace_call_end: ftrace_call return address */
>> +	beq	t2,ra, ftrace_call_end
>> +	nop
>>   #if defined(KBUILD_MCOUNT_RA_ADDRESS) && defined(CONFIG_32BIT)
>>   	PTR_SUBU a0, a0, 16	/* arg1: adjust to module's recorded callsite */
>>   #else
>> @@ -98,7 +105,9 @@ EXPORT_SYMBOL(_mcount)
>>   	.globl ftrace_call
>>   ftrace_call:
>>   	nop	/* a placeholder for the call to a real tracing function */
>> -	 move	a1, AT		/* arg2: parent's return address */
>> +	move	ra, t2		/* t2: "function:stacktrace" return address */
>> +
>> +ftrace_call_end:
>>   
>>   #ifdef CONFIG_FUNCTION_GRAPH_TRACER
>>   	.globl ftrace_graph_call
Maciej W. Rozycki June 4, 2020, 1:17 a.m. UTC | #3
On Fri, 29 May 2020, WANG Xuerui wrote:

> On 2020/5/29 17:29, yuanjunqing wrote:
> 
> >> diff --git a/arch/mips/kernel/mcount.S b/arch/mips/kernel/mcount.S
> >> index cff52b283e03..cd5545764e5f 100644
> >> --- a/arch/mips/kernel/mcount.S
> >> +++ b/arch/mips/kernel/mcount.S
> >> @@ -87,8 +87,15 @@ EXPORT_SYMBOL(_mcount)
> >>   	PTR_LA   t1, _etext
> >>   	sltu     t3, t1, a0	/* t3 = (a0 > _etext) */
> >>   	or       t1, t2, t3
> >> +	PTR_LA	 t2, stlab-4 	/* t2: "function:stacktrace" return address */
> >> +	move	 a1, AT		/* arg2: parent's return address */
> >>   	beqz     t1, ftrace_call
> >> -	 nop
> >> +	 nop			/* "function:stacktrace" return address */
> >> +stlab:
> >> +	PTR_LA	t2, stlab-4
> >> +	/* ftrace_call_end: ftrace_call return address */
> >> +	beq	t2,ra, ftrace_call_end
> >> +	nop

 Broken delay slot indentation.

> >>   #if defined(KBUILD_MCOUNT_RA_ADDRESS) && defined(CONFIG_32BIT)
> >>   	PTR_SUBU a0, a0, 16	/* arg1: adjust to module's recorded callsite */
> >>   #else
> >> @@ -98,7 +105,9 @@ EXPORT_SYMBOL(_mcount)
> >>   	.globl ftrace_call
> >>   ftrace_call:
> >>   	nop	/* a placeholder for the call to a real tracing function */
> >> -	 move	a1, AT		/* arg2: parent's return address */
> >> +	move	ra, t2		/* t2: "function:stacktrace" return address */

 Likewise.  NB I haven't investigated if the change makes sense.  A more 
detailed explanation in the change description is certainly needed.

  Maciej
YuanJunQing June 4, 2020, 3:25 a.m. UTC | #4
在 2020/6/4 上午9:17, Maciej W. Rozycki 写道:
> On Fri, 29 May 2020, WANG Xuerui wrote:
>
>> On 2020/5/29 17:29, yuanjunqing wrote:
>>
>>>> diff --git a/arch/mips/kernel/mcount.S b/arch/mips/kernel/mcount.S
>>>> index cff52b283e03..cd5545764e5f 100644
>>>> --- a/arch/mips/kernel/mcount.S
>>>> +++ b/arch/mips/kernel/mcount.S
>>>> @@ -87,8 +87,15 @@ EXPORT_SYMBOL(_mcount)
>>>>   	PTR_LA   t1, _etext
>>>>   	sltu     t3, t1, a0	/* t3 = (a0 > _etext) */
>>>>   	or       t1, t2, t3
>>>> +	PTR_LA	 t2, stlab-4 	/* t2: "function:stacktrace" return address */
>>>> +	move	 a1, AT		/* arg2: parent's return address */
>>>>   	beqz     t1, ftrace_call
>>>> -	 nop
>>>> +	 nop			/* "function:stacktrace" return address */
>>>> +stlab:
>>>> +	PTR_LA	t2, stlab-4
>>>> +	/* ftrace_call_end: ftrace_call return address */
>>>> +	beq	t2,ra, ftrace_call_end
>>>> +	nop
>  Broken delay slot indentation.

Thank you for your reply. For this question that you mentioned about the delay slot, I will modify my patch again.

>
>>>>   #if defined(KBUILD_MCOUNT_RA_ADDRESS) && defined(CONFIG_32BIT)
>>>>   	PTR_SUBU a0, a0, 16	/* arg1: adjust to module's recorded callsite */
>>>>   #else
>>>> @@ -98,7 +105,9 @@ EXPORT_SYMBOL(_mcount)
>>>>   	.globl ftrace_call
>>>>   ftrace_call:
>>>>   	nop	/* a placeholder for the call to a real tracing function */
>>>> -	 move	a1, AT		/* arg2: parent's return address */
>>>> +	move	ra, t2		/* t2: "function:stacktrace" return address */
>  Likewise.  NB I haven't investigated if the change makes sense.  A more 
> detailed explanation in the change description is certainly needed.

I will attach a specific description for further explanation about the second patch later.

>
>   Maciej

Patch
diff mbox series

diff --git a/arch/mips/kernel/mcount.S b/arch/mips/kernel/mcount.S
index cff52b283e03..cd5545764e5f 100644
--- a/arch/mips/kernel/mcount.S
+++ b/arch/mips/kernel/mcount.S
@@ -87,8 +87,15 @@  EXPORT_SYMBOL(_mcount)
 	PTR_LA   t1, _etext
 	sltu     t3, t1, a0	/* t3 = (a0 > _etext) */
 	or       t1, t2, t3
+	PTR_LA	 t2, stlab-4 	/* t2: "function:stacktrace" return address */
+	move	 a1, AT		/* arg2: parent's return address */
 	beqz     t1, ftrace_call
-	 nop
+	 nop			/* "function:stacktrace" return address */
+stlab:
+	PTR_LA	t2, stlab-4
+	/* ftrace_call_end: ftrace_call return address */
+	beq	t2,ra, ftrace_call_end
+	nop
 #if defined(KBUILD_MCOUNT_RA_ADDRESS) && defined(CONFIG_32BIT)
 	PTR_SUBU a0, a0, 16	/* arg1: adjust to module's recorded callsite */
 #else
@@ -98,7 +105,9 @@  EXPORT_SYMBOL(_mcount)
 	.globl ftrace_call
 ftrace_call:
 	nop	/* a placeholder for the call to a real tracing function */
-	 move	a1, AT		/* arg2: parent's return address */
+	move	ra, t2		/* t2: "function:stacktrace" return address */
+
+ftrace_call_end:
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 	.globl ftrace_graph_call