Skip to content

Commit

Permalink
clocksource: Prevent potential kgdb dead lock
Browse files Browse the repository at this point in the history
commit 0f8e8ef (clocksource: Simplify clocksource watchdog resume
logic) introduced a potential kgdb dead lock. When the kernel is
stopped by kgdb inside code which holds watchdog_lock then kgdb dead
locks in clocksource_resume_watchdog().

clocksource_resume_watchdog() is called from kbdg via
clocksource_touch_watchdog() to avoid that the clock source watchdog
marks TSC unstable after the kernel has been stopped.

Solve this by replacing spin_lock with a spin_trylock and just return
in case the lock is held. Not resetting the watchdog might result in
TSC becoming marked unstable, but that's an acceptable penalty for
using kgdb.

The timekeeping is anyway easily screwed up by kgdb when the system
uses either jiffies or a clock source which wraps in short intervals
(e.g. pm_timer wraps about every 4.6s), so we really do not have to
worry about that occasional TSC marked unstable side effect.

The second caller of clocksource_resume_watchdog() is
clocksource_resume(). The trylock is safe here as well because the
system is UP at this point, interrupts are disabled and nothing else
can hold watchdog_lock().

Reported-by: Jason Wessel <jason.wessel@windriver.com>
LKML-Reference: <1264480000-6997-4-git-send-email-jason.wessel@windriver.com>
Cc: kgdb-bugreport@lists.sourceforge.net
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: John Stultz <johnstul@us.ibm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
  • Loading branch information
Thomas Gleixner committed Jan 26, 2010
1 parent 9a3cbe3 commit 7b7422a
Showing 1 changed file with 15 additions and 3 deletions.
18 changes: 15 additions & 3 deletions kernel/time/clocksource.c
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,19 @@ static void clocksource_resume_watchdog(void)
{
unsigned long flags;

spin_lock_irqsave(&watchdog_lock, flags);
/*
* We use trylock here to avoid a potential dead lock when
* kgdb calls this code after the kernel has been stopped with
* watchdog_lock held. When watchdog_lock is held we just
* return and accept, that the watchdog might trigger and mark
* the monitored clock source (usually TSC) unstable.
*
* This does not affect the other caller clocksource_resume()
* because at this point the kernel is UP, interrupts are
* disabled and nothing can hold watchdog_lock.
*/
if (!spin_trylock_irqsave(&watchdog_lock, flags))
return;
clocksource_reset_watchdog();
spin_unlock_irqrestore(&watchdog_lock, flags);
}
Expand Down Expand Up @@ -458,8 +470,8 @@ void clocksource_resume(void)
* clocksource_touch_watchdog - Update watchdog
*
* Update the watchdog after exception contexts such as kgdb so as not
* to incorrectly trip the watchdog.
*
* to incorrectly trip the watchdog. This might fail when the kernel
* was stopped in code which holds watchdog_lock.
*/
void clocksource_touch_watchdog(void)
{
Expand Down

0 comments on commit 7b7422a

Please sign in to comment.