This introduces some new API calls,
- clocksource_get_clock()
Allows a clock lookup by name.
- clocksource_rating_change()
Used by a clocksource to signal a rating change. Replaces
reselect_clocksource()
I also moved the the clock source list to a plist, which removes some lookup
overhead in the average case.
Signed-Off-By: Daniel Walker <[email protected]>
---
arch/i386/kernel/tsc.c | 2
include/linux/clocksource.h | 45 ++++++++++++-
kernel/time/clocksource.c | 149 ++++++++++++++++++++++++++++----------------
3 files changed, 139 insertions(+), 57 deletions(-)
Index: linux-2.6.17/arch/i386/kernel/tsc.c
===================================================================
--- linux-2.6.17.orig/arch/i386/kernel/tsc.c
+++ linux-2.6.17/arch/i386/kernel/tsc.c
@@ -351,7 +351,7 @@ static int tsc_update_callback(void)
/* check to see if we should switch to the safe clocksource: */
if (clocksource_tsc.rating != 50 && check_tsc_unstable()) {
clocksource_tsc.rating = 50;
- clocksource_reselect();
+ clocksource_rating_change(&clocksource_tsc);
change = 1;
}
Index: linux-2.6.17/include/linux/clocksource.h
===================================================================
--- linux-2.6.17.orig/include/linux/clocksource.h
+++ linux-2.6.17/include/linux/clocksource.h
@@ -12,12 +12,20 @@
#include <linux/timex.h>
#include <linux/time.h>
#include <linux/list.h>
+#include <linux/plist.h>
+#include <linux/sysdev.h>
#include <asm/div64.h>
#include <asm/io.h>
/* clocksource cycle base type */
typedef u64 cycle_t;
+/*
+ * This is the only generic clock, it should be used
+ * for early initialization.
+ */
+extern struct clocksource clocksource_jiffies;
+
/**
* struct clocksource - hardware abstraction for a free running counter
* Provides mostly state-free accessors to the underlying hardware.
@@ -51,7 +59,7 @@ typedef u64 cycle_t;
*/
struct clocksource {
char *name;
- struct list_head list;
+ struct plist_node list;
int rating;
cycle_t (*read)(void);
cycle_t mask;
@@ -148,6 +156,25 @@ static inline s64 cyc2ns(struct clocksou
}
/**
+ * ns2cyc - converts nanoseconds to clocksource cycles
+ * @cs: Pointer to clocksource
+ * @ns: Nanoseconds
+ *
+ * Uses the clocksource to convert nanoseconds back to cycles.
+ *
+ * XXX - This could use some mult_lxl_ll() asm optimization
+ */
+static inline cycle_t ns2cyc(struct clocksource *cs, s64 ns)
+{
+ u64 ret = ns;
+
+ ret <<= cs->shift;
+ do_div(ret, cs->mult);
+
+ return ret;
+}
+
+/**
* clocksource_calculate_interval - Calculates a clocksource interval struct
*
* @c: Pointer to clocksource.
@@ -178,8 +205,18 @@ static inline void clocksource_calculate
/* used to install a new clocksource */
-int clocksource_register(struct clocksource*);
-void clocksource_reselect(void);
-struct clocksource* clocksource_get_next(void);
+extern int clocksource_register(struct clocksource*);
+extern void clocksource_rating_change(struct clocksource*);
+extern struct clocksource * clocksource_get_clock(char*);
+/**
+ * clocksource_get_best_clock - Finds highest rated clocksource
+ *
+ * Returns the highest rated clocksource. If none are register the
+ * jiffies clock is returned.
+ */
+static inline struct clocksource * clocksource_get_best_clock(void)
+{
+ return clocksource_get_clock(NULL);
+}
#endif /* _LINUX_CLOCKSOURCE_H */
Index: linux-2.6.17/kernel/time/clocksource.c
===================================================================
--- linux-2.6.17.orig/kernel/time/clocksource.c
+++ linux-2.6.17/kernel/time/clocksource.c
@@ -32,13 +32,18 @@
/* XXX - Would like a better way for initializing curr_clocksource */
extern struct clocksource clocksource_jiffies;
+/*
+ * Internally used to invert the rating, so lower is better.
+ */
+#define CLOCKSOURCE_RATING(x) (INT_MAX-x)
+
/*[Clocksource internal variables]---------
* curr_clocksource:
* currently selected clocksource. Initialized to clocksource_jiffies.
* next_clocksource:
* pending next selected clocksource.
* clocksource_list:
- * linked list with the registered clocksources
+ * priority list with the registered clocksources
* clocksource_lock:
* protects manipulations to curr_clocksource and next_clocksource
* and the clocksource_list
@@ -47,7 +52,8 @@ extern struct clocksource clocksource_ji
*/
static struct clocksource *curr_clocksource = &clocksource_jiffies;
static struct clocksource *next_clocksource;
-static LIST_HEAD(clocksource_list);
+static struct plist_head clocksource_list =
+ PLIST_HEAD_INIT(clocksource_list, clocksource_lock);
static DEFINE_SPINLOCK(clocksource_lock);
static char override_name[32];
@@ -70,84 +76,111 @@ struct clocksource *clocksource_get_next
}
/**
- * select_clocksource - Finds the best registered clocksource.
+ * __is_registered - Returns a clocksource if it's registered
+ * @name: name of the clocksource to return
*
* Private function. Must hold clocksource_lock when called.
*
- * Looks through the list of registered clocksources, returning
- * the one with the highest rating value. If there is a clocksource
- * name that matches the override string, it returns that clocksource.
+ * Returns the clocksource if registered, zero otherwise.
+ * If no clocksources are registered the jiffies clock is
+ * returned.
*/
-static struct clocksource *select_clocksource(void)
+static struct clocksource * __is_registered(char * name)
{
- struct clocksource *best = NULL;
- struct list_head *tmp;
+ struct plist_node *tmp;
- list_for_each(tmp, &clocksource_list) {
+ plist_for_each(tmp, &clocksource_list) {
struct clocksource *src;
src = list_entry(tmp, struct clocksource, list);
- if (!best)
- best = src;
-
- /* check for override: */
- if (strlen(src->name) == strlen(override_name) &&
- !strcmp(src->name, override_name)) {
- best = src;
- break;
- }
- /* pick the highest rating: */
- if (src->rating > best->rating)
- best = src;
+ if (!strcmp(src->name, name))
+ return src;
}
- return best;
+ return 0;
}
/**
- * is_registered_source - Checks if clocksource is registered
- * @c: pointer to a clocksource
+ * __get_clock - Finds a specific clocksource
+ * @name: name of the clocksource to return
*
- * Private helper function. Must hold clocksource_lock when called.
+ * Private function. Must hold clocksource_lock when called.
*
- * Returns one if the clocksource is already registered, zero otherwise.
+ * Returns the clocksource if registered, zero otherwise.
+ * If the @name is null the highest rated clock is returned.
*/
-static int is_registered_source(struct clocksource *c)
+static inline struct clocksource * __get_clock(char * name)
{
- int len = strlen(c->name);
- struct list_head *tmp;
- list_for_each(tmp, &clocksource_list) {
- struct clocksource *src;
+ if (unlikely(plist_head_empty(&clocksource_list)))
+ return &clocksource_jiffies;
- src = list_entry(tmp, struct clocksource, list);
- if (strlen(src->name) == len && !strcmp(src->name, c->name))
- return 1;
- }
+ if (!name)
+ return plist_first_entry(&clocksource_list, struct clocksource,
+ list);
- return 0;
+ return __is_registered(name);
+}
+
+/**
+ * clocksource_get_clock - Finds a specific clocksource
+ * @name: name of the clocksource to return
+ *
+ * Returns the clocksource if registered, zero otherwise.
+ */
+struct clocksource * clocksource_get_clock(char * name)
+{
+ struct clocksource * ret;
+ unsigned long flags;
+
+ spin_lock_irqsave(&clocksource_lock, flags);
+ ret = __get_clock(name);
+ spin_unlock_irqrestore(&clocksource_lock, flags);
+ return ret;
+}
+
+
+/**
+ * select_clocksource - Finds the best registered clocksource.
+ *
+ * Private function. Must hold clocksource_lock when called.
+ *
+ * Looks through the list of registered clocksources, returning
+ * the one with the highest rating value. If there is a clocksource
+ * name that matches the override string, it returns that clocksource.
+ */
+static struct clocksource *select_clocksource(void)
+{
+ if (!*override_name)
+ return plist_first_entry(&clocksource_list, struct clocksource,
+ list);
+ return get_clock(override_name);
}
/**
* clocksource_register - Used to install new clocksources
* @t: clocksource to be registered
*
- * Returns -EBUSY if registration fails, zero otherwise.
+ * Returns -EBUSY clock is already registered,
+ * Returns -EINVAL if clocksource is invalid,
+ * Return zero otherwise.
*/
int clocksource_register(struct clocksource *c)
{
int ret = 0;
unsigned long flags;
+ if (unlikely(!c))
+ return -EINVAL;
+
spin_lock_irqsave(&clocksource_lock, flags);
- /* check if clocksource is already registered */
- if (is_registered_source(c)) {
- printk("register_clocksource: Cannot register %s. "
+ if (unlikely(!plist_node_empty(&c->list) && __is_registered(c->name))) {
+ printk("register_clocksource: Cannot register %s clocksource. "
"Already registered!", c->name);
ret = -EBUSY;
} else {
- /* register it */
- list_add(&c->list, &clocksource_list);
+ plist_node_init(&c->list, CLOCKSOURCE_RATING(c->rating));
+ plist_add(&c->list, &clocksource_list);
/* scan the registered clocksources, and pick the best one */
next_clocksource = select_clocksource();
}
@@ -157,21 +190,32 @@ int clocksource_register(struct clocksou
EXPORT_SYMBOL(clocksource_register);
/**
- * clocksource_reselect - Rescan list for next clocksource
+ * clocksource_rating_change - Allows dynamic rating changes for register
+ * clocksources.
*
- * A quick helper function to be used if a clocksource changes its
- * rating. Forces the clocksource list to be re-scanned for the best
- * clocksource.
+ * Signals that a clocksource is dynamically changing it's rating.
+ * This could happen if a clocksource becomes more/less stable.
*/
-void clocksource_reselect(void)
+void clocksource_rating_change(struct clocksource *c)
{
unsigned long flags;
+ if (unlikely(plist_node_empty(&c->list)))
+ return;
+
spin_lock_irqsave(&clocksource_lock, flags);
+
+ /*
+ * Re-register the clocksource with it's new rating.
+ */
+ plist_del(&c->list, &clocksource_list);
+ plist_node_init(&c->list, CLOCKSOURCE_RATING(c->rating));
+ plist_add(&c->list, &clocksource_list);
+
next_clocksource = select_clocksource();
spin_unlock_irqrestore(&clocksource_lock, flags);
}
-EXPORT_SYMBOL(clocksource_reselect);
+EXPORT_SYMBOL(clocksource_rating_change);
/**
* sysfs_show_current_clocksources - sysfs interface for current clocksource
@@ -236,16 +280,17 @@ static ssize_t sysfs_override_clocksourc
* @dev: unused
* @buf: char buffer to be filled with clocksource list
*
- * Provides sysfs interface for listing registered clocksources
+ * Provides sysfs interface for listing registered clocksources.
+ * Output in priority sorted order.
*/
static ssize_t
sysfs_show_available_clocksources(struct sys_device *dev, char *buf)
{
- struct list_head *tmp;
+ struct plist_node *tmp;
char *curr = buf;
spin_lock_irq(&clocksource_lock);
- list_for_each(tmp, &clocksource_list) {
+ plist_for_each(tmp, &clocksource_list) {
struct clocksource *src;
src = list_entry(tmp, struct clocksource, list);
--
On Thu, 2006-08-03 at 20:24 -0700, [email protected] wrote:
> plain text document attachment (clocksource_user_api.patch)
> This introduces some new API calls,
>
> - clocksource_get_clock()
> Allows a clock lookup by name.
> - clocksource_rating_change()
> Used by a clocksource to signal a rating change. Replaces
> reselect_clocksource()
>
> I also moved the the clock source list to a plist, which removes some lookup
> overhead in the average case.
>
> Signed-Off-By: Daniel Walker <[email protected]>
>
> ---
> arch/i386/kernel/tsc.c | 2
> include/linux/clocksource.h | 45 ++++++++++++-
> kernel/time/clocksource.c | 149 ++++++++++++++++++++++++++++----------------
> 3 files changed, 139 insertions(+), 57 deletions(-)
>
> Index: linux-2.6.17/arch/i386/kernel/tsc.c
> ===================================================================
> --- linux-2.6.17.orig/arch/i386/kernel/tsc.c
> +++ linux-2.6.17/arch/i386/kernel/tsc.c
> @@ -351,7 +351,7 @@ static int tsc_update_callback(void)
> /* check to see if we should switch to the safe clocksource: */
> if (clocksource_tsc.rating != 50 && check_tsc_unstable()) {
> clocksource_tsc.rating = 50;
> - clocksource_reselect();
> + clocksource_rating_change(&clocksource_tsc);
> change = 1;
> }
>
> Index: linux-2.6.17/include/linux/clocksource.h
> ===================================================================
> --- linux-2.6.17.orig/include/linux/clocksource.h
> +++ linux-2.6.17/include/linux/clocksource.h
> @@ -12,12 +12,20 @@
> #include <linux/timex.h>
> #include <linux/time.h>
> #include <linux/list.h>
> +#include <linux/plist.h>
> +#include <linux/sysdev.h>
> #include <asm/div64.h>
> #include <asm/io.h>
>
> /* clocksource cycle base type */
> typedef u64 cycle_t;
>
> +/*
> + * This is the only generic clock, it should be used
> + * for early initialization.
> + */
> +extern struct clocksource clocksource_jiffies;
> +
> /**
> * struct clocksource - hardware abstraction for a free running counter
> * Provides mostly state-free accessors to the underlying hardware.
> @@ -51,7 +59,7 @@ typedef u64 cycle_t;
> */
> struct clocksource {
> char *name;
> - struct list_head list;
> + struct plist_node list;
> int rating;
> cycle_t (*read)(void);
> cycle_t mask;
> @@ -148,6 +156,25 @@ static inline s64 cyc2ns(struct clocksou
> }
>
> /**
> + * ns2cyc - converts nanoseconds to clocksource cycles
> + * @cs: Pointer to clocksource
> + * @ns: Nanoseconds
> + *
> + * Uses the clocksource to convert nanoseconds back to cycles.
> + *
> + * XXX - This could use some mult_lxl_ll() asm optimization
> + */
> +static inline cycle_t ns2cyc(struct clocksource *cs, s64 ns)
> +{
> + u64 ret = ns;
> +
> + ret <<= cs->shift;
> + do_div(ret, cs->mult);
> +
> + return ret;
> +}
> +
> +/**
> * clocksource_calculate_interval - Calculates a clocksource interval struct
> *
> * @c: Pointer to clocksource.
> @@ -178,8 +205,18 @@ static inline void clocksource_calculate
>
>
> /* used to install a new clocksource */
> -int clocksource_register(struct clocksource*);
> -void clocksource_reselect(void);
> -struct clocksource* clocksource_get_next(void);
> +extern int clocksource_register(struct clocksource*);
> +extern void clocksource_rating_change(struct clocksource*);
> +extern struct clocksource * clocksource_get_clock(char*);
>
> +/**
> + * clocksource_get_best_clock - Finds highest rated clocksource
> + *
> + * Returns the highest rated clocksource. If none are register the
> + * jiffies clock is returned.
> + */
> +static inline struct clocksource * clocksource_get_best_clock(void)
> +{
> + return clocksource_get_clock(NULL);
> +}
> #endif /* _LINUX_CLOCKSOURCE_H */
> Index: linux-2.6.17/kernel/time/clocksource.c
> ===================================================================
> --- linux-2.6.17.orig/kernel/time/clocksource.c
> +++ linux-2.6.17/kernel/time/clocksource.c
> @@ -32,13 +32,18 @@
> /* XXX - Would like a better way for initializing curr_clocksource */
> extern struct clocksource clocksource_jiffies;
>
> +/*
> + * Internally used to invert the rating, so lower is better.
> + */
> +#define CLOCKSOURCE_RATING(x) (INT_MAX-x)
Since this is used for the plist bits, could it get a more explicit
name?
> /*[Clocksource internal variables]---------
> * curr_clocksource:
> * currently selected clocksource. Initialized to clocksource_jiffies.
> * next_clocksource:
> * pending next selected clocksource.
> * clocksource_list:
> - * linked list with the registered clocksources
> + * priority list with the registered clocksources
> * clocksource_lock:
> * protects manipulations to curr_clocksource and next_clocksource
> * and the clocksource_list
> @@ -47,7 +52,8 @@ extern struct clocksource clocksource_ji
> */
> static struct clocksource *curr_clocksource = &clocksource_jiffies;
> static struct clocksource *next_clocksource;
> -static LIST_HEAD(clocksource_list);
> +static struct plist_head clocksource_list =
> + PLIST_HEAD_INIT(clocksource_list, clocksource_lock);
> static DEFINE_SPINLOCK(clocksource_lock);
> static char override_name[32];
>
> @@ -70,84 +76,111 @@ struct clocksource *clocksource_get_next
> }
>
> /**
> - * select_clocksource - Finds the best registered clocksource.
> + * __is_registered - Returns a clocksource if it's registered
> + * @name: name of the clocksource to return
> *
> * Private function. Must hold clocksource_lock when called.
> *
> - * Looks through the list of registered clocksources, returning
> - * the one with the highest rating value. If there is a clocksource
> - * name that matches the override string, it returns that clocksource.
> + * Returns the clocksource if registered, zero otherwise.
> + * If no clocksources are registered the jiffies clock is
> + * returned.
> */
> -static struct clocksource *select_clocksource(void)
> +static struct clocksource * __is_registered(char * name)
> {
> - struct clocksource *best = NULL;
> - struct list_head *tmp;
> + struct plist_node *tmp;
>
> - list_for_each(tmp, &clocksource_list) {
> + plist_for_each(tmp, &clocksource_list) {
> struct clocksource *src;
>
> src = list_entry(tmp, struct clocksource, list);
> - if (!best)
> - best = src;
> -
> - /* check for override: */
> - if (strlen(src->name) == strlen(override_name) &&
> - !strcmp(src->name, override_name)) {
> - best = src;
> - break;
> - }
> - /* pick the highest rating: */
> - if (src->rating > best->rating)
> - best = src;
> + if (!strcmp(src->name, name))
> + return src;
> }
>
> - return best;
> + return 0;
> }
>
> /**
> - * is_registered_source - Checks if clocksource is registered
> - * @c: pointer to a clocksource
> + * __get_clock - Finds a specific clocksource
> + * @name: name of the clocksource to return
> *
> - * Private helper function. Must hold clocksource_lock when called.
> + * Private function. Must hold clocksource_lock when called.
> *
> - * Returns one if the clocksource is already registered, zero otherwise.
> + * Returns the clocksource if registered, zero otherwise.
> + * If the @name is null the highest rated clock is returned.
> */
> -static int is_registered_source(struct clocksource *c)
> +static inline struct clocksource * __get_clock(char * name)
> {
> - int len = strlen(c->name);
> - struct list_head *tmp;
>
> - list_for_each(tmp, &clocksource_list) {
> - struct clocksource *src;
> + if (unlikely(plist_head_empty(&clocksource_list)))
> + return &clocksource_jiffies;
>
> - src = list_entry(tmp, struct clocksource, list);
> - if (strlen(src->name) == len && !strcmp(src->name, c->name))
> - return 1;
> - }
> + if (!name)
> + return plist_first_entry(&clocksource_list, struct clocksource,
> + list);
>
> - return 0;
> + return __is_registered(name);
> +}
> +
> +/**
> + * clocksource_get_clock - Finds a specific clocksource
> + * @name: name of the clocksource to return
> + *
> + * Returns the clocksource if registered, zero otherwise.
> + */
> +struct clocksource * clocksource_get_clock(char * name)
> +{
> + struct clocksource * ret;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&clocksource_lock, flags);
> + ret = __get_clock(name);
> + spin_unlock_irqrestore(&clocksource_lock, flags);
> + return ret;
> +}
> +
> +
> +/**
> + * select_clocksource - Finds the best registered clocksource.
> + *
> + * Private function. Must hold clocksource_lock when called.
> + *
> + * Looks through the list of registered clocksources, returning
> + * the one with the highest rating value. If there is a clocksource
> + * name that matches the override string, it returns that clocksource.
> + */
> +static struct clocksource *select_clocksource(void)
> +{
> + if (!*override_name)
> + return plist_first_entry(&clocksource_list, struct clocksource,
> + list);
> + return get_clock(override_name);
> }
This all looks good.
> /**
> * clocksource_register - Used to install new clocksources
> * @t: clocksource to be registered
> *
> - * Returns -EBUSY if registration fails, zero otherwise.
> + * Returns -EBUSY clock is already registered,
> + * Returns -EINVAL if clocksource is invalid,
> + * Return zero otherwise.
> */
> int clocksource_register(struct clocksource *c)
> {
> int ret = 0;
> unsigned long flags;
>
> + if (unlikely(!c))
> + return -EINVAL;
> +
I'm not sure I understand the need for this. Is it really likely someone
would pass NULL to clocksource_register()?
> spin_lock_irqsave(&clocksource_lock, flags);
> - /* check if clocksource is already registered */
> - if (is_registered_source(c)) {
> - printk("register_clocksource: Cannot register %s. "
> + if (unlikely(!plist_node_empty(&c->list) && __is_registered(c->name))) {
> + printk("register_clocksource: Cannot register %s clocksource. "
> "Already registered!", c->name);
> ret = -EBUSY;
> } else {
> - /* register it */
> - list_add(&c->list, &clocksource_list);
> + plist_node_init(&c->list, CLOCKSOURCE_RATING(c->rating));
> + plist_add(&c->list, &clocksource_list);
> /* scan the registered clocksources, and pick the best one */
> next_clocksource = select_clocksource();
> }
> @@ -157,21 +190,32 @@ int clocksource_register(struct clocksou
> EXPORT_SYMBOL(clocksource_register);
>
> /**
> - * clocksource_reselect - Rescan list for next clocksource
> + * clocksource_rating_change - Allows dynamic rating changes for register
> + * clocksources.
> *
> - * A quick helper function to be used if a clocksource changes its
> - * rating. Forces the clocksource list to be re-scanned for the best
> - * clocksource.
> + * Signals that a clocksource is dynamically changing it's rating.
> + * This could happen if a clocksource becomes more/less stable.
> */
> -void clocksource_reselect(void)
> +void clocksource_rating_change(struct clocksource *c)
> {
> unsigned long flags;
>
> + if (unlikely(plist_node_empty(&c->list)))
> + return;
> +
> spin_lock_irqsave(&clocksource_lock, flags);
> +
> + /*
> + * Re-register the clocksource with it's new rating.
> + */
> + plist_del(&c->list, &clocksource_list);
> + plist_node_init(&c->list, CLOCKSOURCE_RATING(c->rating));
> + plist_add(&c->list, &clocksource_list);
> +
> next_clocksource = select_clocksource();
> spin_unlock_irqrestore(&clocksource_lock, flags);
> }
> -EXPORT_SYMBOL(clocksource_reselect);
> +EXPORT_SYMBOL(clocksource_rating_change);
>
> /**
> * sysfs_show_current_clocksources - sysfs interface for current clocksource
> @@ -236,16 +280,17 @@ static ssize_t sysfs_override_clocksourc
> * @dev: unused
> * @buf: char buffer to be filled with clocksource list
> *
> - * Provides sysfs interface for listing registered clocksources
> + * Provides sysfs interface for listing registered clocksources.
> + * Output in priority sorted order.
> */
> static ssize_t
> sysfs_show_available_clocksources(struct sys_device *dev, char *buf)
> {
> - struct list_head *tmp;
> + struct plist_node *tmp;
> char *curr = buf;
>
> spin_lock_irq(&clocksource_lock);
> - list_for_each(tmp, &clocksource_list) {
> + plist_for_each(tmp, &clocksource_list) {
> struct clocksource *src;
>
> src = list_entry(tmp, struct clocksource, list);
>
No real objections to this one.
thanks
-john
> > +/*
> > + * Internally used to invert the rating, so lower is better.
> > + */
> > +#define CLOCKSOURCE_RATING(x) (INT_MAX-x)
>
> Since this is used for the plist bits, could it get a more explicit
> name?
Sure, like CLOCKSOURCE_INVERT_RATING()
> > int clocksource_register(struct clocksource *c)
> > {
> > int ret = 0;
> > unsigned long flags;
> >
> > + if (unlikely(!c))
> > + return -EINVAL;
> > +
>
> I'm not sure I understand the need for this. Is it really likely someone
> would pass NULL to clocksource_register()?
Not likely, I was just covering all possibilities.. It might be better
as a BUG_ON() actually.
Daniel
On Fri, 2006-08-04 at 12:28 -0700, Daniel Walker wrote:
> > > int clocksource_register(struct clocksource *c)
> > > {
> > > int ret = 0;
> > > unsigned long flags;
> > >
> > > + if (unlikely(!c))
> > > + return -EINVAL;
> > > +
> >
> > I'm not sure I understand the need for this. Is it really likely someone
> > would pass NULL to clocksource_register()?
>
> Not likely, I was just covering all possibilities.. It might be better
> as a BUG_ON() actually.
BUG_ON is the only thing, which can be correct here. Registering a NULL
clocksource simply is a bug, but even the BUG_ON is overkill here.
tglx