Messages in this thread | | | Date | Mon, 17 Aug 2009 09:40:42 +0200 | From | Martin Schwidefsky <> | Subject | Re: [patch 00/15] clocksource / timekeeping rework V4 (resend V3 + bug fix) |
| |
On Sat, 15 Aug 2009 12:08:35 +0200 (CEST) Thomas Gleixner <tglx@linutronix.de> wrote:
> Yeah, we need to call clocksource_change_rating() outside of watchdog > lock. Spotted a missing _irqrestore as well. > > Thanks, > > tglx > --- > > diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c > index a1657b5..02dc22d 100644 > --- a/kernel/time/clocksource.c > +++ b/kernel/time/clocksource.c > @@ -307,16 +307,23 @@ static void clocksource_watchdog_work(struct work_struct *work) > { > struct clocksource *cs, *tmp; > unsigned long flags; > + LIST_HEAD(unstable); > > spin_lock_irqsave(&watchdog_lock, flags); > list_for_each_entry_safe(cs, tmp, &watchdog_list, wd_list) > if (cs->flags & CLOCK_SOURCE_UNSTABLE) { > list_del_init(&cs->wd_list); > - clocksource_change_rating(cs, 0); > + list_add(&cs->wd_list, &unstable); > } > /* Check if the watchdog timer needs to be stopped. */ > clocksource_stop_watchdog(); > - spin_unlock(&watchdog_lock); > + spin_unlock_irqrestore(&watchdog_lock, flags); > + > + /* Needs to be done outside of watchdog lock */ > + list_for_each_entry_safe(cs, tmp, &unstable, wd_list) { > + list_del_init(&cs->wd_list); > + clocksource_change_rating(cs, 0); > + } > } > > #else /* CONFIG_CLOCKSOURCE_WATCHDOG */
Autsch, the s/spin_unlock/spin_unlock_irqrestore/ is rather obvious, no? In theory we now have a reference count problem. The call to clocksource_change_rating are no done outside of the clocksource mutex and outside of the watchdog lock. A concurrent clocksource_unregister could remove the clock.
-- blue skies, Martin.
"Reality continues to ruin my life." - Calvin.
| |