* ftraced and suspend to ram
@ 2008-08-21 15:49 Steven Rostedt
2008-08-21 18:15 ` Rafael J. Wysocki
0 siblings, 1 reply; 27+ messages in thread
From: Steven Rostedt @ 2008-08-21 15:49 UTC (permalink / raw)
To: Rafael J. Wysocki, Nigel Cunningham, Pavel Machek
Cc: LKML, Ingo Molnar, Andrew Morton, Linus Torvalds
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: ftraced and suspend to ram
2008-08-21 15:49 ftraced and suspend to ram Steven Rostedt
@ 2008-08-21 18:15 ` Rafael J. Wysocki
2008-08-21 18:26 ` Steven Rostedt
0 siblings, 1 reply; 27+ messages in thread
From: Rafael J. Wysocki @ 2008-08-21 18:15 UTC (permalink / raw)
To: Steven Rostedt
Cc: Nigel Cunningham, Pavel Machek, LKML, Ingo Molnar, Andrew Morton,
Linus Torvalds
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: ftraced and suspend to ram
2008-08-21 18:15 ` Rafael J. Wysocki
@ 2008-08-21 18:26 ` Steven Rostedt
2008-08-21 18:37 ` Rafael J. Wysocki
0 siblings, 1 reply; 27+ messages in thread
From: Steven Rostedt @ 2008-08-21 18:26 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Nigel Cunningham, Pavel Machek, LKML, Ingo Molnar, Andrew Morton,
Linus Torvalds
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: ftraced and suspend to ram
2008-08-21 18:26 ` Steven Rostedt
@ 2008-08-21 18:37 ` Rafael J. Wysocki
2008-08-21 19:59 ` Rafael J. Wysocki
0 siblings, 1 reply; 27+ messages in thread
From: Rafael J. Wysocki @ 2008-08-21 18:37 UTC (permalink / raw)
To: Steven Rostedt
Cc: Nigel Cunningham, Pavel Machek, LKML, Ingo Molnar, Andrew Morton,
Linus Torvalds
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: ftraced and suspend to ram
2008-08-21 18:37 ` Rafael J. Wysocki
@ 2008-08-21 19:59 ` Rafael J. Wysocki
2008-08-22 4:46 ` Ingo Molnar
0 siblings, 1 reply; 27+ messages in thread
From: Rafael J. Wysocki @ 2008-08-21 19:59 UTC (permalink / raw)
To: Steven Rostedt
Cc: Nigel Cunningham, Pavel Machek, LKML, Ingo Molnar, Andrew Morton,
Linus Torvalds
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 */
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: ftraced and suspend to ram
2008-08-21 19:59 ` Rafael J. Wysocki
@ 2008-08-22 4:46 ` Ingo Molnar
2008-08-22 7:23 ` Pavel Machek
2008-08-22 10:22 ` Rafael J. Wysocki
0 siblings, 2 replies; 27+ messages in thread
From: Ingo Molnar @ 2008-08-22 4:46 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Steven Rostedt, Nigel Cunningham, Pavel Machek, LKML,
Andrew Morton, Linus Torvalds
* Rafael J. Wysocki <rjw@sisk.pl> 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 <rjw@sisk.pl>
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 <mingo@elte.hu>
---
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 */
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: ftraced and suspend to ram
2008-08-22 4:46 ` Ingo Molnar
@ 2008-08-22 7:23 ` Pavel Machek
2008-08-22 10:35 ` Marcin Slusarz
2008-08-22 10:22 ` Rafael J. Wysocki
1 sibling, 1 reply; 27+ messages in thread
From: Pavel Machek @ 2008-08-22 7:23 UTC (permalink / raw)
To: Ingo Molnar
Cc: Rafael J. Wysocki, Steven Rostedt, Nigel Cunningham, LKML,
Andrew Morton, Linus Torvalds
>
> * Rafael J. Wysocki <rjw@sisk.pl> 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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: ftraced and suspend to ram
2008-08-22 4:46 ` Ingo Molnar
2008-08-22 7:23 ` Pavel Machek
@ 2008-08-22 10:22 ` Rafael J. Wysocki
1 sibling, 0 replies; 27+ messages in thread
From: Rafael J. Wysocki @ 2008-08-22 10:22 UTC (permalink / raw)
To: Ingo Molnar
Cc: Steven Rostedt, Nigel Cunningham, Pavel Machek, LKML,
Andrew Morton, Linus Torvalds
On Friday, 22 of August 2008, Ingo Molnar wrote:
>
> * Rafael J. Wysocki <rjw@sisk.pl> 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 <rjw@sisk.pl>
> 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 <mingo@elte.hu>
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 */
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: ftraced and suspend to ram
2008-08-22 7:23 ` Pavel Machek
@ 2008-08-22 10:35 ` Marcin Slusarz
2008-08-22 10:46 ` Pavel Machek
2008-08-22 16:39 ` ftraced and " Rafael J. Wysocki
0 siblings, 2 replies; 27+ messages in thread
From: Marcin Slusarz @ 2008-08-22 10:35 UTC (permalink / raw)
To: Pavel Machek
Cc: Ingo Molnar, Rafael J. Wysocki, Steven Rostedt, Nigel Cunningham,
LKML, Andrew Morton, Linus Torvalds
On Fri, Aug 22, 2008 at 09:23:43AM +0200, Pavel Machek wrote:
> >
> > * Rafael J. Wysocki <rjw@sisk.pl> 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>
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: ftraced and suspend to ram
2008-08-22 10:35 ` Marcin Slusarz
@ 2008-08-22 10:46 ` Pavel Machek
2008-08-22 20:33 ` Steven Rostedt
2008-08-22 16:39 ` ftraced and " Rafael J. Wysocki
1 sibling, 1 reply; 27+ messages in thread
From: Pavel Machek @ 2008-08-22 10:46 UTC (permalink / raw)
To: Marcin Slusarz
Cc: Ingo Molnar, Rafael J. Wysocki, Steven Rostedt, Nigel Cunningham,
LKML, Andrew Morton, Linus Torvalds
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 <rjw@sisk.pl> 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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: ftraced and suspend to ram
2008-08-22 10:35 ` Marcin Slusarz
2008-08-22 10:46 ` Pavel Machek
@ 2008-08-22 16:39 ` Rafael J. Wysocki
2008-08-22 20:54 ` Marcin Slusarz
2008-08-23 4:18 ` Russ Dill
1 sibling, 2 replies; 27+ messages in thread
From: Rafael J. Wysocki @ 2008-08-22 16:39 UTC (permalink / raw)
To: Marcin Slusarz
Cc: Pavel Machek, Ingo Molnar, Steven Rostedt, Nigel Cunningham,
LKML, Andrew Morton, Linus Torvalds
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 <rjw@sisk.pl> 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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: ftraced and suspend to ram
2008-08-22 10:46 ` Pavel Machek
@ 2008-08-22 20:33 ` Steven Rostedt
2008-08-22 20:52 ` Rafael J. Wysocki
0 siblings, 1 reply; 27+ messages in thread
From: Steven Rostedt @ 2008-08-22 20:33 UTC (permalink / raw)
To: Pavel Machek
Cc: Marcin Slusarz, Ingo Molnar, Rafael J. Wysocki, Nigel Cunningham,
LKML, Andrew Morton, Linus Torvalds
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: ftraced and suspend to ram
2008-08-22 20:33 ` Steven Rostedt
@ 2008-08-22 20:52 ` Rafael J. Wysocki
2008-08-22 20:55 ` Steven Rostedt
0 siblings, 1 reply; 27+ messages in thread
From: Rafael J. Wysocki @ 2008-08-22 20:52 UTC (permalink / raw)
To: Steven Rostedt
Cc: Pavel Machek, Marcin Slusarz, Ingo Molnar, Nigel Cunningham,
LKML, Andrew Morton, Linus Torvalds
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: ftraced and suspend to ram
2008-08-22 16:39 ` ftraced and " Rafael J. Wysocki
@ 2008-08-22 20:54 ` Marcin Slusarz
2008-08-22 21:17 ` Rafael J. Wysocki
2008-08-23 4:18 ` Russ Dill
1 sibling, 1 reply; 27+ messages in thread
From: Marcin Slusarz @ 2008-08-22 20:54 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Pavel Machek, Ingo Molnar, Steven Rostedt, Nigel Cunningham,
LKML, Andrew Morton, Linus Torvalds
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 <rjw@sisk.pl> 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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: ftraced and suspend to ram
2008-08-22 20:52 ` Rafael J. Wysocki
@ 2008-08-22 20:55 ` Steven Rostedt
2008-08-22 21:11 ` Rafael J. Wysocki
2008-08-27 13:14 ` [PATCH] ftrace: disable tracing for " Steven Rostedt
0 siblings, 2 replies; 27+ messages in thread
From: Steven Rostedt @ 2008-08-22 20:55 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Pavel Machek, Marcin Slusarz, Ingo Molnar, Nigel Cunningham,
LKML, Andrew Morton, Linus Torvalds
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: ftraced and suspend to ram
2008-08-22 20:55 ` Steven Rostedt
@ 2008-08-22 21:11 ` Rafael J. Wysocki
2008-08-27 13:14 ` [PATCH] ftrace: disable tracing for " Steven Rostedt
1 sibling, 0 replies; 27+ messages in thread
From: Rafael J. Wysocki @ 2008-08-22 21:11 UTC (permalink / raw)
To: Steven Rostedt
Cc: Pavel Machek, Marcin Slusarz, Ingo Molnar, Nigel Cunningham,
LKML, Andrew Morton, Linus Torvalds
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: ftraced and suspend to ram
2008-08-22 20:54 ` Marcin Slusarz
@ 2008-08-22 21:17 ` Rafael J. Wysocki
0 siblings, 0 replies; 27+ messages in thread
From: Rafael J. Wysocki @ 2008-08-22 21:17 UTC (permalink / raw)
To: Marcin Slusarz
Cc: Pavel Machek, Ingo Molnar, Steven Rostedt, Nigel Cunningham,
LKML, Andrew Morton, Linus Torvalds
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 <rjw@sisk.pl> 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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: ftraced and suspend to ram
2008-08-22 16:39 ` ftraced and " Rafael J. Wysocki
2008-08-22 20:54 ` Marcin Slusarz
@ 2008-08-23 4:18 ` Russ Dill
1 sibling, 0 replies; 27+ messages in thread
From: Russ Dill @ 2008-08-23 4:18 UTC (permalink / raw)
To: linux-kernel
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.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH] ftrace: disable tracing for suspend to ram
2008-08-22 20:55 ` Steven Rostedt
2008-08-22 21:11 ` Rafael J. Wysocki
@ 2008-08-27 13:14 ` Steven Rostedt
2008-08-27 13:26 ` Rafael J. Wysocki
` (2 more replies)
1 sibling, 3 replies; 27+ messages in thread
From: Steven Rostedt @ 2008-08-27 13:14 UTC (permalink / raw)
To: LKML
Cc: Pavel Machek, Marcin Slusarz, Ingo Molnar, Nigel Cunningham,
Rafael J. Wysocki, Andrew Morton, Linus Torvalds
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 <srostedt@redhat.com>
---
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)
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] ftrace: disable tracing for suspend to ram
2008-08-27 13:14 ` [PATCH] ftrace: disable tracing for " Steven Rostedt
@ 2008-08-27 13:26 ` Rafael J. Wysocki
2008-08-28 12:39 ` [PATCH] ftrace: disable tracing for hibernation Rafael J. Wysocki
2008-08-27 21:27 ` [PATCH] ftrace: disable tracing for suspend to ram Marcin Slusarz
2008-08-28 7:28 ` Pavel Machek
2 siblings, 1 reply; 27+ messages in thread
From: Rafael J. Wysocki @ 2008-08-27 13:26 UTC (permalink / raw)
To: Steven Rostedt
Cc: LKML, Pavel Machek, Marcin Slusarz, Ingo Molnar,
Nigel Cunningham, Andrew Morton, Linus Torvalds
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 <srostedt@redhat.com>
Acked-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
> 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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] ftrace: disable tracing for suspend to ram
2008-08-27 13:14 ` [PATCH] ftrace: disable tracing for " Steven Rostedt
2008-08-27 13:26 ` Rafael J. Wysocki
@ 2008-08-27 21:27 ` Marcin Slusarz
2008-08-28 7:28 ` Pavel Machek
2 siblings, 0 replies; 27+ messages in thread
From: Marcin Slusarz @ 2008-08-27 21:27 UTC (permalink / raw)
To: Steven Rostedt
Cc: LKML, Pavel Machek, Ingo Molnar, Nigel Cunningham,
Rafael J. Wysocki, Andrew Morton, Linus Torvalds
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 <srostedt@redhat.com>
You can add my:
Tested-by: Marcin Slusarz <marcin.slusarz@gmail.com>
Thanks!
Marcin
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] ftrace: disable tracing for suspend to ram
2008-08-27 13:14 ` [PATCH] ftrace: disable tracing for " Steven Rostedt
2008-08-27 13:26 ` Rafael J. Wysocki
2008-08-27 21:27 ` [PATCH] ftrace: disable tracing for suspend to ram Marcin Slusarz
@ 2008-08-28 7:28 ` Pavel Machek
2008-08-29 13:43 ` Steven Rostedt
2 siblings, 1 reply; 27+ messages in thread
From: Pavel Machek @ 2008-08-28 7:28 UTC (permalink / raw)
To: Steven Rostedt
Cc: LKML, Marcin Slusarz, Ingo Molnar, Nigel Cunningham,
Rafael J. Wysocki, Andrew Morton, Linus Torvalds
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 <srostedt@redhat.com>
So this is going to be reverted after 2.6.27? Okay, then.
Acked-by: Pavel Machek <pavel@suse.cz>
> 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
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH] ftrace: disable tracing for hibernation
2008-08-27 13:26 ` Rafael J. Wysocki
@ 2008-08-28 12:39 ` Rafael J. Wysocki
2008-08-28 12:42 ` Ingo Molnar
` (2 more replies)
0 siblings, 3 replies; 27+ messages in thread
From: Rafael J. Wysocki @ 2008-08-28 12:39 UTC (permalink / raw)
To: Steven Rostedt
Cc: LKML, Pavel Machek, Marcin Slusarz, Ingo Molnar,
Nigel Cunningham, Andrew Morton, Linus Torvalds
From: Rafael J. Wysocki <rjw@sisk.pl>
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 <rjw@sisk.pl>
---
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();
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] ftrace: disable tracing for hibernation
2008-08-28 12:39 ` [PATCH] ftrace: disable tracing for hibernation Rafael J. Wysocki
@ 2008-08-28 12:42 ` Ingo Molnar
2008-08-28 12:44 ` Steven Rostedt
2008-08-29 23:53 ` Pavel Machek
2 siblings, 0 replies; 27+ messages in thread
From: Ingo Molnar @ 2008-08-28 12:42 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Steven Rostedt, LKML, Pavel Machek, Marcin Slusarz,
Nigel Cunningham, Andrew Morton, Linus Torvalds
* Rafael J. Wysocki <rjw@sisk.pl> wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
>
> 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 <rjw@sisk.pl>
applied it to tip/tracing/urgent - thanks Rafael!
Ingo
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] ftrace: disable tracing for hibernation
2008-08-28 12:39 ` [PATCH] ftrace: disable tracing for hibernation Rafael J. Wysocki
2008-08-28 12:42 ` Ingo Molnar
@ 2008-08-28 12:44 ` Steven Rostedt
2008-08-29 23:53 ` Pavel Machek
2 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2008-08-28 12:44 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: LKML, Pavel Machek, Marcin Slusarz, Ingo Molnar,
Nigel Cunningham, Andrew Morton, Linus Torvalds
On Thu, 28 Aug 2008, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
>
> 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 <rjw@sisk.pl>
Looks good to me, Thanks Rafael!
Acked-by: Steven Rostedt <srostedt@redhat.com>
-- Steve
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] ftrace: disable tracing for suspend to ram
2008-08-28 7:28 ` Pavel Machek
@ 2008-08-29 13:43 ` Steven Rostedt
0 siblings, 0 replies; 27+ messages in thread
From: Steven Rostedt @ 2008-08-29 13:43 UTC (permalink / raw)
To: Pavel Machek
Cc: LKML, Marcin Slusarz, Ingo Molnar, Nigel Cunningham,
Rafael J. Wysocki, Andrew Morton, Linus Torvalds
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 <srostedt@redhat.com>
>
> 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 <pavel@suse.cz>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] ftrace: disable tracing for hibernation
2008-08-28 12:39 ` [PATCH] ftrace: disable tracing for hibernation Rafael J. Wysocki
2008-08-28 12:42 ` Ingo Molnar
2008-08-28 12:44 ` Steven Rostedt
@ 2008-08-29 23:53 ` Pavel Machek
2 siblings, 0 replies; 27+ messages in thread
From: Pavel Machek @ 2008-08-29 23:53 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Steven Rostedt, LKML, Marcin Slusarz, Ingo Molnar,
Nigel Cunningham, Andrew Morton, Linus Torvalds
On Thu 2008-08-28 14:39:12, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
>
> 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 <rjw@sisk.pl>
Looks ok to me. ACK.
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2008-08-29 23:52 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-08-21 15:49 ftraced and suspend to ram Steven Rostedt
2008-08-21 18:15 ` Rafael J. Wysocki
2008-08-21 18:26 ` Steven Rostedt
2008-08-21 18:37 ` Rafael J. Wysocki
2008-08-21 19:59 ` Rafael J. Wysocki
2008-08-22 4:46 ` Ingo Molnar
2008-08-22 7:23 ` Pavel Machek
2008-08-22 10:35 ` Marcin Slusarz
2008-08-22 10:46 ` Pavel Machek
2008-08-22 20:33 ` Steven Rostedt
2008-08-22 20:52 ` Rafael J. Wysocki
2008-08-22 20:55 ` Steven Rostedt
2008-08-22 21:11 ` Rafael J. Wysocki
2008-08-27 13:14 ` [PATCH] ftrace: disable tracing for " Steven Rostedt
2008-08-27 13:26 ` Rafael J. Wysocki
2008-08-28 12:39 ` [PATCH] ftrace: disable tracing for hibernation Rafael J. Wysocki
2008-08-28 12:42 ` Ingo Molnar
2008-08-28 12:44 ` Steven Rostedt
2008-08-29 23:53 ` Pavel Machek
2008-08-27 21:27 ` [PATCH] ftrace: disable tracing for suspend to ram Marcin Slusarz
2008-08-28 7:28 ` Pavel Machek
2008-08-29 13:43 ` Steven Rostedt
2008-08-22 16:39 ` ftraced and " Rafael J. Wysocki
2008-08-22 20:54 ` Marcin Slusarz
2008-08-22 21:17 ` Rafael J. Wysocki
2008-08-23 4:18 ` Russ Dill
2008-08-22 10:22 ` Rafael J. Wysocki
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).