[PATCH] PM: Fix crash on graph trace through x86 suspend

[PATCH] PM: Fix crash on graph trace through x86 suspend

[Todd Brandt]

Pause/unpause graph tracing around do_suspend_lowlevel as it has
inconsistent call/return info after it jumps to the wakeup vector.
The graph trace buffer will otherwise become misaligned and
may eventually crash and hang on suspend.

To reproduce the issue and test the fix:
Run a function_graph trace over suspend/resume and set the graph 
function to suspend_devices_and_enter. This consistently hangs the
system without this fix.

Signed-off-by: Todd Brandt <todd.e.brandt@linux.intel.com>
---
 arch/x86/kernel/acpi/sleep.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
index d1daead..311360e 100644
--- a/arch/x86/kernel/acpi/sleep.c
+++ b/arch/x86/kernel/acpi/sleep.c
@@ -16,6 +16,7 @@
 #include <asm/cacheflush.h>
 #include <asm/realmode.h>
 
+#include <linux/ftrace.h>
 #include "../../realmode/rm/wakeup.h"
 #include "sleep.h"
 
@@ -107,7 +108,13 @@ int x86_acpi_suspend_lowlevel(void)
        saved_magic = 0x123456789abcdef0L;
 #endif /* CONFIG_64BIT */
 
+	/*
+	 * Pause/unpause graph tracing around do_suspend_lowlevel as it has
+	 * inconsistent call/return info after it jumps to the wakeup vector
+	 */
+	pause_graph_tracing();
 	do_suspend_lowlevel();
+	unpause_graph_tracing();
 	return 0;
 }
 
-- 
2.1.4

Re: [PATCH] PM: Fix crash on graph trace through x86 suspend

[Rafael J. Wysocki]

On Thu, Mar 3, 2016 at 1:05 AM, Todd Brandt
<todd.e.brandt@linux.intel.com> wrote:
> Pause/unpause graph tracing around do_suspend_lowlevel as it has
> inconsistent call/return info after it jumps to the wakeup vector.
> The graph trace buffer will otherwise become misaligned and
> may eventually crash and hang on suspend.
>
> To reproduce the issue and test the fix:
> Run a function_graph trace over suspend/resume and set the graph
> function to suspend_devices_and_enter. This consistently hangs the
> system without this fix.
>
> Signed-off-by: Todd Brandt <todd.e.brandt@linux.intel.com>

OK

I guess we'll need that in -stable, right?

> ---
>  arch/x86/kernel/acpi/sleep.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
> index d1daead..311360e 100644
> --- a/arch/x86/kernel/acpi/sleep.c
> +++ b/arch/x86/kernel/acpi/sleep.c
> @@ -16,6 +16,7 @@
>  #include <asm/cacheflush.h>
>  #include <asm/realmode.h>
>
> +#include <linux/ftrace.h>
>  #include "../../realmode/rm/wakeup.h"
>  #include "sleep.h"
>
> @@ -107,7 +108,13 @@ int x86_acpi_suspend_lowlevel(void)
>         saved_magic = 0x123456789abcdef0L;
>  #endif /* CONFIG_64BIT */
>
> +       /*
> +        * Pause/unpause graph tracing around do_suspend_lowlevel as it has
> +        * inconsistent call/return info after it jumps to the wakeup vector
> +        */
> +       pause_graph_tracing();
>         do_suspend_lowlevel();
> +       unpause_graph_tracing();
>         return 0;
>  }
>
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] PM: Fix crash on graph trace through x86 suspend

[Todd Brandt]

On Thu, 2016-03-03 at 01:19 +0100, Rafael J. Wysocki wrote:
> On Thu, Mar 3, 2016 at 1:05 AM, Todd Brandt
> <todd.e.brandt@linux.intel.com> wrote:
> > Pause/unpause graph tracing around do_suspend_lowlevel as it has
> > inconsistent call/return info after it jumps to the wakeup vector.
> > The graph trace buffer will otherwise become misaligned and
> > may eventually crash and hang on suspend.
> >
> > To reproduce the issue and test the fix:
> > Run a function_graph trace over suspend/resume and set the graph
> > function to suspend_devices_and_enter. This consistently hangs the
> > system without this fix.
> >
> > Signed-off-by: Todd Brandt <todd.e.brandt@linux.intel.com>
> 
> OK
> 
> I guess we'll need that in -stable, right?

Sure, it's pretty important. The new analyze_suspend features are going
to rely pretty heavily on full graph trace of suspend (all we can do is
standby and freeze now because of the bug). Thanks!

> 
> > ---
> >  arch/x86/kernel/acpi/sleep.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
> > index d1daead..311360e 100644
> > --- a/arch/x86/kernel/acpi/sleep.c
> > +++ b/arch/x86/kernel/acpi/sleep.c
> > @@ -16,6 +16,7 @@
> >  #include <asm/cacheflush.h>
> >  #include <asm/realmode.h>
> >
> > +#include <linux/ftrace.h>
> >  #include "../../realmode/rm/wakeup.h"
> >  #include "sleep.h"
> >
> > @@ -107,7 +108,13 @@ int x86_acpi_suspend_lowlevel(void)
> >         saved_magic = 0x123456789abcdef0L;
> >  #endif /* CONFIG_64BIT */
> >
> > +       /*
> > +        * Pause/unpause graph tracing around do_suspend_lowlevel as it has
> > +        * inconsistent call/return info after it jumps to the wakeup vector
> > +        */
> > +       pause_graph_tracing();
> >         do_suspend_lowlevel();
> > +       unpause_graph_tracing();
> >         return 0;
> >  }
> >
> > --
> > 2.1.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] PM: Fix crash on graph trace through x86 suspend

[Rafael J. Wysocki]

On Thu, Mar 3, 2016 at 1:43 AM, Todd Brandt
<todd.e.brandt@linux.intel.com> wrote:
> On Thu, 2016-03-03 at 01:19 +0100, Rafael J. Wysocki wrote:
>> On Thu, Mar 3, 2016 at 1:05 AM, Todd Brandt
>> <todd.e.brandt@linux.intel.com> wrote:
>> > Pause/unpause graph tracing around do_suspend_lowlevel as it has
>> > inconsistent call/return info after it jumps to the wakeup vector.
>> > The graph trace buffer will otherwise become misaligned and
>> > may eventually crash and hang on suspend.
>> >
>> > To reproduce the issue and test the fix:
>> > Run a function_graph trace over suspend/resume and set the graph
>> > function to suspend_devices_and_enter. This consistently hangs the
>> > system without this fix.
>> >
>> > Signed-off-by: Todd Brandt <todd.e.brandt@linux.intel.com>
>>
>> OK
>>
>> I guess we'll need that in -stable, right?
>
> Sure, it's pretty important. The new analyze_suspend features are going
> to rely pretty heavily on full graph trace of suspend (all we can do is
> standby and freeze now because of the bug). Thanks!

OK, applied, but let Steven and the x86 folks see it.

>>
>> > ---
>> >  arch/x86/kernel/acpi/sleep.c | 7 +++++++
>> >  1 file changed, 7 insertions(+)
>> >
>> > diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
>> > index d1daead..311360e 100644
>> > --- a/arch/x86/kernel/acpi/sleep.c
>> > +++ b/arch/x86/kernel/acpi/sleep.c
>> > @@ -16,6 +16,7 @@
>> >  #include <asm/cacheflush.h>
>> >  #include <asm/realmode.h>
>> >
>> > +#include <linux/ftrace.h>
>> >  #include "../../realmode/rm/wakeup.h"
>> >  #include "sleep.h"
>> >
>> > @@ -107,7 +108,13 @@ int x86_acpi_suspend_lowlevel(void)
>> >         saved_magic = 0x123456789abcdef0L;
>> >  #endif /* CONFIG_64BIT */
>> >
>> > +       /*
>> > +        * Pause/unpause graph tracing around do_suspend_lowlevel as it has
>> > +        * inconsistent call/return info after it jumps to the wakeup vector
>> > +        */
>> > +       pause_graph_tracing();
>> >         do_suspend_lowlevel();
>> > +       unpause_graph_tracing();
>> >         return 0;
>> >  }
>> >
>> > --
>> > 2.1.4
>> >
>> > --


Thanks,
Rafael

Re: [PATCH] PM: Fix crash on graph trace through x86 suspend

[Rafael J. Wysocki]

Well, fix up the Steven's address.

On Thu, Mar 3, 2016 at 2:34 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Thu, Mar 3, 2016 at 1:43 AM, Todd Brandt
> <todd.e.brandt@linux.intel.com> wrote:
>> On Thu, 2016-03-03 at 01:19 +0100, Rafael J. Wysocki wrote:
>>> On Thu, Mar 3, 2016 at 1:05 AM, Todd Brandt
>>> <todd.e.brandt@linux.intel.com> wrote:
>>> > Pause/unpause graph tracing around do_suspend_lowlevel as it has
>>> > inconsistent call/return info after it jumps to the wakeup vector.
>>> > The graph trace buffer will otherwise become misaligned and
>>> > may eventually crash and hang on suspend.
>>> >
>>> > To reproduce the issue and test the fix:
>>> > Run a function_graph trace over suspend/resume and set the graph
>>> > function to suspend_devices_and_enter. This consistently hangs the
>>> > system without this fix.
>>> >
>>> > Signed-off-by: Todd Brandt <todd.e.brandt@linux.intel.com>
>>>
>>> OK
>>>
>>> I guess we'll need that in -stable, right?
>>
>> Sure, it's pretty important. The new analyze_suspend features are going
>> to rely pretty heavily on full graph trace of suspend (all we can do is
>> standby and freeze now because of the bug). Thanks!
>
> OK, applied, but let Steven and the x86 folks see it.
>
>>>
>>> > ---
>>> >  arch/x86/kernel/acpi/sleep.c | 7 +++++++
>>> >  1 file changed, 7 insertions(+)
>>> >
>>> > diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
>>> > index d1daead..311360e 100644
>>> > --- a/arch/x86/kernel/acpi/sleep.c
>>> > +++ b/arch/x86/kernel/acpi/sleep.c
>>> > @@ -16,6 +16,7 @@
>>> >  #include <asm/cacheflush.h>
>>> >  #include <asm/realmode.h>
>>> >
>>> > +#include <linux/ftrace.h>
>>> >  #include "../../realmode/rm/wakeup.h"
>>> >  #include "sleep.h"
>>> >
>>> > @@ -107,7 +108,13 @@ int x86_acpi_suspend_lowlevel(void)
>>> >         saved_magic = 0x123456789abcdef0L;
>>> >  #endif /* CONFIG_64BIT */
>>> >
>>> > +       /*
>>> > +        * Pause/unpause graph tracing around do_suspend_lowlevel as it has
>>> > +        * inconsistent call/return info after it jumps to the wakeup vector
>>> > +        */
>>> > +       pause_graph_tracing();
>>> >         do_suspend_lowlevel();
>>> > +       unpause_graph_tracing();
>>> >         return 0;
>>> >  }
>>> >
>>> > --
>>> > 2.1.4
>>> >
>>> > --
>
>
> Thanks,
> Rafael

Re: [PATCH] PM: Fix crash on graph trace through x86 suspend

[Steven Rostedt]

On Thu, 3 Mar 2016 02:36:26 +0100
"Rafael J. Wysocki" <rafael@kernel.org> wrote:

> > OK, applied, but let Steven and the x86 folks see it.

The below patch looks fine to me. Anything else I should look at?

-- Steve

> >  
> >>>  
> >>> > ---
> >>> >  arch/x86/kernel/acpi/sleep.c | 7 +++++++
> >>> >  1 file changed, 7 insertions(+)
> >>> >
> >>> > diff --git a/arch/x86/kernel/acpi/sleep.c b/arch/x86/kernel/acpi/sleep.c
> >>> > index d1daead..311360e 100644
> >>> > --- a/arch/x86/kernel/acpi/sleep.c
> >>> > +++ b/arch/x86/kernel/acpi/sleep.c
> >>> > @@ -16,6 +16,7 @@
> >>> >  #include <asm/cacheflush.h>
> >>> >  #include <asm/realmode.h>
> >>> >
> >>> > +#include <linux/ftrace.h>
> >>> >  #include "../../realmode/rm/wakeup.h"
> >>> >  #include "sleep.h"
> >>> >
> >>> > @@ -107,7 +108,13 @@ int x86_acpi_suspend_lowlevel(void)
> >>> >         saved_magic = 0x123456789abcdef0L;
> >>> >  #endif /* CONFIG_64BIT */
> >>> >
> >>> > +       /*
> >>> > +        * Pause/unpause graph tracing around do_suspend_lowlevel as it has
> >>> > +        * inconsistent call/return info after it jumps to the wakeup vector
> >>> > +        */
> >>> > +       pause_graph_tracing();
> >>> >         do_suspend_lowlevel();
> >>> > +       unpause_graph_tracing();
> >>> >         return 0;
> >>> >  }
> >>> >
> >>> > --
> >>> > 2.1.4
> >>> >
> >>> > --  
> >
> >
> > Thanks,
> > Rafael  

Re: [PATCH] PM: Fix crash on graph trace through x86 suspend

[Rafael J. Wysocki]

On Thu, Mar 3, 2016 at 3:07 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Thu, 3 Mar 2016 02:36:26 +0100
> "Rafael J. Wysocki" <rafael@kernel.org> wrote:
>
>> > OK, applied, but let Steven and the x86 folks see it.
>
> The below patch looks fine to me. Anything else I should look at?

No, that was it, thanks!