[PATCH 0/2] Ftrace fixes

[PATCH 0/2] Ftrace fixes

[Julien Thierry]

Hi,

>From what Steve Rostedt said, we shouldn't rely on ftrace_trace_function
not being ftrace_stub when using the function graph tracer. As x86 does,
always check whether function graph tracing is requested in static tracing.

While testing that I didn't break things, I discovered that the perf ftrace
builtin doesn't handle kernels built without function graph tracer
(regardless of whether tracing is dynamic or not).

First patch fixes the ftrace issue.
Second patch fixes the perf front-end.

Cheers,

Julien Thierry (2):
  arm64: Fix static use of function graph
  perf: Fix ftrace builtin when kernel doesn't have function_graph

 arch/arm64/kernel/entry-ftrace.S | 12 +++---------
 tools/perf/builtin-ftrace.c      |  3 ++-
 2 files changed, 5 insertions(+), 10 deletions(-)

--
1.9.1

[PATCH 1/2] arm64: Fix static use of function graph

[Julien Thierry]

Function graph does not work currently when CONFIG_DYNAMIC_TRACE is not
set. This is because ftrace_function_trace is not always set to ftrace_stub
when function_graph is in use.

Do not skip checking of graph tracer functions when ftrace_function_trace
is set.

Signed-off-by: Julien Thierry <julien.thierry@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 arch/arm64/kernel/entry-ftrace.S | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kernel/entry-ftrace.S b/arch/arm64/kernel/entry-ftrace.S
index e1be42e..1175f58 100644
--- a/arch/arm64/kernel/entry-ftrace.S
+++ b/arch/arm64/kernel/entry-ftrace.S
@@ -108,13 +108,8 @@ ENTRY(_mcount)
 	mcount_get_lr	x1		//       function's lr (= parent's pc)
 	blr	x2			//   (*ftrace_trace_function)(pc, lr);

-#ifndef CONFIG_FUNCTION_GRAPH_TRACER
-skip_ftrace_call:			//   return;
-	mcount_exit			// }
-#else
-	mcount_exit			//   return;
-					// }
-skip_ftrace_call:
+skip_ftrace_call:			// }
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
 	ldr_l	x2, ftrace_graph_return
 	cmp	x0, x2			//   if ((ftrace_graph_return
 	b.ne	ftrace_graph_caller	//        != ftrace_stub)
@@ -123,9 +118,8 @@ skip_ftrace_call:
 	adr_l	x0, ftrace_graph_entry_stub //     != ftrace_graph_entry_stub))
 	cmp	x0, x2
 	b.ne	ftrace_graph_caller	//     ftrace_graph_caller();
-
-	mcount_exit
 #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
+	mcount_exit
 ENDPROC(_mcount)

 #else /* CONFIG_DYNAMIC_FTRACE */
--
1.9.1

[PATCH 2/2] perf: Fix ftrace builtin when kernel doesn't have function_graph

[Julien Thierry]

When linux is built without support for function graph tracer, the ftrace
builtin of perf will fail when trying to reset max_graph_depth because the
file does not exist. This prevents the use of function tracer from perf.

Do not attempt to write this file when the tracer in use is not
function_graph.

Signed-off-by: Julien Thierry <julien.thierry@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: linux-kernel@vger.kernel.org
---
 tools/perf/builtin-ftrace.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index 25a42ac..48120f2 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -130,7 +130,8 @@ static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
 	if (reset_tracing_cpu() < 0)
 		return -1;

-	if (write_tracing_file("max_graph_depth", "0") < 0)
+	if (!strcmp(ftrace->tracer, "function_graph") &&
+	    write_tracing_file("max_graph_depth", "0") < 0)
 		return -1;

 	reset_tracing_filters();
--
1.9.1

[PATCH 2/2] perf: Fix ftrace builtin when kernel doesn't have function_graph

[Julien Thierry]

When linux is built without support for function graph tracer, the ftrace
builtin of perf will fail when trying to reset max_graph_depth because the
file does not exist. This prevents the use of function tracer from perf.

Do not attempt to write this file when the tracer in use is not
function_graph.

Signed-off-by: Julien Thierry <julien.thierry@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: linux-kernel at vger.kernel.org
---
 tools/perf/builtin-ftrace.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index 25a42ac..48120f2 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -130,7 +130,8 @@ static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
 	if (reset_tracing_cpu() < 0)
 		return -1;

-	if (write_tracing_file("max_graph_depth", "0") < 0)
+	if (!strcmp(ftrace->tracer, "function_graph") &&
+	    write_tracing_file("max_graph_depth", "0") < 0)
 		return -1;

 	reset_tracing_filters();
--
1.9.1

[PATCH 1/2] arm64: Fix static use of function graph

[Steven Rostedt]

On Wed,  1 Nov 2017 14:33:43 +0000
Julien Thierry <julien.thierry@arm.com> wrote:

> Function graph does not work currently when CONFIG_DYNAMIC_TRACE is not
> set. This is because ftrace_function_trace is not always set to ftrace_stub
> when function_graph is in use.
> 
> Do not skip checking of graph tracer functions when ftrace_function_trace
> is set.
> 
> Signed-off-by: Julien Thierry <julien.thierry@arm.com>

My arm code is very rusty and I have no idea if this works or not, but
for what it's worth.

Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve

> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  arch/arm64/kernel/entry-ftrace.S | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm64/kernel/entry-ftrace.S b/arch/arm64/kernel/entry-ftrace.S
> index e1be42e..1175f58 100644
> --- a/arch/arm64/kernel/entry-ftrace.S
> +++ b/arch/arm64/kernel/entry-ftrace.S
> @@ -108,13 +108,8 @@ ENTRY(_mcount)
>  	mcount_get_lr	x1		//       function's lr (= parent's pc)
>  	blr	x2			//   (*ftrace_trace_function)(pc, lr);
> 
> -#ifndef CONFIG_FUNCTION_GRAPH_TRACER
> -skip_ftrace_call:			//   return;
> -	mcount_exit			// }
> -#else
> -	mcount_exit			//   return;
> -					// }
> -skip_ftrace_call:
> +skip_ftrace_call:			// }
> +#ifdef CONFIG_FUNCTION_GRAPH_TRACER
>  	ldr_l	x2, ftrace_graph_return
>  	cmp	x0, x2			//   if ((ftrace_graph_return
>  	b.ne	ftrace_graph_caller	//        != ftrace_stub)
> @@ -123,9 +118,8 @@ skip_ftrace_call:
>  	adr_l	x0, ftrace_graph_entry_stub //     != ftrace_graph_entry_stub))
>  	cmp	x0, x2
>  	b.ne	ftrace_graph_caller	//     ftrace_graph_caller();
> -
> -	mcount_exit
>  #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
> +	mcount_exit
>  ENDPROC(_mcount)
> 
>  #else /* CONFIG_DYNAMIC_FTRACE */
> --
> 1.9.1

Re: [PATCH 2/2] perf: Fix ftrace builtin when kernel doesn't have function_graph

[Steven Rostedt]

On Wed,  1 Nov 2017 14:33:44 +0000
Julien Thierry <julien.thierry@arm.com> wrote:

> When linux is built without support for function graph tracer, the ftrace
> builtin of perf will fail when trying to reset max_graph_depth because the
> file does not exist. This prevents the use of function tracer from perf.
> 
> Do not attempt to write this file when the tracer in use is not
> function_graph.
> 
> Signed-off-by: Julien Thierry <julien.thierry@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: linux-kernel@vger.kernel.org
> ---
>  tools/perf/builtin-ftrace.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> index 25a42ac..48120f2 100644
> --- a/tools/perf/builtin-ftrace.c
> +++ b/tools/perf/builtin-ftrace.c
> @@ -130,7 +130,8 @@ static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
>  	if (reset_tracing_cpu() < 0)
>  		return -1;
> 
> -	if (write_tracing_file("max_graph_depth", "0") < 0)
> +	if (!strcmp(ftrace->tracer, "function_graph") &&
> +	    write_tracing_file("max_graph_depth", "0") < 0)

Hmm, instead of doing this, could we just do a stat on the file first.
As with trace-cmd, I like to reset files like this even when not
enabling function_graph tracer.

-- Steve


>  		return -1;
> 
>  	reset_tracing_filters();
> --
> 1.9.1

[PATCH 2/2] perf: Fix ftrace builtin when kernel doesn't have function_graph

[Steven Rostedt]

On Wed,  1 Nov 2017 14:33:44 +0000
Julien Thierry <julien.thierry@arm.com> wrote:

> When linux is built without support for function graph tracer, the ftrace
> builtin of perf will fail when trying to reset max_graph_depth because the
> file does not exist. This prevents the use of function tracer from perf.
> 
> Do not attempt to write this file when the tracer in use is not
> function_graph.
> 
> Signed-off-by: Julien Thierry <julien.thierry@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: linux-kernel at vger.kernel.org
> ---
>  tools/perf/builtin-ftrace.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> index 25a42ac..48120f2 100644
> --- a/tools/perf/builtin-ftrace.c
> +++ b/tools/perf/builtin-ftrace.c
> @@ -130,7 +130,8 @@ static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
>  	if (reset_tracing_cpu() < 0)
>  		return -1;
> 
> -	if (write_tracing_file("max_graph_depth", "0") < 0)
> +	if (!strcmp(ftrace->tracer, "function_graph") &&
> +	    write_tracing_file("max_graph_depth", "0") < 0)

Hmm, instead of doing this, could we just do a stat on the file first.
As with trace-cmd, I like to reset files like this even when not
enabling function_graph tracer.

-- Steve


>  		return -1;
> 
>  	reset_tracing_filters();
> --
> 1.9.1

Re: [PATCH 2/2] perf: Fix ftrace builtin when kernel doesn't have function_graph

[Julien Thierry]

On 01/11/17 16:44, Steven Rostedt wrote:
> On Wed,  1 Nov 2017 14:33:44 +0000
> Julien Thierry <julien.thierry@arm.com> wrote:
> 
>> When linux is built without support for function graph tracer, the ftrace
>> builtin of perf will fail when trying to reset max_graph_depth because the
>> file does not exist. This prevents the use of function tracer from perf.
>>
>> Do not attempt to write this file when the tracer in use is not
>> function_graph.
>>
>> Signed-off-by: Julien Thierry <julien.thierry@arm.com>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: linux-kernel@vger.kernel.org
>> ---
>>   tools/perf/builtin-ftrace.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
>> index 25a42ac..48120f2 100644
>> --- a/tools/perf/builtin-ftrace.c
>> +++ b/tools/perf/builtin-ftrace.c
>> @@ -130,7 +130,8 @@ static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
>>   	if (reset_tracing_cpu() < 0)
>>   		return -1;
>>
>> -	if (write_tracing_file("max_graph_depth", "0") < 0)
>> +	if (!strcmp(ftrace->tracer, "function_graph") &&
>> +	    write_tracing_file("max_graph_depth", "0") < 0)
> 
> Hmm, instead of doing this, could we just do a stat on the file first.
> As with trace-cmd, I like to reset files like this even when not
> enabling function_graph tracer.
> 

Sounds reasonable.

I'll include that change in the next version of the patches.

Thanks,

-- 
Julien Thierry

[PATCH 2/2] perf: Fix ftrace builtin when kernel doesn't have function_graph

[Julien Thierry]

On 01/11/17 16:44, Steven Rostedt wrote:
> On Wed,  1 Nov 2017 14:33:44 +0000
> Julien Thierry <julien.thierry@arm.com> wrote:
> 
>> When linux is built without support for function graph tracer, the ftrace
>> builtin of perf will fail when trying to reset max_graph_depth because the
>> file does not exist. This prevents the use of function tracer from perf.
>>
>> Do not attempt to write this file when the tracer in use is not
>> function_graph.
>>
>> Signed-off-by: Julien Thierry <julien.thierry@arm.com>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: linux-kernel at vger.kernel.org
>> ---
>>   tools/perf/builtin-ftrace.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
>> index 25a42ac..48120f2 100644
>> --- a/tools/perf/builtin-ftrace.c
>> +++ b/tools/perf/builtin-ftrace.c
>> @@ -130,7 +130,8 @@ static int reset_tracing_files(struct perf_ftrace *ftrace __maybe_unused)
>>   	if (reset_tracing_cpu() < 0)
>>   		return -1;
>>
>> -	if (write_tracing_file("max_graph_depth", "0") < 0)
>> +	if (!strcmp(ftrace->tracer, "function_graph") &&
>> +	    write_tracing_file("max_graph_depth", "0") < 0)
> 
> Hmm, instead of doing this, could we just do a stat on the file first.
> As with trace-cmd, I like to reset files like this even when not
> enabling function_graph tracer.
> 

Sounds reasonable.

I'll include that change in the next version of the patches.

Thanks,

-- 
Julien Thierry

[PATCH 1/2] arm64: Fix static use of function graph

[AKASHI Takahiro]

On Wed, Nov 01, 2017 at 02:33:43PM +0000, Julien Thierry wrote:
> Function graph does not work currently when CONFIG_DYNAMIC_TRACE is not
> set. This is because ftrace_function_trace is not always set to ftrace_stub
> when function_graph is in use.
> 
> Do not skip checking of graph tracer functions when ftrace_function_trace
> is set.
> 
> Signed-off-by: Julien Thierry <julien.thierry@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>

Reviewed-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

I think that arm has the same issue, hoping the following patch
will fix it:
===
>From e25bcf50d1acde698285a0c64f72d97f8b17e3fb Mon Sep 17 00:00:00 2001
From: AKASHI Takahiro <takahiro.akashi@linaro.org>
Date: Thu, 2 Nov 2017 11:35:04 +0900
Subject: [PATCH] arm: ftrace: function_graph with DYNAMIC_FTRACE

---
 arch/arm/kernel/entry-ftrace.S | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/arch/arm/kernel/entry-ftrace.S b/arch/arm/kernel/entry-ftrace.S
index efcd9f25a14b..ef94c73ad996 100644
--- a/arch/arm/kernel/entry-ftrace.S
+++ b/arch/arm/kernel/entry-ftrace.S
@@ -68,8 +68,12 @@
 	ldr	r2, [r0]
 	adr	r0, .Lftrace_stub
 	cmp	r0, r2
-	bne	1f
+	beq	1f
 
+ 	mcount_get_lr	r1			@ lr of instrumented func
+	mcount_adjust_addr	r0, lr		@ instrumented function
+	blx	r2
+1:
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 	ldr     r1, =ftrace_graph_return
 	ldr     r2, [r1]
@@ -84,12 +88,6 @@
 #endif
 
 	mcount_exit
-
-1: 	mcount_get_lr	r1			@ lr of instrumented func
-	mcount_adjust_addr	r0, lr		@ instrumented function
-	badr	lr, 2f
-	mov	pc, r2
-2:	mcount_exit
 .endm
 
 #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
-- 
2.14.1

[PATCH 1/2] arm64: Fix static use of function graph

[Russell King - ARM Linux]

On Thu, Nov 02, 2017 at 01:43:07PM +0900, AKASHI Takahiro wrote:
> On Wed, Nov 01, 2017 at 02:33:43PM +0000, Julien Thierry wrote:
> > Function graph does not work currently when CONFIG_DYNAMIC_TRACE is not
> > set. This is because ftrace_function_trace is not always set to ftrace_stub
> > when function_graph is in use.
> > 
> > Do not skip checking of graph tracer functions when ftrace_function_trace
> > is set.
> > 
> > Signed-off-by: Julien Thierry <julien.thierry@arm.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
> 
> Reviewed-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> 
> I think that arm has the same issue, hoping the following patch
> will fix it:
> ===
> >From e25bcf50d1acde698285a0c64f72d97f8b17e3fb Mon Sep 17 00:00:00 2001
> From: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Date: Thu, 2 Nov 2017 11:35:04 +0900
> Subject: [PATCH] arm: ftrace: function_graph with DYNAMIC_FTRACE
> 
> ---
>  arch/arm/kernel/entry-ftrace.S | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/kernel/entry-ftrace.S b/arch/arm/kernel/entry-ftrace.S
> index efcd9f25a14b..ef94c73ad996 100644
> --- a/arch/arm/kernel/entry-ftrace.S
> +++ b/arch/arm/kernel/entry-ftrace.S
> @@ -68,8 +68,12 @@
>  	ldr	r2, [r0]
>  	adr	r0, .Lftrace_stub
>  	cmp	r0, r2
> -	bne	1f
> +	beq	1f
>  
> + 	mcount_get_lr	r1			@ lr of instrumented func
> +	mcount_adjust_addr	r0, lr		@ instrumented function
> +	blx	r2

NAK.  Not all CPUs support "blx".  I don't see why you'd make this
gratuitous change when just moving code around.  Please separate
your changes in future.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH 1/2] arm64: Fix static use of function graph

[Julien Thierry]

Hi,

On 02/11/17 10:18, Russell King - ARM Linux wrote:
> On Thu, Nov 02, 2017 at 01:43:07PM +0900, AKASHI Takahiro wrote:
>> On Wed, Nov 01, 2017 at 02:33:43PM +0000, Julien Thierry wrote:
>>> Function graph does not work currently when CONFIG_DYNAMIC_TRACE is not
>>> set. This is because ftrace_function_trace is not always set to ftrace_stub
>>> when function_graph is in use.
>>>
>>> Do not skip checking of graph tracer functions when ftrace_function_trace
>>> is set.
>>>
>>> Signed-off-by: Julien Thierry <julien.thierry@arm.com>
>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>> Cc: Will Deacon <will.deacon@arm.com>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
>>
>> Reviewed-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>>
>> I think that arm has the same issue, hoping the following patch
>> will fix it:

Thanks for noticing this and for the patch. I'll re-spin a version for 
this series and include your patch.

>> ===
>> >From e25bcf50d1acde698285a0c64f72d97f8b17e3fb Mon Sep 17 00:00:00 2001
>> From: AKASHI Takahiro <takahiro.akashi@linaro.org>
>> Date: Thu, 2 Nov 2017 11:35:04 +0900
>> Subject: [PATCH] arm: ftrace: function_graph with DYNAMIC_FTRACE
>>
>> ---
>>   arch/arm/kernel/entry-ftrace.S | 12 +++++-------
>>   1 file changed, 5 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm/kernel/entry-ftrace.S b/arch/arm/kernel/entry-ftrace.S
>> index efcd9f25a14b..ef94c73ad996 100644
>> --- a/arch/arm/kernel/entry-ftrace.S
>> +++ b/arch/arm/kernel/entry-ftrace.S
>> @@ -68,8 +68,12 @@
>>   	ldr	r2, [r0]
>>   	adr	r0, .Lftrace_stub
>>   	cmp	r0, r2
>> -	bne	1f
>> +	beq	1f
>>   
>> + 	mcount_get_lr	r1			@ lr of instrumented func
>> +	mcount_adjust_addr	r0, lr		@ instrumented function
>> +	blx	r2
> 
> NAK.  Not all CPUs support "blx".  I don't see why you'd make this
> gratuitous change when just moving code around.  Please separate
> your changes in future.
> 

I'll change the blx back to the badr + mov instructions in the new version.

Thanks,

-- 
Julien Thierry

[PATCH 1/2] arm64: Fix static use of function graph

[AKASHI Takahiro]

On Fri, Nov 03, 2017 at 09:25:07AM +0000, Julien Thierry wrote:
> Hi,
> 
> On 02/11/17 10:18, Russell King - ARM Linux wrote:
> >On Thu, Nov 02, 2017 at 01:43:07PM +0900, AKASHI Takahiro wrote:
> >>On Wed, Nov 01, 2017 at 02:33:43PM +0000, Julien Thierry wrote:
> >>>Function graph does not work currently when CONFIG_DYNAMIC_TRACE is not
> >>>set. This is because ftrace_function_trace is not always set to ftrace_stub
> >>>when function_graph is in use.
> >>>
> >>>Do not skip checking of graph tracer functions when ftrace_function_trace
> >>>is set.
> >>>
> >>>Signed-off-by: Julien Thierry <julien.thierry@arm.com>
> >>>Cc: Catalin Marinas <catalin.marinas@arm.com>
> >>>Cc: Will Deacon <will.deacon@arm.com>
> >>>Cc: Mark Rutland <mark.rutland@arm.com>
> >>>Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
> >>
> >>Reviewed-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> >>
> >>I think that arm has the same issue, hoping the following patch
> >>will fix it:
> 
> Thanks for noticing this and for the patch. I'll re-spin a version for this
> series and include your patch.

Thanks. You'd better fix a bug in the subject, which should be
"!DYNAMIC_FTRACE" or use the same title as yours.

-Takahiro AKASHI

> >>===
> >>>From e25bcf50d1acde698285a0c64f72d97f8b17e3fb Mon Sep 17 00:00:00 2001
> >>From: AKASHI Takahiro <takahiro.akashi@linaro.org>
> >>Date: Thu, 2 Nov 2017 11:35:04 +0900
> >>Subject: [PATCH] arm: ftrace: function_graph with DYNAMIC_FTRACE
> >>
> >>---
> >>  arch/arm/kernel/entry-ftrace.S | 12 +++++-------
> >>  1 file changed, 5 insertions(+), 7 deletions(-)
> >>
> >>diff --git a/arch/arm/kernel/entry-ftrace.S b/arch/arm/kernel/entry-ftrace.S
> >>index efcd9f25a14b..ef94c73ad996 100644
> >>--- a/arch/arm/kernel/entry-ftrace.S
> >>+++ b/arch/arm/kernel/entry-ftrace.S
> >>@@ -68,8 +68,12 @@
> >>  	ldr	r2, [r0]
> >>  	adr	r0, .Lftrace_stub
> >>  	cmp	r0, r2
> >>-	bne	1f
> >>+	beq	1f
> >>+ 	mcount_get_lr	r1			@ lr of instrumented func
> >>+	mcount_adjust_addr	r0, lr		@ instrumented function
> >>+	blx	r2
> >
> >NAK.  Not all CPUs support "blx".  I don't see why you'd make this
> >gratuitous change when just moving code around.  Please separate
> >your changes in future.
> >
> 
> I'll change the blx back to the badr + mov instructions in the new version.
> 
> Thanks,
> 
> -- 
> Julien Thierry

Re: [PATCH 0/2] ftrace fixes

[patchwork-bot+linux-riscv]

Hello:

This series was applied to riscv/linux.git (fixes)
by Palmer Dabbelt <palmer@rivosinc.com>:

On Tue, 26 Mar 2024 21:30:15 +0100 you wrote:
> Both were reported recently as there are efforts ongoing to
> reimplement ftrace on riscv and both are independent of this rework,
> so here they are.
> 
> Alexandre Ghiti (2):
>   riscv: Fix warning by declaring arch_cpu_idle() as noinstr
>   riscv: Disable preemption when using patch_map()
> 
> [...]

Here is the summary with links:
  - [1/2] riscv: Fix warning by declaring arch_cpu_idle() as noinstr
    https://git.kernel.org/riscv/c/8a48ea87ce89
  - [2/2] riscv: Disable preemption when using patch_map()
    https://git.kernel.org/riscv/c/a370c2419e46

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH 0/2] ftrace fixes

[patchwork-bot+linux-riscv]

Hello:

This series was applied to riscv/linux.git (fixes)
by Palmer Dabbelt <palmer@rivosinc.com>:

On Tue, 26 Mar 2024 21:30:15 +0100 you wrote:
> Both were reported recently as there are efforts ongoing to
> reimplement ftrace on riscv and both are independent of this rework,
> so here they are.
> 
> Alexandre Ghiti (2):
>   riscv: Fix warning by declaring arch_cpu_idle() as noinstr
>   riscv: Disable preemption when using patch_map()
> 
> [...]

Here is the summary with links:
  - [1/2] riscv: Fix warning by declaring arch_cpu_idle() as noinstr
    https://git.kernel.org/riscv/c/8a48ea87ce89
  - [2/2] riscv: Disable preemption when using patch_map()
    https://git.kernel.org/riscv/c/a370c2419e46

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 0/2] ftrace fixes

[Puranjay Mohan]

Alexandre Ghiti <alexghiti@rivosinc.com> writes:

> Both were reported recently as there are efforts ongoing to
> reimplement ftrace on riscv and both are independent of this rework,
> so here they are.
>
> Alexandre Ghiti (2):
>   riscv: Fix warning by declaring arch_cpu_idle() as noinstr
>   riscv: Disable preemption when using patch_map()
>

Thanks for this.

Acked-by: Puranjay Mohan <puranjay12@gmail.com>

for both patches.

Puranjay

Re: [PATCH 0/2] ftrace fixes

[Puranjay Mohan]

Alexandre Ghiti <alexghiti@rivosinc.com> writes:

> Both were reported recently as there are efforts ongoing to
> reimplement ftrace on riscv and both are independent of this rework,
> so here they are.
>
> Alexandre Ghiti (2):
>   riscv: Fix warning by declaring arch_cpu_idle() as noinstr
>   riscv: Disable preemption when using patch_map()
>

Thanks for this.

Acked-by: Puranjay Mohan <puranjay12@gmail.com>

for both patches.

Puranjay

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH 0/2] ftrace fixes

[Alexandre Ghiti]

Both were reported recently as there are efforts ongoing to
reimplement ftrace on riscv and both are independent of this rework,
so here they are.

Alexandre Ghiti (2):
  riscv: Fix warning by declaring arch_cpu_idle() as noinstr
  riscv: Disable preemption when using patch_map()

 arch/riscv/kernel/patch.c   | 8 ++++++++
 arch/riscv/kernel/process.c | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

-- 
2.39.2

[PATCH 0/2] ftrace fixes

[Alexandre Ghiti]

Both were reported recently as there are efforts ongoing to
reimplement ftrace on riscv and both are independent of this rework,
so here they are.

Alexandre Ghiti (2):
  riscv: Fix warning by declaring arch_cpu_idle() as noinstr
  riscv: Disable preemption when using patch_map()

 arch/riscv/kernel/patch.c   | 8 ++++++++
 arch/riscv/kernel/process.c | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

-- 
2.39.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv