This patch modifies the current clocksource API so that clocks
can be masked if they have specific negative qualities. For
instance, if a clock is not atomic you can choose not to include
it in the list you select from, atomic in this case being lockless.
The following qualities can be masked off,
#define CLOCKSOURCE_NOT_CONTINUOUS 1
#define CLOCKSOURCE_UNSTABLE 2
#define CLOCKSOURCE_NOT_ATOMIC 4
#define CLOCKSOURCE_UNDER_32BITS 8
#define CLOCKSOURCE_64BITS 16
#define CLOCKSOURCE_PM_AFFECTED 32
I modify the .flags to accomplish this.
The reasoning behind this is that it's not interesting to have a
positive "rating" value, and have a positive flags value.
For instance, if the clock has a high rating you assume it's continuous.
The selection process isn't helped by stating "continuous" in the flags.
The point is to list all the negative side effects that some clocks have
which the programmer knows in advance that their code can not tolerate.
Signed-Off-By: Daniel Walker <[email protected]>
---
include/linux/clocksource.h | 35 +++++++++++++++++++++++++-------
kernel/time/clocksource.c | 47 ++++++++++++++++++--------------------------
kernel/time/jiffies.c | 1
kernel/time/timekeeping.c | 11 ++++++----
4 files changed, 55 insertions(+), 39 deletions(-)
Index: linux-2.6.19/include/linux/clocksource.h
===================================================================
--- linux-2.6.19.orig/include/linux/clocksource.h
+++ linux-2.6.19/include/linux/clocksource.h
@@ -30,6 +30,11 @@ extern struct clocksource clocksource_ji
extern struct sys_device clocksource_sys_device;
/*
+ * Read function type.
+ */
+typedef cycle_t (*clock_read_t)(void);
+
+/*
* Allows inlined calling for notifier routines.
*/
extern struct atomic_notifier_head clocksource_list_notifier;
@@ -40,6 +45,7 @@ extern struct atomic_notifier_head clock
#define CLOCKSOURCE_NOTIFY_REGISTER 1
#define CLOCKSOURCE_NOTIFY_RATING 2
#define CLOCKSOURCE_NOTIFY_FREQ 4
+#define CLOCKSOURCE_NOTIFY_UNSTABLE 8
/*
* Defined so the initcall can be changes without touching
@@ -119,12 +125,6 @@ struct clocksource {
s64 error;
};
-/*
- * Clock source flags bits::
- */
-#define CLOCK_SOURCE_IS_CONTINUOUS 0x01
-#define CLOCK_SOURCE_MUST_VERIFY 0x02
-
/* simplify initialization of mask field */
#define CLOCKSOURCE_MASK(bits) (cycle_t)(bits<64 ? ((1ULL<<bits)-1) : -1)
@@ -235,6 +235,9 @@ static inline void clocksource_calculate
c->xtime_interval = (u64)c->cycle_interval * c->mult;
}
+/*
+ * Clocksource flags
+ */
#define CLOCKSOURCE_NOT_CONTINUOUS 1
#define CLOCKSOURCE_UNSTABLE 2
#define CLOCKSOURCE_NOT_ATOMIC 4
@@ -243,9 +246,25 @@ static inline void clocksource_calculate
#define CLOCKSOURCE_PM_AFFECTED 32
/* used to install a new clocksource */
+extern void clocksource_mark_unstable(struct clocksource *);
+extern struct clocksource *clocksource_get_unstable(void);
extern int clocksource_register(struct clocksource*);
extern void clocksource_rating_change(struct clocksource*);
-extern struct clocksource * clocksource_get_clock(char*);
+extern struct clocksource * clocksource_get_clock(char*, unsigned long);
+
+
+/**
+ * clocksource_get_masked_clock - Finds highest rated clocksource w/o mask
+ * @mask: Clocksource features that are not wanted.
+ *
+ * Returns the highest rated clocksource that doesn't have any of the bits set
+ * in mask. If none are register the jiffies clock is returned. If all the clocks
+ * have the mask bits set, then NULL is returned.
+ */
+static inline struct clocksource * clocksource_get_masked_clock(unsigned long mask)
+{
+ return clocksource_get_clock(NULL, mask);
+}
/**
* clocksource_get_best_clock - Finds highest rated clocksource
@@ -255,7 +274,7 @@ extern struct clocksource * clocksource_
*/
static inline struct clocksource * clocksource_get_best_clock(void)
{
- return clocksource_get_clock(NULL);
+ return clocksource_get_clock(NULL, 0);
}
#ifdef CONFIG_GENERIC_TIME_VSYSCALL
Index: linux-2.6.19/kernel/time/clocksource.c
===================================================================
--- linux-2.6.19.orig/kernel/time/clocksource.c
+++ linux-2.6.19/kernel/time/clocksource.c
@@ -51,7 +51,8 @@ ATOMIC_NOTIFIER_HEAD(clocksource_list_no
* If no clocksources are registered the jiffies clock is
* returned.
*/
-static struct clocksource * __is_registered(char * name)
+static inline
+struct clocksource * __is_registered(char * name, unsigned long mask)
{
struct list_head *tmp;
@@ -59,7 +60,11 @@ static struct clocksource * __is_registe
struct clocksource *src;
src = list_entry(tmp, struct clocksource, list);
- if (!strcmp(src->name, name))
+ if (name) {
+ if (!strcmp(src->name, name))
+ return src;
+
+ } else if (!(src->flags & mask))
return src;
}
@@ -75,32 +80,34 @@ static struct clocksource * __is_registe
* Returns the clocksource if registered, zero otherwise.
* If the @name is null the highest rated clock is returned.
*/
-static inline struct clocksource * __get_clock(char * name)
+static inline struct clocksource * __get_clock(char * name, unsigned long mask)
{
if (unlikely(list_empty(&clocksource_list)))
return &clocksource_jiffies;
- if (!name)
+ if (!name && !mask)
return list_entry(clocksource_list.next,
struct clocksource, list);
- return __is_registered(name);
+ return __is_registered(name, mask);
}
/**
* clocksource_get_clock - Finds a specific clocksource
* @name: name of the clocksource to return
+ * @mask: Clocks with these flags set will
+ * be ignored.
*
* Returns the clocksource if registered, zero otherwise.
*/
-struct clocksource * clocksource_get_clock(char * name)
+struct clocksource * clocksource_get_clock(char * name, unsigned long mask)
{
struct clocksource * ret;
unsigned long flags;
spin_lock_irqsave(&clocksource_lock, flags);
- ret = __get_clock(name);
+ ret = __get_clock(name, mask);
spin_unlock_irqrestore(&clocksource_lock, flags);
return ret;
}
@@ -111,7 +118,7 @@ struct clocksource * clocksource_get_clo
*
* Adds a clocksource to the clocksource_list in sorted order.
*/
-static void __sorted_list_add(struct clocksource *c)
+static inline void __sorted_list_add(struct clocksource *c)
{
struct list_head *tmp;
@@ -158,31 +165,17 @@ int clocksource_register(struct clocksou
EXPORT_SYMBOL(clocksource_register);
/**
- * clocksource_rating_change - Allows dynamic rating changes for register
- * clocksources.
+ * clocksource_mark_unstable - Mark the clocks as unstable.
*
- * Signals that a clocksource is dynamically changing it's rating.
- * This could happen if a clocksource becomes more/less stable.
+ * This signals that this clock has become unstable.
*/
-void clocksource_rating_change(struct clocksource *c)
+void clocksource_mark_unstable(struct clocksource *c)
{
- unsigned long flags;
-
- spin_lock_irqsave(&clocksource_lock, flags);
-
- /*
- * Re-register the clocksource with it's new rating.
- */
- list_del_init(&c->list);
- __sorted_list_add(c);
-
- spin_unlock_irqrestore(&clocksource_lock, flags);
+ c->flags |= CLOCKSOURCE_UNSTABLE;
atomic_notifier_call_chain(&clocksource_list_notifier,
- CLOCKSOURCE_NOTIFY_RATING, c);
-
+ CLOCKSOURCE_NOTIFY_UNSTABLE, c);
}
-EXPORT_SYMBOL(clocksource_rating_change);
#ifdef CONFIG_SYSFS
/**
Index: linux-2.6.19/kernel/time/jiffies.c
===================================================================
--- linux-2.6.19.orig/kernel/time/jiffies.c
+++ linux-2.6.19/kernel/time/jiffies.c
@@ -62,6 +62,7 @@ struct clocksource clocksource_jiffies =
.mask = 0xffffffff, /*32bits*/
.mult = NSEC_PER_JIFFY << JIFFIES_SHIFT, /* details above */
.shift = JIFFIES_SHIFT,
+ .flags = CLOCKSOURCE_NOT_CONTINUOUS, /* not free running */
};
static int __init init_jiffies_clocksource(void)
Index: linux-2.6.19/kernel/time/timekeeping.c
===================================================================
--- linux-2.6.19.orig/kernel/time/timekeeping.c
+++ linux-2.6.19/kernel/time/timekeeping.c
@@ -181,9 +181,12 @@ static void timekeeping_change_clocksour
u64 nsec;
if (!*override_name)
- new = clocksource_get_clock(NULL);
+ /*
+ * Mask off unstable clocks.
+ */
+ new = clocksource_get_masked_clock(CLOCKSOURCE_UNSTABLE);
else
- new = clocksource_get_clock(override_name);
+ new = clocksource_get_clock(override_name, 0);
if (clock != new) {
now = clocksource_read(new);
@@ -216,7 +219,7 @@ int timekeeping_is_continuous(void)
do {
seq = read_seqbegin(&xtime_lock);
- ret = clock->flags & CLOCK_SOURCE_IS_CONTINUOUS;
+ ret = !(clock->flags & CLOCKSOURCE_NOT_CONTINUOUS);
} while (read_seqretry(&xtime_lock, seq));
@@ -231,7 +234,7 @@ clocksource_callback(struct notifier_blo
switch (op) {
case CLOCKSOURCE_NOTIFY_FREQ:
- case CLOCKSOURCE_NOTIFY_RATING:
+ case CLOCKSOURCE_NOTIFY_UNSTABLE:
atomic_inc(&clock_check);
}
--
On Wed, 2007-01-31 at 09:39 -0800, Daniel Walker wrote:
> > please read my reply above! To repeat: such flags tend to get forgotten,
> > resulting in a less safe default behavior. Clock hardware and thus
> > clocksources are fundamentally fragile so we want to default to the
> > safest behavior. I.e. if the IS_CONTINOUS flag is 'forgotten', the clock
> > wont be usable as a clock verification base for example. The flag has to
> > be affirmatively set to mark the clocksource continous.
>
> I don't see this as an issue .. Your assuming that not continuous clocks
> will be prevalent which they aren't ..
And how does this change Ingo's statement ? Such beasts exist and having
them default to the safe side is good.
Also I really do not see the "huge burden" for a clocksource coder to
add this flag. If he forgets it, then he can not use highres / dynticks
and nothing breaks.
tglx
On Wed, 2007-01-31 at 18:55 +0100, Thomas Gleixner wrote:
> On Wed, 2007-01-31 at 09:39 -0800, Daniel Walker wrote:
> > > please read my reply above! To repeat: such flags tend to get forgotten,
> > > resulting in a less safe default behavior. Clock hardware and thus
> > > clocksources are fundamentally fragile so we want to default to the
> > > safest behavior. I.e. if the IS_CONTINOUS flag is 'forgotten', the clock
> > > wont be usable as a clock verification base for example. The flag has to
> > > be affirmatively set to mark the clocksource continous.
> >
> > I don't see this as an issue .. Your assuming that not continuous clocks
> > will be prevalent which they aren't ..
>
> And how does this change Ingo's statement ? Such beasts exist and having
> them default to the safe side is good.
>
> Also I really do not see the "huge burden" for a clocksource coder to
> add this flag. If he forgets it, then he can not use highres / dynticks
> and nothing breaks.
We're not talking about a rare system runtime configuration . If that
were the case I would agree safe is better..
I'm assuming that programmers will test their code, and others will
review the code .. Catering to any other situation doesn't make sense to
me. On top of that those clocks are rare, and not desirable ..
Daniel
On Wed, 2007-01-31 at 10:07 -0800, Daniel Walker wrote:
> I'm assuming that programmers will test their code, and others will
> review the code .. Catering to any other situation doesn't make sense to
> me. On top of that those clocks are rare, and not desirable ..
So what are you arguing about burden ? It is a property of the
clocksource, that it is continuous, and not the other way round. It does
not matter how rare or desirable devices are.
What about the other bunch of useless defines:
+#define CLOCKSOURCE_NOT_ATOMIC 4
+#define CLOCKSOURCE_UNDER_32BITS 8
+#define CLOCKSOURCE_64BITS 16
Are those just for fun or are you actually putting the burden to get
them straight on the poor overworked clocksource developer ?
Also please define bit values as hexadecimal constants and not as
integers.
tglx