lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Tue, 21 Jul 2009 15:07:49 -0700
From:	john stultz <johnstul@...ibm.com>
To:	Martin Schwidefsky <schwidefsky@...ibm.com>
Cc:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [RFC][patch 2/5] cleanup clocksource selection

On Tue, 2009-07-21 at 21:17 +0200, Martin Schwidefsky wrote:
> plain text document attachment (clocksource-queue.diff)
> From: Martin Schwidefsky <schwidefsky@...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@...e.hu>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: john stultz <johnstul@...ibm.com>
> Signed-off-by: Martin Schwidefsky <schwidefsky@...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;
>  }
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ