In latest 2.6.27(git) enabling dynamic ftrace makes resume from a suspend
to ram reboot instead of resuming. Queued for 2.6.28 is a new method of
recording mcount callers at compile time that does not have this issue.
But the new method is still too "green" to be pulled into 27, so the old
ftraced (daemon method) needs to be fixed for 27.
The way dynamic ftrace works with the daemon method is this. On boot up
the mcount function simply returns. When ftrace is initialized, it calls
kstop_machine to modify the mcount function to call another function
called "ftrace_record_ip". This new function will record in a preallocated
hash (allocated by the ftrace initializer) all the callers of mcount. A
check is made to see if the caller has already been put into the hash, and
if so, it is not recorded again.
Later on a kernel thread ftraced is created. This kernel thread wakes up
once a second and checks to see if any new functions were added to the
hash. If so, it then calls kstop_machine and modifies those callers to
mcount into nops.
Again, this daemon method makes resume from suspend to ram reboot instead
of resuming. Now, I'm asking the s2r gurus, what did I miss? Do I need to
add a "NO_FREEZE" flag or something to the "ftraced" kernel thread?
Just asking for some advice.
Thanks,
-- Steve
On Thursday, 21 of August 2008, Steven Rostedt wrote:
>
> In latest 2.6.27(git) enabling dynamic ftrace makes resume from a suspend
> to ram reboot instead of resuming. Queued for 2.6.28 is a new method of
> recording mcount callers at compile time that does not have this issue.
>
> But the new method is still too "green" to be pulled into 27, so the old
> ftraced (daemon method) needs to be fixed for 27.
>
> The way dynamic ftrace works with the daemon method is this. On boot up
> the mcount function simply returns. When ftrace is initialized, it calls
> kstop_machine to modify the mcount function to call another function
> called "ftrace_record_ip". This new function will record in a preallocated
> hash (allocated by the ftrace initializer) all the callers of mcount. A
> check is made to see if the caller has already been put into the hash, and
> if so, it is not recorded again.
>
> Later on a kernel thread ftraced is created. This kernel thread wakes up
> once a second and checks to see if any new functions were added to the
> hash. If so, it then calls kstop_machine and modifies those callers to
> mcount into nops.
>
> Again, this daemon method makes resume from suspend to ram reboot instead
> of resuming. Now, I'm asking the s2r gurus, what did I miss? Do I need to
> add a "NO_FREEZE" flag or something to the "ftraced" kernel thread?
>
> Just asking for some advice.
If I'm not mistaken, it'll probably suffice to make it freezable, so that it
doesn't run while the system is suspending and resuming. Would that be
acceptable?
Please tell me where exactly the ftraced source code is located.
Thanks,
Rafael
On Thu, 21 Aug 2008, Rafael J. Wysocki wrote:
> On Thursday, 21 of August 2008, Steven Rostedt wrote:
> >
> > In latest 2.6.27(git) enabling dynamic ftrace makes resume from a suspend
> > to ram reboot instead of resuming. Queued for 2.6.28 is a new method of
> > recording mcount callers at compile time that does not have this issue.
> >
> > But the new method is still too "green" to be pulled into 27, so the old
> > ftraced (daemon method) needs to be fixed for 27.
> >
> > The way dynamic ftrace works with the daemon method is this. On boot up
> > the mcount function simply returns. When ftrace is initialized, it calls
> > kstop_machine to modify the mcount function to call another function
> > called "ftrace_record_ip". This new function will record in a preallocated
> > hash (allocated by the ftrace initializer) all the callers of mcount. A
> > check is made to see if the caller has already been put into the hash, and
> > if so, it is not recorded again.
> >
> > Later on a kernel thread ftraced is created. This kernel thread wakes up
> > once a second and checks to see if any new functions were added to the
> > hash. If so, it then calls kstop_machine and modifies those callers to
> > mcount into nops.
> >
> > Again, this daemon method makes resume from suspend to ram reboot instead
> > of resuming. Now, I'm asking the s2r gurus, what did I miss? Do I need to
> > add a "NO_FREEZE" flag or something to the "ftraced" kernel thread?
> >
> > Just asking for some advice.
>
> If I'm not mistaken, it'll probably suffice to make it freezable, so that it
> doesn't run while the system is suspending and resuming. Would that be
> acceptable?
Does it not freeze by default.
>
> Please tell me where exactly the ftraced source code is located.
It's in Linus's latest git tree.
The code in question is the ftraced() function in kernel/trace/ftrace.c
Thanks,
-- Steve
On Thursday, 21 of August 2008, Steven Rostedt wrote:
>
> On Thu, 21 Aug 2008, Rafael J. Wysocki wrote:
>
> > On Thursday, 21 of August 2008, Steven Rostedt wrote:
> > >
> > > In latest 2.6.27(git) enabling dynamic ftrace makes resume from a suspend
> > > to ram reboot instead of resuming. Queued for 2.6.28 is a new method of
> > > recording mcount callers at compile time that does not have this issue.
> > >
> > > But the new method is still too "green" to be pulled into 27, so the old
> > > ftraced (daemon method) needs to be fixed for 27.
> > >
> > > The way dynamic ftrace works with the daemon method is this. On boot up
> > > the mcount function simply returns. When ftrace is initialized, it calls
> > > kstop_machine to modify the mcount function to call another function
> > > called "ftrace_record_ip". This new function will record in a preallocated
> > > hash (allocated by the ftrace initializer) all the callers of mcount. A
> > > check is made to see if the caller has already been put into the hash, and
> > > if so, it is not recorded again.
> > >
> > > Later on a kernel thread ftraced is created. This kernel thread wakes up
> > > once a second and checks to see if any new functions were added to the
> > > hash. If so, it then calls kstop_machine and modifies those callers to
> > > mcount into nops.
> > >
> > > Again, this daemon method makes resume from suspend to ram reboot instead
> > > of resuming. Now, I'm asking the s2r gurus, what did I miss? Do I need to
> > > add a "NO_FREEZE" flag or something to the "ftraced" kernel thread?
> > >
> > > Just asking for some advice.
> >
> > If I'm not mistaken, it'll probably suffice to make it freezable, so that it
> > doesn't run while the system is suspending and resuming. Would that be
> > acceptable?
>
> Does it not freeze by default.
No, it doesn't. Kernel threads are not freezable by default
> > Please tell me where exactly the ftraced source code is located.
>
> It's in Linus's latest git tree.
>
> The code in question is the ftraced() function in kernel/trace/ftrace.c
Thanks, I'll have a look in a while.
Rafael
On Thursday, 21 of August 2008, Rafael J. Wysocki wrote:
> On Thursday, 21 of August 2008, Steven Rostedt wrote:
> >
> > On Thu, 21 Aug 2008, Rafael J. Wysocki wrote:
> >
> > > On Thursday, 21 of August 2008, Steven Rostedt wrote:
> > > >
> > > > In latest 2.6.27(git) enabling dynamic ftrace makes resume from a suspend
> > > > to ram reboot instead of resuming. Queued for 2.6.28 is a new method of
> > > > recording mcount callers at compile time that does not have this issue.
> > > >
> > > > But the new method is still too "green" to be pulled into 27, so the old
> > > > ftraced (daemon method) needs to be fixed for 27.
> > > >
> > > > The way dynamic ftrace works with the daemon method is this. On boot up
> > > > the mcount function simply returns. When ftrace is initialized, it calls
> > > > kstop_machine to modify the mcount function to call another function
> > > > called "ftrace_record_ip". This new function will record in a preallocated
> > > > hash (allocated by the ftrace initializer) all the callers of mcount. A
> > > > check is made to see if the caller has already been put into the hash, and
> > > > if so, it is not recorded again.
> > > >
> > > > Later on a kernel thread ftraced is created. This kernel thread wakes up
> > > > once a second and checks to see if any new functions were added to the
> > > > hash. If so, it then calls kstop_machine and modifies those callers to
> > > > mcount into nops.
> > > >
> > > > Again, this daemon method makes resume from suspend to ram reboot instead
> > > > of resuming. Now, I'm asking the s2r gurus, what did I miss? Do I need to
> > > > add a "NO_FREEZE" flag or something to the "ftraced" kernel thread?
> > > >
> > > > Just asking for some advice.
> > >
> > > If I'm not mistaken, it'll probably suffice to make it freezable, so that it
> > > doesn't run while the system is suspending and resuming. Would that be
> > > acceptable?
> >
> > Does it not freeze by default.
>
> No, it doesn't. Kernel threads are not freezable by default
>
> > > Please tell me where exactly the ftraced source code is located.
> >
> > It's in Linus's latest git tree.
> >
> > The code in question is the ftraced() function in kernel/trace/ftrace.c
>
> Thanks, I'll have a look in a while.
Can you try the appended patch, please?
Thanks,
Rafael
---
kernel/trace/ftrace.c | 5 +++++
1 file changed, 5 insertions(+)
Index: linux-2.6/kernel/trace/ftrace.c
===================================================================
--- linux-2.6.orig/kernel/trace/ftrace.c
+++ linux-2.6/kernel/trace/ftrace.c
@@ -796,8 +796,13 @@ static int ftraced(void *ignore)
{
unsigned long usecs;
+ set_freezable();
+
while (!kthread_should_stop()) {
+ if (try_to_freeze())
+ continue;
+
set_current_state(TASK_INTERRUPTIBLE);
/* check once a second */
* Rafael J. Wysocki <[email protected]> wrote:
> > > The code in question is the ftraced() function in
> > > kernel/trace/ftrace.c
> >
> > Thanks, I'll have a look in a while.
>
> Can you try the appended patch, please?
makes sense - i've applied it to tip/tracing/urgent, see the tidied up
commit below.
It should be no big issue not being able to trace across suspend+resume
- and that restriction will go away with Steve's build-time based mcount
patching mechanism in v2.6.28.
Ingo
------------->
>From 0e556695ddc8eebf6f6dd86bb0c4911b2b90c12a Mon Sep 17 00:00:00 2001
From: Rafael J. Wysocki <[email protected]>
Date: Thu, 21 Aug 2008 21:59:36 +0200
Subject: [PATCH] ftrace: fix ftraced and suspend to ram
Steven Rostedt observed:
> In latest 2.6.27(git) enabling dynamic ftrace makes resume from a suspend
> to ram reboot instead of resuming. Queued for 2.6.28 is a new method of
> recording mcount callers at compile time that does not have this issue.
>
> But the new method is still too "green" to be pulled into 27, so the old
> ftraced (daemon method) needs to be fixed for 27.
>
> The way dynamic ftrace works with the daemon method is this. On boot up
> the mcount function simply returns. When ftrace is initialized, it calls
> kstop_machine to modify the mcount function to call another function
> called "ftrace_record_ip". This new function will record in a preallocated
> hash (allocated by the ftrace initializer) all the callers of mcount. A
> check is made to see if the caller has already been put into the hash, and
> if so, it is not recorded again.
>
> Later on a kernel thread ftraced is created. This kernel thread wakes up
> once a second and checks to see if any new functions were added to the
> hash. If so, it then calls kstop_machine and modifies those callers to
> mcount into nops.
It will suffice to make it freezable, so that it doesn't run while the
system is suspending and resuming.
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/trace/ftrace.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 639e16c..49f4c3f 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -819,8 +819,13 @@ static int ftraced(void *ignore)
{
unsigned long usecs;
+ set_freezable();
+
while (!kthread_should_stop()) {
+ if (try_to_freeze())
+ continue;
+
set_current_state(TASK_INTERRUPTIBLE);
/* check once a second */
>
> * Rafael J. Wysocki <[email protected]> wrote:
>
> > > > The code in question is the ftraced() function in
> > > > kernel/trace/ftrace.c
> > >
> > > Thanks, I'll have a look in a while.
> >
> > Can you try the appended patch, please?
>
> makes sense - i've applied it to tip/tracing/urgent, see the tidied up
> commit below.
>
> It should be no big issue not being able to trace across suspend+resume
> - and that restriction will go away with Steve's build-time based mcount
> patching mechanism in v2.6.28.
Patch looks okay to me, but I'm not sure if another issue is not
hiding under it. Did anyone actually test ftrace + suspend after
applying this?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Friday, 22 of August 2008, Ingo Molnar wrote:
>
> * Rafael J. Wysocki <[email protected]> wrote:
>
> > > > The code in question is the ftraced() function in
> > > > kernel/trace/ftrace.c
> > >
> > > Thanks, I'll have a look in a while.
> >
> > Can you try the appended patch, please?
>
> makes sense - i've applied it to tip/tracing/urgent, see the tidied up
> commit below.
>
> It should be no big issue not being able to trace across suspend+resume
> - and that restriction will go away with Steve's build-time based mcount
> patching mechanism in v2.6.28.
>
> Ingo
>
> ------------->
> From 0e556695ddc8eebf6f6dd86bb0c4911b2b90c12a Mon Sep 17 00:00:00 2001
> From: Rafael J. Wysocki <[email protected]>
> Date: Thu, 21 Aug 2008 21:59:36 +0200
> Subject: [PATCH] ftrace: fix ftraced and suspend to ram
>
> Steven Rostedt observed:
>
> > In latest 2.6.27(git) enabling dynamic ftrace makes resume from a suspend
> > to ram reboot instead of resuming. Queued for 2.6.28 is a new method of
> > recording mcount callers at compile time that does not have this issue.
> >
> > But the new method is still too "green" to be pulled into 27, so the old
> > ftraced (daemon method) needs to be fixed for 27.
> >
> > The way dynamic ftrace works with the daemon method is this. On boot up
> > the mcount function simply returns. When ftrace is initialized, it calls
> > kstop_machine to modify the mcount function to call another function
> > called "ftrace_record_ip". This new function will record in a preallocated
> > hash (allocated by the ftrace initializer) all the callers of mcount. A
> > check is made to see if the caller has already been put into the hash, and
> > if so, it is not recorded again.
> >
> > Later on a kernel thread ftraced is created. This kernel thread wakes up
> > once a second and checks to see if any new functions were added to the
> > hash. If so, it then calls kstop_machine and modifies those callers to
> > mcount into nops.
>
> It will suffice to make it freezable, so that it doesn't run while the
> system is suspending and resuming.
>
> Signed-off-by: Ingo Molnar <[email protected]>
Well, you can add my sign-off too. ;-)
> ---
> kernel/trace/ftrace.c | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 639e16c..49f4c3f 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -819,8 +819,13 @@ static int ftraced(void *ignore)
> {
> unsigned long usecs;
>
> + set_freezable();
> +
> while (!kthread_should_stop()) {
>
> + if (try_to_freeze())
> + continue;
> +
> set_current_state(TASK_INTERRUPTIBLE);
>
> /* check once a second */
>
>
On Fri, Aug 22, 2008 at 09:23:43AM +0200, Pavel Machek wrote:
> >
> > * Rafael J. Wysocki <[email protected]> wrote:
> >
> > > > > The code in question is the ftraced() function in
> > > > > kernel/trace/ftrace.c
> > > >
> > > > Thanks, I'll have a look in a while.
> > >
> > > Can you try the appended patch, please?
> >
> > makes sense - i've applied it to tip/tracing/urgent, see the tidied up
> > commit below.
> >
> > It should be no big issue not being able to trace across suspend+resume
> > - and that restriction will go away with Steve's build-time based mcount
> > patching mechanism in v2.6.28.
>
> Patch looks okay to me, but I'm not sure if another issue is not
> hiding under it. Did anyone actually test ftrace + suspend after
> applying this?
I just tested this patch - it didn't help ;(
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 49f4c3f..02e41b2 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -27,6 +27,7 @@
#include <linux/ctype.h>
#include <linux/hash.h>
#include <linux/list.h>
+#include <linux/freezer.h>
#include <asm/ftrace.h>
On Friday, 22 of August 2008, Marcin Slusarz wrote:
> On Fri, Aug 22, 2008 at 09:23:43AM +0200, Pavel Machek wrote:
> > >
> > > * Rafael J. Wysocki <[email protected]> wrote:
> > >
> > > > > > The code in question is the ftraced() function in
> > > > > > kernel/trace/ftrace.c
> > > > >
> > > > > Thanks, I'll have a look in a while.
> > > >
> > > > Can you try the appended patch, please?
> > >
> > > makes sense - i've applied it to tip/tracing/urgent, see the tidied up
> > > commit below.
> > >
> > > It should be no big issue not being able to trace across suspend+resume
> > > - and that restriction will go away with Steve's build-time based mcount
> > > patching mechanism in v2.6.28.
> >
> > Patch looks okay to me, but I'm not sure if another issue is not
> > hiding under it. Did anyone actually test ftrace + suspend after
> > applying this?
>
> I just tested this patch - it didn't help ;(
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 49f4c3f..02e41b2 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -27,6 +27,7 @@
> #include <linux/ctype.h>
> #include <linux/hash.h>
> #include <linux/list.h>
> +#include <linux/freezer.h>
>
> #include <asm/ftrace.h>
This is needed to fix compilation, sorry for the omission.
Still, did you test ftrace + suspend with the original patch and your fix
applied and if you did, then what happend?
Rafael
On Fri 2008-08-22 12:35:39, Marcin Slusarz wrote:
> On Fri, Aug 22, 2008 at 09:23:43AM +0200, Pavel Machek wrote:
> > >
> > > * Rafael J. Wysocki <[email protected]> wrote:
> > >
> > > > > > The code in question is the ftraced() function in
> > > > > > kernel/trace/ftrace.c
> > > > >
> > > > > Thanks, I'll have a look in a while.
> > > >
> > > > Can you try the appended patch, please?
> > >
> > > makes sense - i've applied it to tip/tracing/urgent, see the tidied up
> > > commit below.
> > >
> > > It should be no big issue not being able to trace across suspend+resume
> > > - and that restriction will go away with Steve's build-time based mcount
> > > patching mechanism in v2.6.28.
> >
> > Patch looks okay to me, but I'm not sure if another issue is not
> > hiding under it. Did anyone actually test ftrace + suspend after
> > applying this?
>
> I just tested this patch - it didn't help ;(
Does ftrace hook itself onto _all_ the functions? Or all .c functions?
I guess low-level suspend code needs to be exempt from
tracing. Certainly all the assembly functions.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Fri, 22 Aug 2008, Pavel Machek wrote:
>
> Does ftrace hook itself onto _all_ the functions? Or all .c functions?
It hooks into all .c functions that are not annotated with "notrace"
or the files have not been marked in the Makefile like:
CFLAGS_REMOVE_<file>.o = -pg
>
> I guess low-level suspend code needs to be exempt from
> tracing. Certainly all the assembly functions.
I'm looking into that now too. Are the functions in arch/x86/power/cpu*.c
the suspend to ram code?
Thanks,
-- Steve
On Friday, 22 of August 2008, Steven Rostedt wrote:
>
> On Fri, 22 Aug 2008, Pavel Machek wrote:
> >
> > Does ftrace hook itself onto _all_ the functions? Or all .c functions?
>
> It hooks into all .c functions that are not annotated with "notrace"
> or the files have not been marked in the Makefile like:
>
> CFLAGS_REMOVE_<file>.o = -pg
>
>
> >
> > I guess low-level suspend code needs to be exempt from
> > tracing. Certainly all the assembly functions.
>
> I'm looking into that now too. Are the functions in arch/x86/power/cpu*.c
> the suspend to ram code?
They contain code executed during suspend to RAM, but such code is also:
- in all files under arch/x86/kernel/acpi/
- in main.c, console.c under kernel/power
- in all files under drivers/acpi/sleep
- in drivers/acpi/hardware/hwsleep.c
Generally, ACPI is heavily involved and I'm not the right person to ask which
of the ACPI functions should get the 'notrace' thing. Also, I'm not sure about
the device drivers' ->suspend() and ->resume() callbacks, especially for
sysdevs and ->suspend_late(), ->resume_early() for platform devices and PCI.
Well, how exactly suspend to RAM is broken by ftrace?
Rafael
On Fri, 22 Aug 2008, Rafael J. Wysocki wrote:
> > > tracing. Certainly all the assembly functions.
> >
> > I'm looking into that now too. Are the functions in arch/x86/power/cpu*.c
> > the suspend to ram code?
>
> They contain code executed during suspend to RAM, but such code is also:
> - in all files under arch/x86/kernel/acpi/
> - in main.c, console.c under kernel/power
> - in all files under drivers/acpi/sleep
> - in drivers/acpi/hardware/hwsleep.c
>
> Generally, ACPI is heavily involved and I'm not the right person to ask which
> of the ACPI functions should get the 'notrace' thing. Also, I'm not sure about
> the device drivers' ->suspend() and ->resume() callbacks, especially for
> sysdevs and ->suspend_late(), ->resume_early() for platform devices and PCI.
>
> Well, how exactly suspend to RAM is broken by ftrace?
>
I know that the smp_processor_id may be defined in the %fs register, but
if ftrace is called before the %fs is set up, it may crash because it
uses smp_processor_id.
-- Steve
On Fri, Aug 22, 2008 at 06:39:34PM +0200, Rafael J. Wysocki wrote:
> On Friday, 22 of August 2008, Marcin Slusarz wrote:
> > On Fri, Aug 22, 2008 at 09:23:43AM +0200, Pavel Machek wrote:
> > > >
> > > > * Rafael J. Wysocki <[email protected]> wrote:
> > > >
> > > > > > > The code in question is the ftraced() function in
> > > > > > > kernel/trace/ftrace.c
> > > > > >
> > > > > > Thanks, I'll have a look in a while.
> > > > >
> > > > > Can you try the appended patch, please?
> > > >
> > > > makes sense - i've applied it to tip/tracing/urgent, see the tidied up
> > > > commit below.
> > > >
> > > > It should be no big issue not being able to trace across suspend+resume
> > > > - and that restriction will go away with Steve's build-time based mcount
> > > > patching mechanism in v2.6.28.
> > >
> > > Patch looks okay to me, but I'm not sure if another issue is not
> > > hiding under it. Did anyone actually test ftrace + suspend after
> > > applying this?
> >
> > I just tested this patch - it didn't help ;(
> >
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index 49f4c3f..02e41b2 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c
> > @@ -27,6 +27,7 @@
> > #include <linux/ctype.h>
> > #include <linux/hash.h>
> > #include <linux/list.h>
> > +#include <linux/freezer.h>
> >
> > #include <asm/ftrace.h>
>
> This is needed to fix compilation, sorry for the omission.
>
> Still, did you test ftrace + suspend with the original patch and your fix
> applied and if you did, then what happend?
Suspend works fine. When I hit power button I see some informations from
video card (nvidia), then screen goes black and monitor says "no signal"
and then I see the same informations from video card, then BIOS and GRUB.
When everything works fine, X comes back after first infos from video card.
Marcin
On Friday, 22 of August 2008, Steven Rostedt wrote:
>
> On Fri, 22 Aug 2008, Rafael J. Wysocki wrote:
> > > > tracing. Certainly all the assembly functions.
> > >
> > > I'm looking into that now too. Are the functions in arch/x86/power/cpu*.c
> > > the suspend to ram code?
> >
> > They contain code executed during suspend to RAM, but such code is also:
> > - in all files under arch/x86/kernel/acpi/
> > - in main.c, console.c under kernel/power
> > - in all files under drivers/acpi/sleep
> > - in drivers/acpi/hardware/hwsleep.c
> >
> > Generally, ACPI is heavily involved and I'm not the right person to ask which
> > of the ACPI functions should get the 'notrace' thing. Also, I'm not sure about
> > the device drivers' ->suspend() and ->resume() callbacks, especially for
> > sysdevs and ->suspend_late(), ->resume_early() for platform devices and PCI.
> >
> > Well, how exactly suspend to RAM is broken by ftrace?
> >
>
> I know that the smp_processor_id may be defined in the %fs register, but
> if ftrace is called before the %fs is set up, it may crash because it
> uses smp_processor_id.
The wake-up code is the most interesting for you, then. It's located in
arch/x86/kernel/acpi/ , but on x86-64 it also uses trampoline code in
trampoline_64.S (this is assembly, though).
On x86-64, wakeup_long64 in wakeup_64.S is where we get from the real-mode
wake-up code, but under arch/x86/kernel/acpi/realmode there are some C files
containing functions executed in real mode. Everything in there and everything
referred to from there should be 'notrace' IMO.
Thanks,
Rafael
On Friday, 22 of August 2008, Marcin Slusarz wrote:
> On Fri, Aug 22, 2008 at 06:39:34PM +0200, Rafael J. Wysocki wrote:
> > On Friday, 22 of August 2008, Marcin Slusarz wrote:
> > > On Fri, Aug 22, 2008 at 09:23:43AM +0200, Pavel Machek wrote:
> > > > >
> > > > > * Rafael J. Wysocki <[email protected]> wrote:
> > > > >
> > > > > > > > The code in question is the ftraced() function in
> > > > > > > > kernel/trace/ftrace.c
> > > > > > >
> > > > > > > Thanks, I'll have a look in a while.
> > > > > >
> > > > > > Can you try the appended patch, please?
> > > > >
> > > > > makes sense - i've applied it to tip/tracing/urgent, see the tidied up
> > > > > commit below.
> > > > >
> > > > > It should be no big issue not being able to trace across suspend+resume
> > > > > - and that restriction will go away with Steve's build-time based mcount
> > > > > patching mechanism in v2.6.28.
> > > >
> > > > Patch looks okay to me, but I'm not sure if another issue is not
> > > > hiding under it. Did anyone actually test ftrace + suspend after
> > > > applying this?
> > >
> > > I just tested this patch - it didn't help ;(
> > >
> > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > > index 49f4c3f..02e41b2 100644
> > > --- a/kernel/trace/ftrace.c
> > > +++ b/kernel/trace/ftrace.c
> > > @@ -27,6 +27,7 @@
> > > #include <linux/ctype.h>
> > > #include <linux/hash.h>
> > > #include <linux/list.h>
> > > +#include <linux/freezer.h>
> > >
> > > #include <asm/ftrace.h>
> >
> > This is needed to fix compilation, sorry for the omission.
> >
> > Still, did you test ftrace + suspend with the original patch and your fix
> > applied and if you did, then what happend?
>
> Suspend works fine. When I hit power button I see some informations from
> video card (nvidia), then screen goes black and monitor says "no signal"
> and then I see the same informations from video card, then BIOS and GRUB.
> When everything works fine, X comes back after first infos from video card.
I suspect that something under arch/x86/kernel/acpi/realmode is broken
somehow.
Thanks,
Rafael
Rafael J. Wysocki <rjw <at> sisk.pl> writes:
>
> This is needed to fix compilation, sorry for the omission.
>
> Still, did you test ftrace + suspend with the original patch and your fix
> applied and if you did, then what happend?
>
> Rafael
>
On my x86_64 laptop, both with and without the patch, the laptop never resumes,
nor does it reboot. Just the power light comes on. I've tried the PM test rtc
thing, but the clock never gets changed. Disabling FTRACE causes suspend/resume
to work.
I've been painstakingly debugging the issue with suspend to ram and
ftraced. The 2.6.28 code does not have this issue, but since the mcount
recording is not going to be in 27, this must be solved for the ftrace
daemon version.
The resume from suspend to ram would reboot because it was triple
faulting. Debugging further, I found that calling the mcount function
itself was not an issue, but it would fault when it incremented
preempt_count. preempt_count is on the tasks info structure that is on the
low memory address of the task's stack. For some reason, it could not
write to it. Resuming out of suspend to ram does quite a lot of funny
tricks to get to work, so it is not surprising at all that simply doing a
preempt_disable() would cause a fault.
Thanks to Rafael for suggesting to add a "while (1);" to find the place in
resuming that is causing the fault. I would place the loop somewhere in
the code, compile and reboot and see if it would either reboot (hit the
fault) or simply hang (hit the loop). Doing this over and over again, I
narrowed it down that it was happening in enable_nonboot_cpus.
At this point, I found that it is easier to simply disable tracing around
the suspend code, instead of searching for the particular function that
can not handle doing a preempt_disable.
This patch disables the tracer as it suspends and reenables it on resume.
I tested this patch on my Laptop, and it can resume fine with the patch.
Signed-off-by: Steven Rostedt <[email protected]>
---
kernel/power/main.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
Index: linux-compile.git/kernel/power/main.c
===================================================================
--- linux-compile.git.orig/kernel/power/main.c 2008-08-27 08:53:11.000000000 -0400
+++ linux-compile.git/kernel/power/main.c 2008-08-27 08:53:58.000000000 -0400
@@ -21,6 +21,7 @@
#include <linux/freezer.h>
#include <linux/vmstat.h>
#include <linux/syscalls.h>
+#include <linux/ftrace.h>
#include "power.h"
@@ -310,7 +311,7 @@ static int suspend_enter(suspend_state_t
*/
int suspend_devices_and_enter(suspend_state_t state)
{
- int error;
+ int error, ftrace_save;
if (!suspend_ops)
return -ENOSYS;
@@ -321,6 +322,7 @@ int suspend_devices_and_enter(suspend_st
goto Close;
}
suspend_console();
+ ftrace_save = __ftrace_enabled_save();
suspend_test_start();
error = device_suspend(PMSG_SUSPEND);
if (error) {
@@ -352,6 +354,7 @@ int suspend_devices_and_enter(suspend_st
suspend_test_start();
device_resume(PMSG_RESUME);
suspend_test_finish("resume devices");
+ __ftrace_enabled_restore(ftrace_save);
resume_console();
Close:
if (suspend_ops->end)
On Wednesday, 27 of August 2008, Steven Rostedt wrote:
>
> I've been painstakingly debugging the issue with suspend to ram and
> ftraced. The 2.6.28 code does not have this issue, but since the mcount
> recording is not going to be in 27, this must be solved for the ftrace
> daemon version.
>
> The resume from suspend to ram would reboot because it was triple
> faulting. Debugging further, I found that calling the mcount function
> itself was not an issue, but it would fault when it incremented
> preempt_count. preempt_count is on the tasks info structure that is on the
> low memory address of the task's stack. For some reason, it could not
> write to it. Resuming out of suspend to ram does quite a lot of funny
> tricks to get to work, so it is not surprising at all that simply doing a
> preempt_disable() would cause a fault.
>
> Thanks to Rafael for suggesting to add a "while (1);" to find the place in
> resuming that is causing the fault. I would place the loop somewhere in
> the code, compile and reboot and see if it would either reboot (hit the
> fault) or simply hang (hit the loop). Doing this over and over again, I
> narrowed it down that it was happening in enable_nonboot_cpus.
>
> At this point, I found that it is easier to simply disable tracing around
> the suspend code, instead of searching for the particular function that
> can not handle doing a preempt_disable.
>
> This patch disables the tracer as it suspends and reenables it on resume.
>
> I tested this patch on my Laptop, and it can resume fine with the patch.
Great, thanks for fixing that!
> Signed-off-by: Steven Rostedt <[email protected]>
Acked-by: Rafael J. Wysocki <[email protected]>
> ---
> kernel/power/main.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> Index: linux-compile.git/kernel/power/main.c
> ===================================================================
> --- linux-compile.git.orig/kernel/power/main.c 2008-08-27 08:53:11.000000000 -0400
> +++ linux-compile.git/kernel/power/main.c 2008-08-27 08:53:58.000000000 -0400
> @@ -21,6 +21,7 @@
> #include <linux/freezer.h>
> #include <linux/vmstat.h>
> #include <linux/syscalls.h>
> +#include <linux/ftrace.h>
>
> #include "power.h"
>
> @@ -310,7 +311,7 @@ static int suspend_enter(suspend_state_t
> */
> int suspend_devices_and_enter(suspend_state_t state)
> {
> - int error;
> + int error, ftrace_save;
>
> if (!suspend_ops)
> return -ENOSYS;
> @@ -321,6 +322,7 @@ int suspend_devices_and_enter(suspend_st
> goto Close;
> }
> suspend_console();
> + ftrace_save = __ftrace_enabled_save();
> suspend_test_start();
> error = device_suspend(PMSG_SUSPEND);
> if (error) {
> @@ -352,6 +354,7 @@ int suspend_devices_and_enter(suspend_st
> suspend_test_start();
> device_resume(PMSG_RESUME);
> suspend_test_finish("resume devices");
> + __ftrace_enabled_restore(ftrace_save);
> resume_console();
> Close:
> if (suspend_ops->end)
Well, if that's enable_nonboot_cpus() that's faulting, I guess a similar change
in the hibernation code is needed. I'll try to prepare a patch for that.
Thanks,
Rafael
On Wed, Aug 27, 2008 at 09:14:40AM -0400, Steven Rostedt wrote:
>
> I've been painstakingly debugging the issue with suspend to ram and
> ftraced. The 2.6.28 code does not have this issue, but since the mcount
> recording is not going to be in 27, this must be solved for the ftrace
> daemon version.
>
> The resume from suspend to ram would reboot because it was triple
> faulting. Debugging further, I found that calling the mcount function
> itself was not an issue, but it would fault when it incremented
> preempt_count. preempt_count is on the tasks info structure that is on the
> low memory address of the task's stack. For some reason, it could not
> write to it. Resuming out of suspend to ram does quite a lot of funny
> tricks to get to work, so it is not surprising at all that simply doing a
> preempt_disable() would cause a fault.
>
> Thanks to Rafael for suggesting to add a "while (1);" to find the place in
> resuming that is causing the fault. I would place the loop somewhere in
> the code, compile and reboot and see if it would either reboot (hit the
> fault) or simply hang (hit the loop). Doing this over and over again, I
> narrowed it down that it was happening in enable_nonboot_cpus.
>
> At this point, I found that it is easier to simply disable tracing around
> the suspend code, instead of searching for the particular function that
> can not handle doing a preempt_disable.
>
> This patch disables the tracer as it suspends and reenables it on resume.
>
> I tested this patch on my Laptop, and it can resume fine with the patch.
>
> Signed-off-by: Steven Rostedt <[email protected]>
You can add my:
Tested-by: Marcin Slusarz <[email protected]>
Thanks!
Marcin
From: Rafael J. Wysocki <[email protected]>
ftrace: disable tracing for hibernation
In accordance with commit f42ac38c59e0a03d6da0c24a63fb211393f484b0
("ftrace: disable tracing for suspend to ram"), disable tracing
around the suspend code in hibernation code paths.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
kernel/power/disk.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
Index: linux-2.6/kernel/power/disk.c
===================================================================
--- linux-2.6.orig/kernel/power/disk.c
+++ linux-2.6/kernel/power/disk.c
@@ -21,6 +21,7 @@
#include <linux/console.h>
#include <linux/cpu.h>
#include <linux/freezer.h>
+#include <linux/ftrace.h>
#include "power.h"
@@ -255,7 +256,7 @@ static int create_image(int platform_mod
int hibernation_snapshot(int platform_mode)
{
- int error;
+ int error, ftrace_save;
/* Free memory before shutting down devices. */
error = swsusp_shrink_memory();
@@ -267,6 +268,7 @@ int hibernation_snapshot(int platform_mo
goto Close;
suspend_console();
+ ftrace_save = __ftrace_enabled_save();
error = device_suspend(PMSG_FREEZE);
if (error)
goto Recover_platform;
@@ -296,6 +298,7 @@ int hibernation_snapshot(int platform_mo
Resume_devices:
device_resume(in_suspend ?
(error ? PMSG_RECOVER : PMSG_THAW) : PMSG_RESTORE);
+ __ftrace_enabled_restore(ftrace_save);
resume_console();
Close:
platform_end(platform_mode);
@@ -366,10 +369,11 @@ static int resume_target_kernel(void)
int hibernation_restore(int platform_mode)
{
- int error;
+ int error, ftrace_save;
pm_prepare_console();
suspend_console();
+ ftrace_save = __ftrace_enabled_save();
error = device_suspend(PMSG_QUIESCE);
if (error)
goto Finish;
@@ -384,6 +388,7 @@ int hibernation_restore(int platform_mod
platform_restore_cleanup(platform_mode);
device_resume(PMSG_RECOVER);
Finish:
+ __ftrace_enabled_restore(ftrace_save);
resume_console();
pm_restore_console();
return error;
@@ -396,7 +401,7 @@ int hibernation_restore(int platform_mod
int hibernation_platform_enter(void)
{
- int error;
+ int error, ftrace_save;
if (!hibernation_ops)
return -ENOSYS;
@@ -411,6 +416,7 @@ int hibernation_platform_enter(void)
goto Close;
suspend_console();
+ ftrace_save = __ftrace_enabled_save();
error = device_suspend(PMSG_HIBERNATE);
if (error) {
if (hibernation_ops->recover)
@@ -445,6 +451,7 @@ int hibernation_platform_enter(void)
hibernation_ops->finish();
Resume_devices:
device_resume(PMSG_RESTORE);
+ __ftrace_enabled_restore(ftrace_save);
resume_console();
Close:
hibernation_ops->end();
* Rafael J. Wysocki <[email protected]> wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> ftrace: disable tracing for hibernation
>
> In accordance with commit f42ac38c59e0a03d6da0c24a63fb211393f484b0
> ("ftrace: disable tracing for suspend to ram"), disable tracing
> around the suspend code in hibernation code paths.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
applied it to tip/tracing/urgent - thanks Rafael!
Ingo
On Thu, 28 Aug 2008, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> ftrace: disable tracing for hibernation
>
> In accordance with commit f42ac38c59e0a03d6da0c24a63fb211393f484b0
> ("ftrace: disable tracing for suspend to ram"), disable tracing
> around the suspend code in hibernation code paths.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
Looks good to me, Thanks Rafael!
Acked-by: Steven Rostedt <[email protected]>
-- Steve
On Wed 2008-08-27 09:14:40, Steven Rostedt wrote:
>
> I've been painstakingly debugging the issue with suspend to ram and
> ftraced. The 2.6.28 code does not have this issue, but since the mcount
> recording is not going to be in 27, this must be solved for the ftrace
> daemon version.
>
> The resume from suspend to ram would reboot because it was triple
> faulting. Debugging further, I found that calling the mcount function
> itself was not an issue, but it would fault when it incremented
> preempt_count. preempt_count is on the tasks info structure that is on the
> low memory address of the task's stack. For some reason, it could not
> write to it. Resuming out of suspend to ram does quite a lot of funny
> tricks to get to work, so it is not surprising at all that simply doing a
> preempt_disable() would cause a fault.
>
> Thanks to Rafael for suggesting to add a "while (1);" to find the place in
> resuming that is causing the fault. I would place the loop somewhere in
> the code, compile and reboot and see if it would either reboot (hit the
> fault) or simply hang (hit the loop). Doing this over and over again, I
> narrowed it down that it was happening in enable_nonboot_cpus.
>
> At this point, I found that it is easier to simply disable tracing around
> the suspend code, instead of searching for the particular function that
> can not handle doing a preempt_disable.
>
> This patch disables the tracer as it suspends and reenables it on resume.
>
> I tested this patch on my Laptop, and it can resume fine with the patch.
>
> Signed-off-by: Steven Rostedt <[email protected]>
So this is going to be reverted after 2.6.27? Okay, then.
Acked-by: Pavel Machek <[email protected]>
> kernel/power/main.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> Index: linux-compile.git/kernel/power/main.c
> ===================================================================
> --- linux-compile.git.orig/kernel/power/main.c 2008-08-27 08:53:11.000000000 -0400
> +++ linux-compile.git/kernel/power/main.c 2008-08-27 08:53:58.000000000 -0400
> @@ -21,6 +21,7 @@
> #include <linux/freezer.h>
> #include <linux/vmstat.h>
> #include <linux/syscalls.h>
> +#include <linux/ftrace.h>
>
> #include "power.h"
>
> @@ -310,7 +311,7 @@ static int suspend_enter(suspend_state_t
> */
> int suspend_devices_and_enter(suspend_state_t state)
> {
> - int error;
> + int error, ftrace_save;
>
> if (!suspend_ops)
> return -ENOSYS;
> @@ -321,6 +322,7 @@ int suspend_devices_and_enter(suspend_st
> goto Close;
> }
> suspend_console();
> + ftrace_save = __ftrace_enabled_save();
> suspend_test_start();
> error = device_suspend(PMSG_SUSPEND);
> if (error) {
> @@ -352,6 +354,7 @@ int suspend_devices_and_enter(suspend_st
> suspend_test_start();
> device_resume(PMSG_RESUME);
> suspend_test_finish("resume devices");
> + __ftrace_enabled_restore(ftrace_save);
> resume_console();
> Close:
> if (suspend_ops->end)
>
>
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Thu, 28 Aug 2008, Pavel Machek wrote:
> On Wed 2008-08-27 09:14:40, Steven Rostedt wrote:
> >
> > I've been painstakingly debugging the issue with suspend to ram and
> > ftraced. The 2.6.28 code does not have this issue, but since the mcount
> > recording is not going to be in 27, this must be solved for the ftrace
> > daemon version.
> >
> > The resume from suspend to ram would reboot because it was triple
> > faulting. Debugging further, I found that calling the mcount function
> > itself was not an issue, but it would fault when it incremented
> > preempt_count. preempt_count is on the tasks info structure that is on the
> > low memory address of the task's stack. For some reason, it could not
> > write to it. Resuming out of suspend to ram does quite a lot of funny
> > tricks to get to work, so it is not surprising at all that simply doing a
> > preempt_disable() would cause a fault.
> >
> > Thanks to Rafael for suggesting to add a "while (1);" to find the place in
> > resuming that is causing the fault. I would place the loop somewhere in
> > the code, compile and reboot and see if it would either reboot (hit the
> > fault) or simply hang (hit the loop). Doing this over and over again, I
> > narrowed it down that it was happening in enable_nonboot_cpus.
> >
> > At this point, I found that it is easier to simply disable tracing around
> > the suspend code, instead of searching for the particular function that
> > can not handle doing a preempt_disable.
> >
> > This patch disables the tracer as it suspends and reenables it on resume.
> >
> > I tested this patch on my Laptop, and it can resume fine with the patch.
> >
> > Signed-off-by: Steven Rostedt <[email protected]>
>
> So this is going to be reverted after 2.6.27? Okay, then.
Maybe not. The difference is that in 27 it would cause s2ram to fail every
time. In 28 (I have not proved this yet), it may fail s2ram if the tracer
is on.
-- Steve
>
> Acked-by: Pavel Machek <[email protected]>
>
On Thu 2008-08-28 14:39:12, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> ftrace: disable tracing for hibernation
>
> In accordance with commit f42ac38c59e0a03d6da0c24a63fb211393f484b0
> ("ftrace: disable tracing for suspend to ram"), disable tracing
> around the suspend code in hibernation code paths.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
Looks ok to me. ACK.
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html