* [PATCH 0/2] Android PM fixes @ 2009-01-14 1:07 Arve Hjønnevåg 2009-01-14 1:07 ` [PATCH 1/2] PM: Wait for console in resume Arve Hjønnevåg 0 siblings, 1 reply; 12+ messages in thread From: Arve Hjønnevåg @ 2009-01-14 1:07 UTC (permalink / raw) To: linux-pm; +Cc: swetland 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] PM: Wait for console in resume. 2009-01-14 1:07 [PATCH 0/2] Android PM fixes Arve Hjønnevåg @ 2009-01-14 1:07 ` Arve Hjønnevåg 2009-01-14 1:07 ` [PATCH 2/2] PM: Fix suspend_console/resume_console to use only one semaphore Arve Hjønnevåg 0 siblings, 1 reply; 12+ messages in thread From: Arve Hjønnevåg @ 2009-01-14 1:07 UTC (permalink / raw) To: linux-pm; +Cc: swetland 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] PM: Fix suspend_console/resume_console to use only one semaphore. 2009-01-14 1:07 ` [PATCH 1/2] PM: Wait for console in resume Arve Hjønnevåg @ 2009-01-14 1:07 ` Arve Hjønnevåg 2009-01-28 19:30 ` Pavel Machek 0 siblings, 1 reply; 12+ messages in thread From: Arve Hjønnevåg @ 2009-01-14 1:07 UTC (permalink / raw) To: linux-pm; +Cc: swetland 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] PM: Fix suspend_console/resume_console to use only one semaphore. 2009-01-14 1:07 ` [PATCH 2/2] PM: Fix suspend_console/resume_console to use only one semaphore Arve Hjønnevåg @ 2009-01-28 19:30 ` Pavel Machek 2009-01-31 3:27 ` [PATCH 1/2] PM: Wait for console in resume Arve Hjønnevåg 2009-01-31 3:27 ` Arve Hjønnevåg 0 siblings, 2 replies; 12+ messages in thread From: Pavel Machek @ 2009-01-28 19:30 UTC (permalink / raw) To: Arve Hj??nnev??g; +Cc: swetland, linux-pm 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] PM: Wait for console in resume. 2009-01-28 19:30 ` Pavel Machek @ 2009-01-31 3:27 ` Arve Hjønnevåg 2009-01-31 3:27 ` Arve Hjønnevåg 1 sibling, 0 replies; 12+ messages in thread From: Arve Hjønnevåg @ 2009-01-31 3:27 UTC (permalink / raw) To: linux-pm, linux-kernel; +Cc: swetland 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 1/2] PM: Wait for console in resume. 2009-01-28 19:30 ` Pavel Machek 2009-01-31 3:27 ` [PATCH 1/2] PM: Wait for console in resume Arve Hjønnevåg @ 2009-01-31 3:27 ` Arve Hjønnevåg 2009-01-31 3:27 ` [PATCH 2/2] PM: Fix suspend_console/resume_console to use only one semaphore Arve Hjønnevåg 2009-01-31 3:27 ` [PATCH 2/2] " Arve Hjønnevåg 1 sibling, 2 replies; 12+ messages in thread From: Arve Hjønnevåg @ 2009-01-31 3:27 UTC (permalink / raw) To: linux-pm, linux-kernel; +Cc: swetland, pavel, 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] PM: Fix suspend_console/resume_console to use only one semaphore. 2009-01-31 3:27 ` Arve Hjønnevåg @ 2009-01-31 3:27 ` Arve Hjønnevåg 2009-02-03 2:03 ` [PATCH] " Arve Hjønnevåg 2009-02-03 2:03 ` Arve Hjønnevåg 2009-01-31 3:27 ` [PATCH 2/2] " Arve Hjønnevåg 1 sibling, 2 replies; 12+ messages in thread From: Arve Hjønnevåg @ 2009-01-31 3:27 UTC (permalink / raw) To: linux-pm, linux-kernel; +Cc: swetland, pavel, 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH] PM: Fix suspend_console/resume_console to use only one semaphore. 2009-01-31 3:27 ` [PATCH 2/2] PM: Fix suspend_console/resume_console to use only one semaphore Arve Hjønnevåg @ 2009-02-03 2:03 ` Arve Hjønnevåg 2009-02-08 2:36 ` Benjamin Herrenschmidt 2009-02-08 2:36 ` [linux-pm] " Benjamin Herrenschmidt 2009-02-03 2:03 ` Arve Hjønnevåg 1 sibling, 2 replies; 12+ messages in thread From: Arve Hjønnevåg @ 2009-02-03 2:03 UTC (permalink / raw) To: linux-pm, linux-kernel; +Cc: swetland, mm-commits, Arve Hjønnevåg This fixes a race where a thread acquires the console while the console is suspended, and the console is resumed before this thread releases it. In this case, the secondary console semaphore would be left locked, and the primary semaphore would be released twice. This in turn would cause 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. One client of try_acquire_console_sem is acquire_console_semaphore_for_printk, which uses it to prevent printk from using the console while it is suspended. 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 69188f2..e3602d0 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); @@ -891,12 +890,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(); } @@ -912,11 +913,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; } @@ -926,6 +925,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; @@ -979,7 +982,7 @@ void release_console_sem(void) unsigned wake_klogd = 0; if (console_suspended) { - up(&secondary_console_sem); + up(&console_sem); return; } -- 1.6.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] PM: Fix suspend_console/resume_console to use only one semaphore. 2009-02-03 2:03 ` [PATCH] " Arve Hjønnevåg @ 2009-02-08 2:36 ` Benjamin Herrenschmidt 2009-02-08 2:36 ` [linux-pm] " Benjamin Herrenschmidt 1 sibling, 0 replies; 12+ messages in thread From: Benjamin Herrenschmidt @ 2009-02-08 2:36 UTC (permalink / raw) To: Arve Hjønnevåg; +Cc: swetland, linux-pm, mm-commits, linux-kernel On Mon, 2009-02-02 at 18:03 -0800, Arve Hjønnevåg wrote: > This fixes a race where a thread acquires the console while the > console is suspended, and the console is resumed before this > thread releases it. In this case, the secondary console > semaphore would be left locked, and the primary semaphore would > be released twice. This in turn would cause 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. One client of try_acquire_console_sem > is acquire_console_semaphore_for_printk, which uses it to > prevent printk from using the console while it is suspended. Right, we shouldn't hold the semaphore over the whole time. In fact, I even have some doubts about the need for suspending the console at all... The way I did it back then for powerbooks was to implement a "suspended" state in the fbdev/fbcon core which the drivers call effectively causing the core to stop touching the framebuffer. Works fine for kernel consoles though of course userspace or X needs something different, but that's orthogonal. Of course that doesn't help with serial, vgacon etc... so each drivers needs to suspend itself and be made to ignore incoming requests but that's something that has to be done anyway at some stage... I suspect the suspend console trick is mostly helpful with vgacon.. Ben. > 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 69188f2..e3602d0 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); > > @@ -891,12 +890,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(); > } > @@ -912,11 +913,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; > } > @@ -926,6 +925,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; > @@ -979,7 +982,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 _______________________________________________ linux-pm mailing list linux-pm@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/linux-pm ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [linux-pm] [PATCH] PM: Fix suspend_console/resume_console to use only one semaphore. 2009-02-03 2:03 ` [PATCH] " Arve Hjønnevåg 2009-02-08 2:36 ` Benjamin Herrenschmidt @ 2009-02-08 2:36 ` Benjamin Herrenschmidt 1 sibling, 0 replies; 12+ messages in thread From: Benjamin Herrenschmidt @ 2009-02-08 2:36 UTC (permalink / raw) To: Arve Hjønnevåg; +Cc: linux-pm, linux-kernel, swetland, mm-commits On Mon, 2009-02-02 at 18:03 -0800, Arve Hjønnevåg wrote: > This fixes a race where a thread acquires the console while the > console is suspended, and the console is resumed before this > thread releases it. In this case, the secondary console > semaphore would be left locked, and the primary semaphore would > be released twice. This in turn would cause 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. One client of try_acquire_console_sem > is acquire_console_semaphore_for_printk, which uses it to > prevent printk from using the console while it is suspended. Right, we shouldn't hold the semaphore over the whole time. In fact, I even have some doubts about the need for suspending the console at all... The way I did it back then for powerbooks was to implement a "suspended" state in the fbdev/fbcon core which the drivers call effectively causing the core to stop touching the framebuffer. Works fine for kernel consoles though of course userspace or X needs something different, but that's orthogonal. Of course that doesn't help with serial, vgacon etc... so each drivers needs to suspend itself and be made to ignore incoming requests but that's something that has to be done anyway at some stage... I suspect the suspend console trick is mostly helpful with vgacon.. Ben. > 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 69188f2..e3602d0 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); > > @@ -891,12 +890,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(); > } > @@ -912,11 +913,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; > } > @@ -926,6 +925,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; > @@ -979,7 +982,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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] PM: Fix suspend_console/resume_console to use only one semaphore. 2009-01-31 3:27 ` [PATCH 2/2] PM: Fix suspend_console/resume_console to use only one semaphore Arve Hjønnevåg 2009-02-03 2:03 ` [PATCH] " Arve Hjønnevåg @ 2009-02-03 2:03 ` Arve Hjønnevåg 1 sibling, 0 replies; 12+ messages in thread From: Arve Hjønnevåg @ 2009-02-03 2:03 UTC (permalink / raw) To: linux-pm, linux-kernel; +Cc: swetland, mm-commits This fixes a race where a thread acquires the console while the console is suspended, and the console is resumed before this thread releases it. In this case, the secondary console semaphore would be left locked, and the primary semaphore would be released twice. This in turn would cause 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. One client of try_acquire_console_sem is acquire_console_semaphore_for_printk, which uses it to prevent printk from using the console while it is suspended. 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 69188f2..e3602d0 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); @@ -891,12 +890,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(); } @@ -912,11 +913,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; } @@ -926,6 +925,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; @@ -979,7 +982,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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] PM: Fix suspend_console/resume_console to use only one semaphore. 2009-01-31 3:27 ` Arve Hjønnevåg 2009-01-31 3:27 ` [PATCH 2/2] PM: Fix suspend_console/resume_console to use only one semaphore Arve Hjønnevåg @ 2009-01-31 3:27 ` Arve Hjønnevåg 1 sibling, 0 replies; 12+ messages in thread From: Arve Hjønnevåg @ 2009-01-31 3:27 UTC (permalink / raw) To: linux-pm, linux-kernel; +Cc: swetland 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2009-02-08 2:37 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-01-14 1:07 [PATCH 0/2] Android PM fixes Arve Hjønnevåg 2009-01-14 1:07 ` [PATCH 1/2] PM: Wait for console in resume Arve Hjønnevåg 2009-01-14 1:07 ` [PATCH 2/2] PM: Fix suspend_console/resume_console to use only one semaphore Arve Hjønnevåg 2009-01-28 19:30 ` Pavel Machek 2009-01-31 3:27 ` [PATCH 1/2] PM: Wait for console in resume Arve Hjønnevåg 2009-01-31 3:27 ` Arve Hjønnevåg 2009-01-31 3:27 ` [PATCH 2/2] PM: Fix suspend_console/resume_console to use only one semaphore Arve Hjønnevåg 2009-02-03 2:03 ` [PATCH] " Arve Hjønnevåg 2009-02-08 2:36 ` Benjamin Herrenschmidt 2009-02-08 2:36 ` [linux-pm] " Benjamin Herrenschmidt 2009-02-03 2:03 ` Arve Hjønnevåg 2009-01-31 3:27 ` [PATCH 2/2] " Arve Hjønnevåg
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.