diff mbox

[RFC,Maverick] UBUNTU: SAUCE: pm: Config option to disable handling of console during suspend/resume

Message ID 1276717326.2985.1.camel@emiko
State Accepted
Delegated to: Leann Ogasawara
Headers show

Commit Message

Leann Ogasawara June 16, 2010, 7:42 p.m. UTC
Hi Amit,

We've got a bug report [1] from Phillip (CC'd) that when resuming from
suspend on Maverick his display never comes back out of power save mode
with the latest 2.6.35 Maverick kernels.  He tested yesterday's 2.6.35
daily mainline build and could not reproduce the issue.  He was able to
narrow it down to the CONFIG_PM_DISABLE_CONSOLE option being enabled.
This config option originates from an Ubuntu SAUCE patch (inlined below)
which was authored by yourself a few years ago.  Is it still necessary
that we carry this patch?  I'm inclined to drop it, or at least disable
the config option, especially since it appears to be causing issues.
Your thoughts? 

Thanks,
Leann

[1] https://bugs.edge.launchpad.net/ubuntu/+source/linux/+bug/594885

>From 6063b4dc3721a63d70f81522fa130372ded60b45 Mon Sep 17 00:00:00 2001
From: Amit Kucheria <amit.kucheria@ubuntu.com>
Date: Fri, 23 May 2008 11:43:45 +0300
Subject: [PATCH] UBUNTU: SAUCE: pm: Config option to disable handling of console during suspend/resume

Signed-off-by: Amit Kucheria <amit.kucheria@ubuntu.com>

Signed-off-by: Ben Collins <ben.collins@canonical.com>
---
 kernel/power/Kconfig   |   15 +++++++++++++++
 kernel/power/console.c |    4 ++++
 2 files changed, 19 insertions(+), 0 deletions(-)

Comments

Lee Jones June 16, 2010, 7:46 p.m. UTC | #1
Hi Leann,

I have found this option invaluable whilst working on suspend/resume bugs.

It should be disabled by default however.

Do you know why it is enabled in Phillip's kernel?

Kind regards,
Lee

On 16/06/10 20:42, Leann Ogasawara wrote:
> Hi Amit,
> 
> We've got a bug report [1] from Phillip (CC'd) that when resuming from
> suspend on Maverick his display never comes back out of power save mode
> with the latest 2.6.35 Maverick kernels.  He tested yesterday's 2.6.35
> daily mainline build and could not reproduce the issue.  He was able to
> narrow it down to the CONFIG_PM_DISABLE_CONSOLE option being enabled.
> This config option originates from an Ubuntu SAUCE patch (inlined below)
> which was authored by yourself a few years ago.  Is it still necessary
> that we carry this patch?  I'm inclined to drop it, or at least disable
> the config option, especially since it appears to be causing issues.
> Your thoughts? 
> 
> Thanks,
> Leann
> 
> [1] https://bugs.edge.launchpad.net/ubuntu/+source/linux/+bug/594885
> 
>>From 6063b4dc3721a63d70f81522fa130372ded60b45 Mon Sep 17 00:00:00 2001
> From: Amit Kucheria <amit.kucheria@ubuntu.com>
> Date: Fri, 23 May 2008 11:43:45 +0300
> Subject: [PATCH] UBUNTU: SAUCE: pm: Config option to disable handling of console during suspend/resume
> 
> Signed-off-by: Amit Kucheria <amit.kucheria@ubuntu.com>
> 
> Signed-off-by: Ben Collins <ben.collins@canonical.com>
> ---
>  kernel/power/Kconfig   |   15 +++++++++++++++
>  kernel/power/console.c |    4 ++++
>  2 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> index 5c36ea9..e829331 100644
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -133,6 +133,21 @@ config SUSPEND_FREEZER
>  config HIBERNATION_NVS
>  	bool
>  
> +config PM_DISABLE_CONSOLE
> +	bool "Disable Power Management messing with the active console"
> +	depends on PM
> +	default n
> +	---help---
> +	  By default, PM will take over the active console (generally, this means
> +	  switching to the console when suspending from X). This can at times cause
> +	  problems, especially if userspace suspend scripts try to do things with
> +	  the console before or after suspending (e.g. calling vbestate).
> +
> +	  To work around this, enable this option so that PM will not handle the
> +	  console.
> +
> +	  If unsure, say N.
> +
>  config HIBERNATION
>  	bool "Hibernation (aka 'suspend to disk')"
>  	depends on PM && SWAP && ARCH_HIBERNATION_POSSIBLE
> diff --git a/kernel/power/console.c b/kernel/power/console.c
> index 218e5af..5b254ba 100644
> --- a/kernel/power/console.c
> +++ b/kernel/power/console.c
> @@ -17,19 +17,23 @@ static int orig_fgconsole, orig_kmsg;
>  
>  int pm_prepare_console(void)
>  {
> +#ifndef CONFIG_PM_DISABLE_CONSOLE
>  	orig_fgconsole = vt_move_to_console(SUSPEND_CONSOLE, 1);
>  	if (orig_fgconsole < 0)
>  		return 1;
>  
>  	orig_kmsg = vt_kmsg_redirect(SUSPEND_CONSOLE);
> +#endif
>  	return 0;
>  }
>  
>  void pm_restore_console(void)
>  {
> +#ifndef CONFIG_PM_DISABLE_CONSOLE
>  	if (orig_fgconsole >= 0) {
>  		vt_move_to_console(orig_fgconsole, 0);
>  		vt_kmsg_redirect(orig_kmsg);
>  	}
> +#endif
>  }
>  #endif
Leann Ogasawara June 16, 2010, 7:51 p.m. UTC | #2
On Wed, 2010-06-16 at 20:46 +0100, Lee Jones wrote:
> Hi Leann,
> 
> I have found this option invaluable whilst working on suspend/resume bugs.
> 
> It should be disabled by default however.
> 
> Do you know why it is enabled in Phillip's kernel?

Hrm, it looked enabled by default to me:

ogasawara@emiko:~/ubuntu-maverick/debian.master/config$ grep -rn "CONFIG_PM_DISABLE_CONSOLE" *
config.common.ports:2627:CONFIG_PM_DISABLE_CONSOLE=y
config.common.ubuntu:3272:CONFIG_PM_DISABLE_CONSOLE=y

Thanks,
Leann

> On 16/06/10 20:42, Leann Ogasawara wrote:
> > Hi Amit,
> > 
> > We've got a bug report [1] from Phillip (CC'd) that when resuming from
> > suspend on Maverick his display never comes back out of power save mode
> > with the latest 2.6.35 Maverick kernels.  He tested yesterday's 2.6.35
> > daily mainline build and could not reproduce the issue.  He was able to
> > narrow it down to the CONFIG_PM_DISABLE_CONSOLE option being enabled.
> > This config option originates from an Ubuntu SAUCE patch (inlined below)
> > which was authored by yourself a few years ago.  Is it still necessary
> > that we carry this patch?  I'm inclined to drop it, or at least disable
> > the config option, especially since it appears to be causing issues.
> > Your thoughts? 
> > 
> > Thanks,
> > Leann
> > 
> > [1] https://bugs.edge.launchpad.net/ubuntu/+source/linux/+bug/594885
> > 
> >>From 6063b4dc3721a63d70f81522fa130372ded60b45 Mon Sep 17 00:00:00 2001
> > From: Amit Kucheria <amit.kucheria@ubuntu.com>
> > Date: Fri, 23 May 2008 11:43:45 +0300
> > Subject: [PATCH] UBUNTU: SAUCE: pm: Config option to disable handling of console during suspend/resume
> > 
> > Signed-off-by: Amit Kucheria <amit.kucheria@ubuntu.com>
> > 
> > Signed-off-by: Ben Collins <ben.collins@canonical.com>
> > ---
> >  kernel/power/Kconfig   |   15 +++++++++++++++
> >  kernel/power/console.c |    4 ++++
> >  2 files changed, 19 insertions(+), 0 deletions(-)
> > 
> > diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> > index 5c36ea9..e829331 100644
> > --- a/kernel/power/Kconfig
> > +++ b/kernel/power/Kconfig
> > @@ -133,6 +133,21 @@ config SUSPEND_FREEZER
> >  config HIBERNATION_NVS
> >  	bool
> >  
> > +config PM_DISABLE_CONSOLE
> > +	bool "Disable Power Management messing with the active console"
> > +	depends on PM
> > +	default n
> > +	---help---
> > +	  By default, PM will take over the active console (generally, this means
> > +	  switching to the console when suspending from X). This can at times cause
> > +	  problems, especially if userspace suspend scripts try to do things with
> > +	  the console before or after suspending (e.g. calling vbestate).
> > +
> > +	  To work around this, enable this option so that PM will not handle the
> > +	  console.
> > +
> > +	  If unsure, say N.
> > +
> >  config HIBERNATION
> >  	bool "Hibernation (aka 'suspend to disk')"
> >  	depends on PM && SWAP && ARCH_HIBERNATION_POSSIBLE
> > diff --git a/kernel/power/console.c b/kernel/power/console.c
> > index 218e5af..5b254ba 100644
> > --- a/kernel/power/console.c
> > +++ b/kernel/power/console.c
> > @@ -17,19 +17,23 @@ static int orig_fgconsole, orig_kmsg;
> >  
> >  int pm_prepare_console(void)
> >  {
> > +#ifndef CONFIG_PM_DISABLE_CONSOLE
> >  	orig_fgconsole = vt_move_to_console(SUSPEND_CONSOLE, 1);
> >  	if (orig_fgconsole < 0)
> >  		return 1;
> >  
> >  	orig_kmsg = vt_kmsg_redirect(SUSPEND_CONSOLE);
> > +#endif
> >  	return 0;
> >  }
> >  
> >  void pm_restore_console(void)
> >  {
> > +#ifndef CONFIG_PM_DISABLE_CONSOLE
> >  	if (orig_fgconsole >= 0) {
> >  		vt_move_to_console(orig_fgconsole, 0);
> >  		vt_kmsg_redirect(orig_kmsg);
> >  	}
> > +#endif
> >  }
> >  #endif
> 
>
Amit Kucheria June 16, 2010, 8:18 p.m. UTC | #3
On 10 Jun 16, Leann Ogasawara wrote:
> Hi Amit,
> 
> We've got a bug report [1] from Phillip (CC'd) that when resuming from
> suspend on Maverick his display never comes back out of power save mode
> with the latest 2.6.35 Maverick kernels.  He tested yesterday's 2.6.35
> daily mainline build and could not reproduce the issue.  He was able to
> narrow it down to the CONFIG_PM_DISABLE_CONSOLE option being enabled.
> This config option originates from an Ubuntu SAUCE patch (inlined below)
> which was authored by yourself a few years ago.  Is it still necessary
> that we carry this patch?  I'm inclined to drop it, or at least disable
> the config option, especially since it appears to be causing issues.
> Your thoughts? 
> 

I think PM is in a lot better shape than 2 years ago. I'd forgotten about it.
Drop it.

It might be worth creating a test kernel with that patch dropped for Lucid
too and posting it to suspend-related bugs.

/Amit

> [1] https://bugs.edge.launchpad.net/ubuntu/+source/linux/+bug/594885
> 
> >From 6063b4dc3721a63d70f81522fa130372ded60b45 Mon Sep 17 00:00:00 2001
> From: Amit Kucheria <amit.kucheria@ubuntu.com>
> Date: Fri, 23 May 2008 11:43:45 +0300
> Subject: [PATCH] UBUNTU: SAUCE: pm: Config option to disable handling of console during suspend/resume
> 
> Signed-off-by: Amit Kucheria <amit.kucheria@ubuntu.com>
> 
> Signed-off-by: Ben Collins <ben.collins@canonical.com>
> ---
>  kernel/power/Kconfig   |   15 +++++++++++++++
>  kernel/power/console.c |    4 ++++
>  2 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> index 5c36ea9..e829331 100644
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -133,6 +133,21 @@ config SUSPEND_FREEZER
>  config HIBERNATION_NVS
>  	bool
>  
> +config PM_DISABLE_CONSOLE
> +	bool "Disable Power Management messing with the active console"
> +	depends on PM
> +	default n
> +	---help---
> +	  By default, PM will take over the active console (generally, this means
> +	  switching to the console when suspending from X). This can at times cause
> +	  problems, especially if userspace suspend scripts try to do things with
> +	  the console before or after suspending (e.g. calling vbestate).
> +
> +	  To work around this, enable this option so that PM will not handle the
> +	  console.
> +
> +	  If unsure, say N.
> +
>  config HIBERNATION
>  	bool "Hibernation (aka 'suspend to disk')"
>  	depends on PM && SWAP && ARCH_HIBERNATION_POSSIBLE
> diff --git a/kernel/power/console.c b/kernel/power/console.c
> index 218e5af..5b254ba 100644
> --- a/kernel/power/console.c
> +++ b/kernel/power/console.c
> @@ -17,19 +17,23 @@ static int orig_fgconsole, orig_kmsg;
>  
>  int pm_prepare_console(void)
>  {
> +#ifndef CONFIG_PM_DISABLE_CONSOLE
>  	orig_fgconsole = vt_move_to_console(SUSPEND_CONSOLE, 1);
>  	if (orig_fgconsole < 0)
>  		return 1;
>  
>  	orig_kmsg = vt_kmsg_redirect(SUSPEND_CONSOLE);
> +#endif
>  	return 0;
>  }
>  
>  void pm_restore_console(void)
>  {
> +#ifndef CONFIG_PM_DISABLE_CONSOLE
>  	if (orig_fgconsole >= 0) {
>  		vt_move_to_console(orig_fgconsole, 0);
>  		vt_kmsg_redirect(orig_kmsg);
>  	}
> +#endif
>  }
>  #endif
> -- 
> 1.7.0.4
> 
> 
> 
> 
>
Amit Kucheria June 16, 2010, 8:28 p.m. UTC | #4
On 10 Jun 16, Amit Kucheria wrote:
> 
> 
> On 10 Jun 16, Leann Ogasawara wrote:
> > Hi Amit,
> > 
> > We've got a bug report [1] from Phillip (CC'd) that when resuming from
> > suspend on Maverick his display never comes back out of power save mode
> > with the latest 2.6.35 Maverick kernels.  He tested yesterday's 2.6.35
> > daily mainline build and could not reproduce the issue.  He was able to
> > narrow it down to the CONFIG_PM_DISABLE_CONSOLE option being enabled.
> > This config option originates from an Ubuntu SAUCE patch (inlined below)
> > which was authored by yourself a few years ago.  Is it still necessary
> > that we carry this patch?  I'm inclined to drop it, or at least disable
> > the config option, especially since it appears to be causing issues.
> > Your thoughts? 
> > 
> 
> I think PM is in a lot better shape than 2 years ago. I'd forgotten about it.
> Drop it.
> 
> It might be worth creating a test kernel with that patch dropped for Lucid
> too and posting it to suspend-related bugs.
> 
> /Amit
> 

I forgot to mention that it was more of a debugging tool, so I am not sure if
should be enabled by default.

/Amit
Lee Jones June 16, 2010, 10:40 p.m. UTC | #5
>> I think PM is in a lot better shape than 2 years ago. I'd forgotten about it.
>> Drop it.

Again, personally I'd like to keep it for use as a debugging tool.

It has served me well over the last few weeks.

By all means turn it off by default, but I'd like to keep it for future use if possible. 

Kind regards,
Lee
Leann Ogasawara June 16, 2010, 11:14 p.m. UTC | #6
On Wed, 2010-06-16 at 23:40 +0100, Lee Jones wrote:
> >> I think PM is in a lot better shape than 2 years ago. I'd forgotten about it.
> >> Drop it.
> 
> Again, personally I'd like to keep it for use as a debugging tool.
> 
> It has served me well over the last few weeks.
> 
> By all means turn it off by default, but I'd like to keep it for future use if possible. 

Even if we do keep it but disable by default, not to mention I'd add to
to the config enforcer, you'll still have to modify the config and build
a custom kernel to enable it.  In that sense it seems it wouldn't be
that much more effort to apply the patch at the same time.

By dropping it we avoid having old cruft in the distro kernel which
deviates us from mainline and knowingly breaks suspend/resume for some
users.  It also has the potential to add a maintenance burden every time
we rebase and definitely adds additional bloat when we do our delta
review at the beginning of each release cycle.

Give me till tomorrow to ponder over it more.  I'd be happy to hear
anyone else's feedback as well.

Thanks,
Leann
Tim Gardner June 17, 2010, 1:55 a.m. UTC | #7
On 06/16/2010 05:14 PM, Leann Ogasawara wrote:
> On Wed, 2010-06-16 at 23:40 +0100, Lee Jones wrote:
>>>> I think PM is in a lot better shape than 2 years ago. I'd forgotten about it.
>>>> Drop it.
>>
>> Again, personally I'd like to keep it for use as a debugging tool.
>>
>> It has served me well over the last few weeks.
>>
>> By all means turn it off by default, but I'd like to keep it for future use if possible.
>
> Even if we do keep it but disable by default, not to mention I'd add to
> to the config enforcer, you'll still have to modify the config and build
> a custom kernel to enable it.  In that sense it seems it wouldn't be
> that much more effort to apply the patch at the same time.
>
> By dropping it we avoid having old cruft in the distro kernel which
> deviates us from mainline and knowingly breaks suspend/resume for some
> users.  It also has the potential to add a maintenance burden every time
> we rebase and definitely adds additional bloat when we do our delta
> review at the beginning of each release cycle.
>
> Give me till tomorrow to ponder over it more.  I'd be happy to hear
> anyone else's feedback as well.
>
> Thanks,
> Leann
>
>

+1 for Leann's opinion. You can instrument your own kernels. The distro 
does not need to carry this kind of cruft.

rtg
Lee Jones June 17, 2010, 6:46 a.m. UTC | #8
> +1 for Leann's opinion. You can instrument your own kernels. The distro
> does not need to carry this kind of cruft.
> 
> rtg

No problem.

I'll add it to my toolbox instead. :)

Kind regards,
Lee
Leann Ogasawara June 17, 2010, 10:48 p.m. UTC | #9
Dropped from Maverick linux master.

Thanks,
Leann

On Wed, 2010-06-16 at 12:42 -0700, Leann Ogasawara wrote:
> Hi Amit,
> 
> We've got a bug report [1] from Phillip (CC'd) that when resuming from
> suspend on Maverick his display never comes back out of power save mode
> with the latest 2.6.35 Maverick kernels.  He tested yesterday's 2.6.35
> daily mainline build and could not reproduce the issue.  He was able to
> narrow it down to the CONFIG_PM_DISABLE_CONSOLE option being enabled.
> This config option originates from an Ubuntu SAUCE patch (inlined below)
> which was authored by yourself a few years ago.  Is it still necessary
> that we carry this patch?  I'm inclined to drop it, or at least disable
> the config option, especially since it appears to be causing issues.
> Your thoughts? 
> 
> Thanks,
> Leann
> 
> [1] https://bugs.edge.launchpad.net/ubuntu/+source/linux/+bug/594885
> 
> >From 6063b4dc3721a63d70f81522fa130372ded60b45 Mon Sep 17 00:00:00 2001
> From: Amit Kucheria <amit.kucheria@ubuntu.com>
> Date: Fri, 23 May 2008 11:43:45 +0300
> Subject: [PATCH] UBUNTU: SAUCE: pm: Config option to disable handling of console during suspend/resume
> 
> Signed-off-by: Amit Kucheria <amit.kucheria@ubuntu.com>
> 
> Signed-off-by: Ben Collins <ben.collins@canonical.com>
> ---
>  kernel/power/Kconfig   |   15 +++++++++++++++
>  kernel/power/console.c |    4 ++++
>  2 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> index 5c36ea9..e829331 100644
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -133,6 +133,21 @@ config SUSPEND_FREEZER
>  config HIBERNATION_NVS
>  	bool
>  
> +config PM_DISABLE_CONSOLE
> +	bool "Disable Power Management messing with the active console"
> +	depends on PM
> +	default n
> +	---help---
> +	  By default, PM will take over the active console (generally, this means
> +	  switching to the console when suspending from X). This can at times cause
> +	  problems, especially if userspace suspend scripts try to do things with
> +	  the console before or after suspending (e.g. calling vbestate).
> +
> +	  To work around this, enable this option so that PM will not handle the
> +	  console.
> +
> +	  If unsure, say N.
> +
>  config HIBERNATION
>  	bool "Hibernation (aka 'suspend to disk')"
>  	depends on PM && SWAP && ARCH_HIBERNATION_POSSIBLE
> diff --git a/kernel/power/console.c b/kernel/power/console.c
> index 218e5af..5b254ba 100644
> --- a/kernel/power/console.c
> +++ b/kernel/power/console.c
> @@ -17,19 +17,23 @@ static int orig_fgconsole, orig_kmsg;
>  
>  int pm_prepare_console(void)
>  {
> +#ifndef CONFIG_PM_DISABLE_CONSOLE
>  	orig_fgconsole = vt_move_to_console(SUSPEND_CONSOLE, 1);
>  	if (orig_fgconsole < 0)
>  		return 1;
>  
>  	orig_kmsg = vt_kmsg_redirect(SUSPEND_CONSOLE);
> +#endif
>  	return 0;
>  }
>  
>  void pm_restore_console(void)
>  {
> +#ifndef CONFIG_PM_DISABLE_CONSOLE
>  	if (orig_fgconsole >= 0) {
>  		vt_move_to_console(orig_fgconsole, 0);
>  		vt_kmsg_redirect(orig_kmsg);
>  	}
> +#endif
>  }
>  #endif
> -- 
> 1.7.0.4
> 
> 
> 
> 
> 
>
diff mbox

Patch

diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index 5c36ea9..e829331 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -133,6 +133,21 @@  config SUSPEND_FREEZER
 config HIBERNATION_NVS
 	bool
 
+config PM_DISABLE_CONSOLE
+	bool "Disable Power Management messing with the active console"
+	depends on PM
+	default n
+	---help---
+	  By default, PM will take over the active console (generally, this means
+	  switching to the console when suspending from X). This can at times cause
+	  problems, especially if userspace suspend scripts try to do things with
+	  the console before or after suspending (e.g. calling vbestate).
+
+	  To work around this, enable this option so that PM will not handle the
+	  console.
+
+	  If unsure, say N.
+
 config HIBERNATION
 	bool "Hibernation (aka 'suspend to disk')"
 	depends on PM && SWAP && ARCH_HIBERNATION_POSSIBLE
diff --git a/kernel/power/console.c b/kernel/power/console.c
index 218e5af..5b254ba 100644
--- a/kernel/power/console.c
+++ b/kernel/power/console.c
@@ -17,19 +17,23 @@  static int orig_fgconsole, orig_kmsg;
 
 int pm_prepare_console(void)
 {
+#ifndef CONFIG_PM_DISABLE_CONSOLE
 	orig_fgconsole = vt_move_to_console(SUSPEND_CONSOLE, 1);
 	if (orig_fgconsole < 0)
 		return 1;
 
 	orig_kmsg = vt_kmsg_redirect(SUSPEND_CONSOLE);
+#endif
 	return 0;
 }
 
 void pm_restore_console(void)
 {
+#ifndef CONFIG_PM_DISABLE_CONSOLE
 	if (orig_fgconsole >= 0) {
 		vt_move_to_console(orig_fgconsole, 0);
 		vt_kmsg_redirect(orig_kmsg);
 	}
+#endif
 }
 #endif