[PATCH 0/2] Android PM fixes

[PATCH 0/2] Android PM fixes

[Arve Hjønnevåg]

These patches fixes some bugs where we occasionally got stuck entering or leaving suspend.

--
Arve Hjønnevåg

_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

[PATCH 1/2] PM: Wait for console in resume.

[Arve Hjønnevåg]

Avoids later waking up to a blinking cursor if the device
woke up and returned to sleep before the console switch happened.

Signed-off-by: Brian Swetland <swetland@google.com>
Signed-off-by: Arve Hjønnevåg <arve@android.com>
---
 kernel/power/console.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/kernel/power/console.c b/kernel/power/console.c
index b8628be..a3961b2 100644
--- a/kernel/power/console.c
+++ b/kernel/power/console.c
@@ -78,6 +78,12 @@ void pm_restore_console(void)
 	}
 	set_console(orig_fgconsole);
 	release_console_sem();
+
+	if (vt_waitactive(orig_fgconsole)) {
+		pr_debug("Resume: Can't switch VCs.");
+		return;
+	}
+
 	kmsg_redirect = orig_kmsg;
 }
 #endif
-- 
1.6.1

_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

[PATCH 2/2] PM: Fix suspend_console/resume_console to use only one semaphore.

[Arve Hjønnevåg]

This fixes a race if another thread acquired the console
while it was suspended and released it after it was resumed
the secondary console semaphre would be left locked, and the
primary semaphore would be released twice. This in turn caused
the console switch on suspend or resume to hang forever.

Note that suspend_console does not actually lock the console
for clients that use acquire_console_sem, it only locks it for
clients that use try_acquire_console_sem. If we change
suspend_console to fully lock the console, then the kernel
may deadlock on suspend.

Signed-off-by: Arve Hjønnevåg <arve@android.com>
---
 kernel/printk.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/kernel/printk.c b/kernel/printk.c
index 7015733..118ee77 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -73,7 +73,6 @@ EXPORT_SYMBOL(oops_in_progress);
  * driver system.
  */
 static DECLARE_MUTEX(console_sem);
-static DECLARE_MUTEX(secondary_console_sem);
 struct console *console_drivers;
 EXPORT_SYMBOL_GPL(console_drivers);
 
@@ -896,12 +895,14 @@ void suspend_console(void)
 	printk("Suspending console(s) (use no_console_suspend to debug)\n");
 	acquire_console_sem();
 	console_suspended = 1;
+	up(&console_sem);
 }
 
 void resume_console(void)
 {
 	if (!console_suspend_enabled)
 		return;
+	down(&console_sem);
 	console_suspended = 0;
 	release_console_sem();
 }
@@ -917,11 +918,9 @@ void resume_console(void)
 void acquire_console_sem(void)
 {
 	BUG_ON(in_interrupt());
-	if (console_suspended) {
-		down(&secondary_console_sem);
-		return;
-	}
 	down(&console_sem);
+	if (console_suspended)
+		return;
 	console_locked = 1;
 	console_may_schedule = 1;
 }
@@ -931,6 +930,10 @@ int try_acquire_console_sem(void)
 {
 	if (down_trylock(&console_sem))
 		return -1;
+	if (console_suspended) {
+		up(&console_sem);
+		return -1;
+	}
 	console_locked = 1;
 	console_may_schedule = 0;
 	return 0;
@@ -984,7 +987,7 @@ void release_console_sem(void)
 	unsigned wake_klogd = 0;
 
 	if (console_suspended) {
-		up(&secondary_console_sem);
+		up(&console_sem);
 		return;
 	}
 
-- 
1.6.1

_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

Re: [PATCH 2/2] PM: Fix suspend_console/resume_console to use only one semaphore.

[Pavel Machek]

On Tue 2009-01-13 17:07:51, Arve Hj??nnev??g wrote:
> This fixes a race if another thread acquired the console
> while it was suspended and released it after it was resumed
> the secondary console semaphre would be left locked, and the
> primary semaphore would be released twice. This in turn caused
> the console switch on suspend or resume to hang forever.
> 
> Note that suspend_console does not actually lock the console
> for clients that use acquire_console_sem, it only locks it for
> clients that use try_acquire_console_sem. If we change
> suspend_console to fully lock the console, then the kernel
> may deadlock on suspend.
> 
> Signed-off-by: Arve Hj??nnev??g <arve@android.com>

This probably needs to go to lkml for proper review. Getting rid of
secondary_console_sem would be nice cleanup in itself...


> ---
>  kernel/printk.c |   15 +++++++++------
>  1 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/printk.c b/kernel/printk.c
> index 7015733..118ee77 100644
> --- a/kernel/printk.c
> +++ b/kernel/printk.c
> @@ -73,7 +73,6 @@ EXPORT_SYMBOL(oops_in_progress);
>   * driver system.
>   */
>  static DECLARE_MUTEX(console_sem);
> -static DECLARE_MUTEX(secondary_console_sem);
>  struct console *console_drivers;
>  EXPORT_SYMBOL_GPL(console_drivers);
>  
> @@ -896,12 +895,14 @@ void suspend_console(void)
>  	printk("Suspending console(s) (use no_console_suspend to debug)\n");
>  	acquire_console_sem();
>  	console_suspended = 1;
> +	up(&console_sem);
>  }
>  
>  void resume_console(void)
>  {
>  	if (!console_suspend_enabled)
>  		return;
> +	down(&console_sem);
>  	console_suspended = 0;
>  	release_console_sem();
>  }
> @@ -917,11 +918,9 @@ void resume_console(void)
>  void acquire_console_sem(void)
>  {
>  	BUG_ON(in_interrupt());
> -	if (console_suspended) {
> -		down(&secondary_console_sem);
> -		return;
> -	}
>  	down(&console_sem);
> +	if (console_suspended)
> +		return;
>  	console_locked = 1;
>  	console_may_schedule = 1;
>  }
> @@ -931,6 +930,10 @@ int try_acquire_console_sem(void)
>  {
>  	if (down_trylock(&console_sem))
>  		return -1;
> +	if (console_suspended) {
> +		up(&console_sem);
> +		return -1;
> +	}
>  	console_locked = 1;
>  	console_may_schedule = 0;
>  	return 0;
> @@ -984,7 +987,7 @@ void release_console_sem(void)
>  	unsigned wake_klogd = 0;
>  
>  	if (console_suspended) {
> -		up(&secondary_console_sem);
> +		up(&console_sem);
>  		return;
>  	}
>  
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH 1/2] PM: Wait for console in resume.

[Arve Hjønnevåg]

Avoids later waking up to a blinking cursor if the device
woke up and returned to sleep before the console switch happened.

Signed-off-by: Brian Swetland <swetland@google.com>
Signed-off-by: Arve Hjønnevåg <arve@android.com>
---
 kernel/power/console.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/kernel/power/console.c b/kernel/power/console.c
index b8628be..a3961b2 100644
--- a/kernel/power/console.c
+++ b/kernel/power/console.c
@@ -78,6 +78,12 @@ void pm_restore_console(void)
 	}
 	set_console(orig_fgconsole);
 	release_console_sem();
+
+	if (vt_waitactive(orig_fgconsole)) {
+		pr_debug("Resume: Can't switch VCs.");
+		return;
+	}
+
 	kmsg_redirect = orig_kmsg;
 }
 #endif
-- 
1.6.1

_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

[PATCH 2/2] PM: Fix suspend_console/resume_console to use only one semaphore.

[Arve Hjønnevåg]

This fixes a race if another thread acquired the console
while it was suspended and released it after it was resumed
the secondary console semaphre would be left locked, and the
primary semaphore would be released twice. This in turn caused
the console switch on suspend or resume to hang forever.

Note that suspend_console does not actually lock the console
for clients that use acquire_console_sem, it only locks it for
clients that use try_acquire_console_sem. If we change
suspend_console to fully lock the console, then the kernel
may deadlock on suspend.

Signed-off-by: Arve Hjønnevåg <arve@android.com>
---
 kernel/printk.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/kernel/printk.c b/kernel/printk.c
index 7015733..118ee77 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -73,7 +73,6 @@ EXPORT_SYMBOL(oops_in_progress);
  * driver system.
  */
 static DECLARE_MUTEX(console_sem);
-static DECLARE_MUTEX(secondary_console_sem);
 struct console *console_drivers;
 EXPORT_SYMBOL_GPL(console_drivers);
 
@@ -896,12 +895,14 @@ void suspend_console(void)
 	printk("Suspending console(s) (use no_console_suspend to debug)\n");
 	acquire_console_sem();
 	console_suspended = 1;
+	up(&console_sem);
 }
 
 void resume_console(void)
 {
 	if (!console_suspend_enabled)
 		return;
+	down(&console_sem);
 	console_suspended = 0;
 	release_console_sem();
 }
@@ -917,11 +918,9 @@ void resume_console(void)
 void acquire_console_sem(void)
 {
 	BUG_ON(in_interrupt());
-	if (console_suspended) {
-		down(&secondary_console_sem);
-		return;
-	}
 	down(&console_sem);
+	if (console_suspended)
+		return;
 	console_locked = 1;
 	console_may_schedule = 1;
 }
@@ -931,6 +930,10 @@ int try_acquire_console_sem(void)
 {
 	if (down_trylock(&console_sem))
 		return -1;
+	if (console_suspended) {
+		up(&console_sem);
+		return -1;
+	}
 	console_locked = 1;
 	console_may_schedule = 0;
 	return 0;
@@ -984,7 +987,7 @@ void release_console_sem(void)
 	unsigned wake_klogd = 0;
 
 	if (console_suspended) {
-		up(&secondary_console_sem);
+		up(&console_sem);
 		return;
 	}
 
-- 
1.6.1

_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm