lkml.org 
[lkml]   [2009]   [Aug]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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);

\
 
 \ /
  Last update: 2009-08-17 10:47    [W:1.506 / U:0.536 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site