Messages in this thread Patch in this message | | | Date | Mon, 17 Aug 2009 10:45:00 +0200 (CEST) | From | Thomas Gleixner <> | Subject | Re: [patch 00/15] clocksource / timekeeping rework V4 (resend V3 + bug fix) |
| |
On Mon, 17 Aug 2009, Martin Schwidefsky wrote: > > - 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.
Darn, yes. But it's not hard to fix.
Thanks,
tglx --- diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c index 02dc22d..c6bff11 100644 --- a/kernel/time/clocksource.c +++ b/kernel/time/clocksource.c @@ -131,6 +131,7 @@ static cycle_t watchdog_last; static int watchdog_running; static void clocksource_watchdog_work(struct work_struct *work); +static void __clocksource_change_rating(struct clocksource *cs, int rating); /* * Interval: 0.5sec Threshold: 0.0625s @@ -309,6 +310,7 @@ static void clocksource_watchdog_work(struct work_struct *work) unsigned long flags; LIST_HEAD(unstable); + mutex_lock(&clocksource_mutex); spin_lock_irqsave(&watchdog_lock, flags); list_for_each_entry_safe(cs, tmp, &watchdog_list, wd_list) if (cs->flags & CLOCK_SOURCE_UNSTABLE) { @@ -322,8 +324,9 @@ static void clocksource_watchdog_work(struct work_struct *work) /* 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); + __clocksource_change_rating(cs, 0); } + mutex_unlock(&clocksource_mutex); } #else /* CONFIG_CLOCKSOURCE_WATCHDOG */ @@ -470,16 +473,21 @@ int clocksource_register(struct clocksource *cs) } EXPORT_SYMBOL(clocksource_register); +static void __clocksource_change_rating(struct clocksource *cs, int rating) +{ + list_del(&cs->list); + cs->rating = rating; + clocksource_enqueue(cs); + clocksource_select(); +} + /** * clocksource_change_rating - Change the rating of a registered clocksource */ void clocksource_change_rating(struct clocksource *cs, int rating) { mutex_lock(&clocksource_mutex); - list_del(&cs->list); - cs->rating = rating; - clocksource_enqueue(cs); - clocksource_select(); + __clocksource_change_rating(cs, rating); mutex_unlock(&clocksource_mutex); } EXPORT_SYMBOL(clocksource_change_rating);
| |