linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ARM: ftrace: pause/unpause function graph tracer in cpu_suspend()
@ 2021-01-04  5:35 Louis Wang
  2021-02-24 10:35 ` [PATCH] [RFC]ARM: " liang wang
  0 siblings, 1 reply; 6+ messages in thread
From: Louis Wang @ 2021-01-04  5:35 UTC (permalink / raw)
  To: linux
  Cc: liang26812, linux-kernel, penberg, geert, rppt, akpm, linux-arm-kernel

From: "louis.wang1" <louis.wang1@unisoc.com>

Enabling function_graph tracer on ARM causes kernel panic, because the
function graph tracer updates the "return address" of a function in order
to insert a trace callback on function exit, it saves the function's
original return address in a return trace stack, but cpu_suspend() may not
return through the normal return path.

cpu_suspend() will resume directly via the cpu_resume path, but the return
trace stack has been set-up by the subfunctions of cpu_suspend(), which
makes the "return address" inconsistent with cpu_suspend().

This patch refers to Commit de818bd4522c40ea02a81b387d2fa86f989c9623
("arm64: kernel: pause/unpause function graph tracer in cpu_suspend()"),
fixes the issue by pausing/resuming the function graph tracer on the thread
executing cpu_suspend(), so that the function graph tracer state is kept
consistent across functions that enter power down states and never return
by effectively disabling graph tracer while they are executing.

Signed-off-by: louis.wang1 <louis.wang1@unisoc.com>
---
 arch/arm/kernel/suspend.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/suspend.c b/arch/arm/kernel/suspend.c
index 24bd205..43f0a3e 100644
--- a/arch/arm/kernel/suspend.c
+++ b/arch/arm/kernel/suspend.c
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0
+#include <linux/ftrace.h>
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/mm_types.h>
@@ -26,12 +27,22 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
 		return -EINVAL;
 
 	/*
+	 * Function graph tracer state gets incosistent when the kernel
+	 * calls functions that never return (aka suspend finishers) hence
+	 * disable graph tracing during their execution.
+	 */
+	pause_graph_tracing();
+
+	/*
 	 * Provide a temporary page table with an identity mapping for
 	 * the MMU-enable code, required for resuming.  On successful
 	 * resume (indicated by a zero return code), we need to switch
 	 * back to the correct page tables.
 	 */
 	ret = __cpu_suspend(arg, fn, __mpidr);
+
+	unpause_graph_tracing();
+
 	if (ret == 0) {
 		cpu_switch_mm(mm->pgd, mm);
 		local_flush_bp_all();
@@ -45,7 +56,13 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
 int cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
 {
 	u32 __mpidr = cpu_logical_map(smp_processor_id());
-	return __cpu_suspend(arg, fn, __mpidr);
+	int ret;
+
+	pause_graph_tracing();
+	ret = __cpu_suspend(arg, fn, __mpidr);
+	unpause_graph_tracing();
+
+	return ret;
 }
 #define	idmap_pgd	NULL
 #endif
-- 
2.7.4


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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] [RFC]ARM: ftrace: pause/unpause function graph tracer in cpu_suspend()
  2021-01-04  5:35 [PATCH] ARM: ftrace: pause/unpause function graph tracer in cpu_suspend() Louis Wang
@ 2021-02-24 10:35 ` liang wang
  2021-02-24 10:39   ` Russell King - ARM Linux admin
  0 siblings, 1 reply; 6+ messages in thread
From: liang wang @ 2021-02-24 10:35 UTC (permalink / raw)
  To: linux; +Cc: linux-kernel, penberg, geert, rppt, akpm, linux-arm-kernel

Hi,all

On Mon, 4 Jan 2021 at 13:35, Louis Wang <liang26812@gmail.com> wrote:
>
> From: "louis.wang1" <louis.wang1@unisoc.com>
>
> Enabling function_graph tracer on ARM causes kernel panic, because the
> function graph tracer updates the "return address" of a function in order
> to insert a trace callback on function exit, it saves the function's
> original return address in a return trace stack, but cpu_suspend() may not
> return through the normal return path.
>
> cpu_suspend() will resume directly via the cpu_resume path, but the return
> trace stack has been set-up by the subfunctions of cpu_suspend(), which
> makes the "return address" inconsistent with cpu_suspend().
>
> This patch refers to Commit de818bd4522c40ea02a81b387d2fa86f989c9623
> ("arm64: kernel: pause/unpause function graph tracer in cpu_suspend()"),
> fixes the issue by pausing/resuming the function graph tracer on the thread
> executing cpu_suspend(), so that the function graph tracer state is kept
> consistent across functions that enter power down states and never return
> by effectively disabling graph tracer while they are executing.
>
> Signed-off-by: louis.wang1 <louis.wang1@unisoc.com>
> ---
>  arch/arm/kernel/suspend.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/kernel/suspend.c b/arch/arm/kernel/suspend.c
> index 24bd205..43f0a3e 100644
> --- a/arch/arm/kernel/suspend.c
> +++ b/arch/arm/kernel/suspend.c
> @@ -1,4 +1,5 @@
>  // SPDX-License-Identifier: GPL-2.0
> +#include <linux/ftrace.h>
>  #include <linux/init.h>
>  #include <linux/slab.h>
>  #include <linux/mm_types.h>
> @@ -26,12 +27,22 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
>                 return -EINVAL;
>
>         /*
> +        * Function graph tracer state gets incosistent when the kernel
> +        * calls functions that never return (aka suspend finishers) hence
> +        * disable graph tracing during their execution.
> +        */
> +       pause_graph_tracing();
> +
> +       /*
>          * Provide a temporary page table with an identity mapping for
>          * the MMU-enable code, required for resuming.  On successful
>          * resume (indicated by a zero return code), we need to switch
>          * back to the correct page tables.
>          */
>         ret = __cpu_suspend(arg, fn, __mpidr);
> +
> +       unpause_graph_tracing();
> +
>         if (ret == 0) {
>                 cpu_switch_mm(mm->pgd, mm);
>                 local_flush_bp_all();
> @@ -45,7 +56,13 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
>  int cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
>  {
>         u32 __mpidr = cpu_logical_map(smp_processor_id());
> -       return __cpu_suspend(arg, fn, __mpidr);
> +       int ret;
> +
> +       pause_graph_tracing();
> +       ret = __cpu_suspend(arg, fn, __mpidr);
> +       unpause_graph_tracing();
> +
> +       return ret;
>  }
>  #define        idmap_pgd       NULL
>  #endif
> --
> 2.7.4
>
ftrace function_graph tracer always cause kernel panic on my ARM device with
multiple CPUs, I found a solution for the problem on ARM64, refers to
the patch above,
I was wondering why this bugfix on ARM64 hasn't been upstreamed to ARM,
Does anyone have a similar problem and can share information with me?
Thanks.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] [RFC]ARM: ftrace: pause/unpause function graph tracer in cpu_suspend()
  2021-02-24 10:35 ` [PATCH] [RFC]ARM: " liang wang
@ 2021-02-24 10:39   ` Russell King - ARM Linux admin
  2021-02-24 11:14     ` liang wang
  0 siblings, 1 reply; 6+ messages in thread
From: Russell King - ARM Linux admin @ 2021-02-24 10:39 UTC (permalink / raw)
  To: liang wang; +Cc: linux-kernel, penberg, geert, rppt, akpm, linux-arm-kernel

On Wed, Feb 24, 2021 at 06:35:47PM +0800, liang wang wrote:
> Hi,all
> 
> ftrace function_graph tracer always cause kernel panic on my ARM device with
> multiple CPUs, I found a solution for the problem on ARM64, refers to
> the patch above,
> I was wondering why this bugfix on ARM64 hasn't been upstreamed to ARM,

Patches get applied to the ARM tree after they've been submitted to
the patch system. If they don't get submitted to the patch system,
then they get buried and forgotten.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] [RFC]ARM: ftrace: pause/unpause function graph tracer in cpu_suspend()
  2021-02-24 10:39   ` Russell King - ARM Linux admin
@ 2021-02-24 11:14     ` liang wang
  2021-02-24 11:20       ` Russell King - ARM Linux admin
  0 siblings, 1 reply; 6+ messages in thread
From: liang wang @ 2021-02-24 11:14 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: linux-kernel, penberg, geert, rppt, akpm, linux-arm-kernel

Hi Russell,

On Wed, 24 Feb 2021 at 18:39, Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Wed, Feb 24, 2021 at 06:35:47PM +0800, liang wang wrote:
> > Hi,all
> >
> > ftrace function_graph tracer always cause kernel panic on my ARM device with
> > multiple CPUs, I found a solution for the problem on ARM64, refers to
> > the patch above,
> > I was wondering why this bugfix on ARM64 hasn't been upstreamed to ARM,
>
> Patches get applied to the ARM tree after they've been submitted to
> the patch system. If they don't get submitted to the patch system,
> then they get buried and forgotten.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Thanks for replying,
So I refers to the solution on ARM64 and sent my patch on January as below.

http://lists.infradead.org/pipermail/linux-arm-kernel/2021-January/628460.html

But I haven't got any replies on my patch. How can I get my patch reviewed
and submitted to the patch system?Should I resend a new patch to this maillist?
Thanks!

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] [RFC]ARM: ftrace: pause/unpause function graph tracer in cpu_suspend()
  2021-02-24 11:14     ` liang wang
@ 2021-02-24 11:20       ` Russell King - ARM Linux admin
  2021-02-24 11:36         ` liang wang
  0 siblings, 1 reply; 6+ messages in thread
From: Russell King - ARM Linux admin @ 2021-02-24 11:20 UTC (permalink / raw)
  To: liang wang; +Cc: linux-kernel, penberg, geert, linux-arm-kernel, akpm, rppt

On Wed, Feb 24, 2021 at 07:14:54PM +0800, liang wang wrote:
> Hi Russell,
> 
> On Wed, 24 Feb 2021 at 18:39, Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > On Wed, Feb 24, 2021 at 06:35:47PM +0800, liang wang wrote:
> > > Hi,all
> > >
> > > ftrace function_graph tracer always cause kernel panic on my ARM device with
> > > multiple CPUs, I found a solution for the problem on ARM64, refers to
> > > the patch above,
> > > I was wondering why this bugfix on ARM64 hasn't been upstreamed to ARM,
> >
> > Patches get applied to the ARM tree after they've been submitted to
> > the patch system. If they don't get submitted to the patch system,
> > then they get buried and forgotten.
> >
> > --
> > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
> 
> Thanks for replying,
> So I refers to the solution on ARM64 and sent my patch on January as below.
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2021-January/628460.html
> 
> But I haven't got any replies on my patch. How can I get my patch reviewed
> and submitted to the patch system?

I don't use ftrace, so I'm not the right person to review it - but it
seems that lots of patches for ARM are left to me to try and review,
despite me not knowing sufficient to know whether it's correct or not.
This isn't sustainable.

> Should I resend a new patch to this maillist?

You can try but I suspect the same thing will happen. The only thing I
can think is if you put it in the patch system and I apply it, if
someone has a problem with it, they'll shout about it soon enough. Not
the best way, but I don't see much other option.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] [RFC]ARM: ftrace: pause/unpause function graph tracer in cpu_suspend()
  2021-02-24 11:20       ` Russell King - ARM Linux admin
@ 2021-02-24 11:36         ` liang wang
  0 siblings, 0 replies; 6+ messages in thread
From: liang wang @ 2021-02-24 11:36 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: linux-kernel, penberg, geert, linux-arm-kernel, akpm, rppt

Hi Russell

On Wed, 24 Feb 2021 at 19:20, Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Wed, Feb 24, 2021 at 07:14:54PM +0800, liang wang wrote:
> > Hi Russell,
> >
> > On Wed, 24 Feb 2021 at 18:39, Russell King - ARM Linux admin
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Wed, Feb 24, 2021 at 06:35:47PM +0800, liang wang wrote:
> > > > Hi,all
> > > >
> > > > ftrace function_graph tracer always cause kernel panic on my ARM device with
> > > > multiple CPUs, I found a solution for the problem on ARM64, refers to
> > > > the patch above,
> > > > I was wondering why this bugfix on ARM64 hasn't been upstreamed to ARM,
> > >
> > > Patches get applied to the ARM tree after they've been submitted to
> > > the patch system. If they don't get submitted to the patch system,
> > > then they get buried and forgotten.
> > >
> > > --
> > > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
> >
> > Thanks for replying,
> > So I refers to the solution on ARM64 and sent my patch on January as below.
> >
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2021-January/628460.html
> >
> > But I haven't got any replies on my patch. How can I get my patch reviewed
> > and submitted to the patch system?
>
> I don't use ftrace, so I'm not the right person to review it - but it
> seems that lots of patches for ARM are left to me to try and review,
> despite me not knowing sufficient to know whether it's correct or not.
> This isn't sustainable.
>
> > Should I resend a new patch to this maillist?
>
> You can try but I suspect the same thing will happen. The only thing I
> can think is if you put it in the patch system and I apply it, if
> someone has a problem with it, they'll shout about it soon enough. Not
> the best way, but I don't see much other option.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Thanks for answering my question.
I will upload it to the patch system as needed, and see whether there
is a problem
or someone to discuss with.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-02-24 11:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-04  5:35 [PATCH] ARM: ftrace: pause/unpause function graph tracer in cpu_suspend() Louis Wang
2021-02-24 10:35 ` [PATCH] [RFC]ARM: " liang wang
2021-02-24 10:39   ` Russell King - ARM Linux admin
2021-02-24 11:14     ` liang wang
2021-02-24 11:20       ` Russell King - ARM Linux admin
2021-02-24 11:36         ` liang wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).