lkml.org 
[lkml]   [2009]   [Jul]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC][patch 2/5] cleanup clocksource selection
    From
    Date
    On Tue, 2009-07-21 at 21:17 +0200, Martin Schwidefsky wrote:
    > plain text document attachment (clocksource-queue.diff)
    > From: Martin Schwidefsky <schwidefsky@de.ibm.com>
    >
    > Introduce clocksource_dequeue & clocksource_update and move spinlock
    > calls. clocksource_update does nothing for GENERIC_TIME=n since
    > change_clocksource does nothing as well.
    >
    > Cc: Ingo Molnar <mingo@elte.hu>
    > Cc: Thomas Gleixner <tglx@linutronix.de>
    > Cc: john stultz <johnstul@us.ibm.com>
    > Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
    > ---
    > kernel/time/clocksource.c | 123 ++++++++++++++++++++++++++--------------------
    > 1 file changed, 70 insertions(+), 53 deletions(-)


    >From a very quick skim, this patch looks structurally ok to me.

    thanks
    -john


    > Index: linux-2.6/kernel/time/clocksource.c
    > ===================================================================
    > --- linux-2.6.orig/kernel/time/clocksource.c
    > +++ linux-2.6/kernel/time/clocksource.c
    > @@ -341,46 +341,25 @@ struct clocksource *clocksource_get_next
    > return curr_clocksource;
    > }
    >
    > -/**
    > - * select_clocksource - Selects the best registered clocksource.
    > - *
    > - * Private function. Must hold clocksource_lock when called.
    > - *
    > - * Select the clocksource with the best rating, or the clocksource,
    > - * which is selected by userspace override.
    > - */
    > -static struct clocksource *select_clocksource(void)
    > -{
    > - struct clocksource *next;
    > -
    > - if (list_empty(&clocksource_list))
    > - return NULL;
    > -
    > - if (clocksource_override)
    > - next = clocksource_override;
    > - else
    > - next = list_entry(clocksource_list.next, struct clocksource,
    > - list);
    > -
    > - if (next == curr_clocksource)
    > - return NULL;
    > -
    > - return next;
    > -}
    > -
    > /*
    > * Enqueue the clocksource sorted by rating
    > */
    > static int clocksource_enqueue(struct clocksource *c)
    > {
    > struct list_head *tmp, *entry = &clocksource_list;
    > + unsigned long flags;
    > + int rc;
    >
    > + spin_lock_irqsave(&clocksource_lock, flags);
    > + rc = 0;
    > list_for_each(tmp, &clocksource_list) {
    > struct clocksource *cs;
    >
    > cs = list_entry(tmp, struct clocksource, list);
    > - if (cs == c)
    > - return -EBUSY;
    > + if (cs == c) {
    > + rc = -EBUSY;
    > + goto out;
    > + }
    > /* Keep track of the place, where to insert */
    > if (cs->rating >= c->rating)
    > entry = tmp;
    > @@ -390,8 +369,23 @@ static int clocksource_enqueue(struct cl
    > if (strlen(c->name) == strlen(override_name) &&
    > !strcmp(c->name, override_name))
    > clocksource_override = c;
    > +out:
    > + spin_unlock_irqrestore(&clocksource_lock, flags);
    > + return rc;
    > +}
    > +
    > +/*
    > + * Dequeue a clocksource
    > + */
    > +static void clocksource_dequeue(struct clocksource *cs)
    > +{
    > + unsigned long flags;
    >
    > - return 0;
    > + spin_lock_irqsave(&clocksource_lock, flags);
    > + list_del(&cs->list);
    > + if (clocksource_override == cs)
    > + clocksource_override = NULL;
    > + spin_unlock_irqrestore(&clocksource_lock, flags);
    > }
    >
    > /**
    > @@ -539,6 +533,25 @@ void clocksource_forward_now(void)
    > }
    >
    > /**
    > + * select_clocksource - Selects the best registered clocksource.
    > + *
    > + * Private function. Must hold clocksource_lock when called.
    > + *
    > + * Select the clocksource with the best rating, or the clocksource,
    > + * which is selected by userspace override.
    > + */
    > +static struct clocksource *select_clocksource(void)
    > +{
    > + if (list_empty(&clocksource_list))
    > + return NULL;
    > +
    > + if (clocksource_override)
    > + return clocksource_override;
    > +
    > + return list_entry(clocksource_list.next, struct clocksource, list);
    > +}
    > +
    > +/**
    > * change_clocksource - Swaps clocksources if a new one is available
    > *
    > * Accumulates current time interval and initializes new clocksource
    > @@ -577,6 +590,23 @@ void change_clocksource(void)
    > clock->name);
    > */
    > }
    > +
    > +/**
    > + * clocksource_update - Check if a better clocksource is available
    > + */
    > +static void clocksource_update(void)
    > +{
    > + struct clocksource *new;
    > + unsigned long flags;
    > +
    > + spin_lock_irqsave(&clocksource_lock, flags);
    > + new = select_clocksource();
    > + if (new)
    > + next_clocksource = new;
    > + spin_unlock_irqrestore(&clocksource_lock, flags);
    > +}
    > +#else /* CONFIG_GENERIC_TIME */
    > +static inline void clocksource_update(void) { }
    > #endif
    >
    > /**
    > @@ -587,16 +617,13 @@ void change_clocksource(void)
    > */
    > int clocksource_register(struct clocksource *c)
    > {
    > - unsigned long flags;
    > int ret;
    >
    > - spin_lock_irqsave(&clocksource_lock, flags);
    > ret = clocksource_enqueue(c);
    > - if (!ret)
    > - next_clocksource = select_clocksource();
    > - spin_unlock_irqrestore(&clocksource_lock, flags);
    > - if (!ret)
    > + if (!ret) {
    > + clocksource_update();
    > clocksource_check_watchdog(c);
    > + }
    > return ret;
    > }
    > EXPORT_SYMBOL(clocksource_register);
    > @@ -607,14 +634,10 @@ EXPORT_SYMBOL(clocksource_register);
    > */
    > void clocksource_change_rating(struct clocksource *cs, int rating)
    > {
    > - unsigned long flags;
    > -
    > - spin_lock_irqsave(&clocksource_lock, flags);
    > - list_del(&cs->list);
    > + clocksource_dequeue(cs);
    > cs->rating = rating;
    > clocksource_enqueue(cs);
    > - next_clocksource = select_clocksource();
    > - spin_unlock_irqrestore(&clocksource_lock, flags);
    > + clocksource_update();
    > }
    >
    > /**
    > @@ -622,14 +645,8 @@ void clocksource_change_rating(struct cl
    > */
    > void clocksource_unregister(struct clocksource *cs)
    > {
    > - unsigned long flags;
    > -
    > - spin_lock_irqsave(&clocksource_lock, flags);
    > - list_del(&cs->list);
    > - if (clocksource_override == cs)
    > - clocksource_override = NULL;
    > - next_clocksource = select_clocksource();
    > - spin_unlock_irqrestore(&clocksource_lock, flags);
    > + clocksource_dequeue(cs);
    > + clocksource_update();
    > }
    >
    > /**
    > @@ -721,13 +738,13 @@ static ssize_t sysfs_override_clocksourc
    > }
    >
    > /* Reselect, when the override name has changed */
    > - if (ovr != clocksource_override) {
    > + if (ovr != clocksource_override)
    > clocksource_override = ovr;
    > - next_clocksource = select_clocksource();
    > - }
    >
    > spin_unlock_irq(&clocksource_lock);
    >
    > + clocksource_update();
    > +
    > return ret;
    > }
    >
    >



    \
     
     \ /
      Last update: 2009-07-22 00:09    [W:4.417 / U:0.124 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site