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

[Pavel Machek <pavel@ucw.cz>]

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