[patch 00/15] clocksource/events: Overhaul (un)registration

[patch 00/15] clocksource/events: Overhaul (un)registration

[Thomas Gleixner]

The clocksource_unregister() interface is broken. A clocksource can be
removed unconditionally even if there is no fallback clocksource
available. clocksources in a module can be removed as well. Aside of
that the registration part lacks a few sanity checks as well.

The clockevents layer does not allow unregistration, but for
reconfigurable hardware this is a must have feature.

The following patch series fixes the clocksource part and adds
unregister support for clockevent devices.

Thanks,

	tglx
---
 drivers/clocksource/dw_apb_timer.c |   12 -
 include/linux/clockchips.h         |    4 
 include/linux/clocksource.h        |    7 
 include/linux/dw_apb_timer.h       |    1 
 kernel/time/clockevents.c          |  271 ++++++++++++++++++++++++++++++++-----
 kernel/time/clocksource.c          |  190 +++++++++++++++++++------
 kernel/time/tick-broadcast.c       |   33 +++-
 kernel/time/tick-common.c          |  189 +++++++++----------------
 kernel/time/tick-internal.h        |   17 +-
 kernel/time/timekeeping.c          |   20 +-
 10 files changed, 514 insertions(+), 230 deletions(-)

[patch 02/15] clocksource: Always verify highres capability

[Thomas Gleixner]

[-- Attachment #1: clocksource-verify-highres-capability.patch --]
[-- Type: text/plain, Size: 2401 bytes --]

If a clocksource has a (wrong) high rating, but can't be used as a
timebase for oneshot tick mode, it is unconditionally selected even
when the system is already in oneshot tick mode. This causes full
system failure.

Verify the clocksource selection against the oneshot mode.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/clocksource.c |   31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

Index: tip/kernel/time/clocksource.c
===================================================================
--- tip.orig/kernel/time/clocksource.c
+++ tip/kernel/time/clocksource.c
@@ -553,6 +553,26 @@ static u64 clocksource_max_deferment(str
 
 #ifndef CONFIG_ARCH_USES_GETTIMEOFFSET
 
+static struct clocksource *clocksource_find_best(bool oneshot)
+{
+	struct clocksource *cs;
+
+	if (!finished_booting || list_empty(&clocksource_list))
+		return NULL;
+
+	/*
+	 * We pick the clocksource with the highest rating. If oneshot
+	 * mode is active, we pick the highres valid clocksource with
+	 * the best rating.
+	 */
+	list_for_each_entry(cs, &clocksource_list, list) {
+		if (oneshot && !(cs->flags & CLOCK_SOURCE_VALID_FOR_HRES))
+			continue;
+		return cs;
+	}
+	return NULL;
+}
+
 /**
  * clocksource_select - Select the best clocksource available
  *
@@ -563,12 +583,14 @@ static u64 clocksource_max_deferment(str
  */
 static void clocksource_select(void)
 {
+	bool oneshot = tick_oneshot_mode_active();
 	struct clocksource *best, *cs;
 
-	if (!finished_booting || list_empty(&clocksource_list))
+	/* Find the best suitable clocksource */
+	best = clocksource_find_best(oneshot);
+	if (!best)
 		return;
-	/* First clocksource on the list has the best rating. */
-	best = list_first_entry(&clocksource_list, struct clocksource, list);
+
 	/* Check for the override clocksource. */
 	list_for_each_entry(cs, &clocksource_list, list) {
 		if (strcmp(cs->name, override_name) != 0)
@@ -578,8 +600,7 @@ static void clocksource_select(void)
 		 * capable clocksource if the tick code is in oneshot
 		 * mode (highres or nohz)
 		 */
-		if (!(cs->flags & CLOCK_SOURCE_VALID_FOR_HRES) &&
-		    tick_oneshot_mode_active()) {
+		if (!(cs->flags & CLOCK_SOURCE_VALID_FOR_HRES) && oneshot) {
 			/* Override clocksource cannot be used. */
 			printk(KERN_WARNING "Override clocksource %s is not "
 			       "HRT compatible. Cannot switch while in "

Re: [patch 02/15] clocksource: Always verify highres capability

[John Stultz]

On 04/25/2013 01:31 PM, Thomas Gleixner wrote:
> If a clocksource has a (wrong) high rating, but can't be used as a
> timebase for oneshot tick mode, it is unconditionally selected even
> when the system is already in oneshot tick mode. This causes full
> system failure.
>
> Verify the clocksource selection against the oneshot mode.
>
> Signed-off-by: Thomas Gleixner<tglx@linutronix.de>

Acked-by: John Stultz <john.stultz@linaro.org>

thanks
-john

[tip:timers/core] clocksource: Always verify highres capability

[tip-bot for Thomas Gleixner]

Commit-ID:  5d33b883aed81c6fbcd09c6f7c3619eee850a7e2
Gitweb:     http://git.kernel.org/tip/5d33b883aed81c6fbcd09c6f7c3619eee850a7e2
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 25 Apr 2013 20:31:43 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 16 May 2013 11:09:14 +0200

clocksource: Always verify highres capability

If a clocksource has a (wrong) high rating, but can't be used as a
timebase for oneshot tick mode, it is unconditionally selected even
when the system is already in oneshot tick mode. This causes full
system failure.

Verify the clocksource selection against the oneshot mode.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: John Stultz <john.stultz@linaro.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Link: http://lkml.kernel.org/r/20130425143435.635040849@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/clocksource.c | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index c958338..dda5c71 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -553,6 +553,26 @@ static u64 clocksource_max_deferment(struct clocksource *cs)
 
 #ifndef CONFIG_ARCH_USES_GETTIMEOFFSET
 
+static struct clocksource *clocksource_find_best(bool oneshot)
+{
+	struct clocksource *cs;
+
+	if (!finished_booting || list_empty(&clocksource_list))
+		return NULL;
+
+	/*
+	 * We pick the clocksource with the highest rating. If oneshot
+	 * mode is active, we pick the highres valid clocksource with
+	 * the best rating.
+	 */
+	list_for_each_entry(cs, &clocksource_list, list) {
+		if (oneshot && !(cs->flags & CLOCK_SOURCE_VALID_FOR_HRES))
+			continue;
+		return cs;
+	}
+	return NULL;
+}
+
 /**
  * clocksource_select - Select the best clocksource available
  *
@@ -563,12 +583,14 @@ static u64 clocksource_max_deferment(struct clocksource *cs)
  */
 static void clocksource_select(void)
 {
+	bool oneshot = tick_oneshot_mode_active();
 	struct clocksource *best, *cs;
 
-	if (!finished_booting || list_empty(&clocksource_list))
+	/* Find the best suitable clocksource */
+	best = clocksource_find_best(oneshot);
+	if (!best)
 		return;
-	/* First clocksource on the list has the best rating. */
-	best = list_first_entry(&clocksource_list, struct clocksource, list);
+
 	/* Check for the override clocksource. */
 	list_for_each_entry(cs, &clocksource_list, list) {
 		if (strcmp(cs->name, override_name) != 0)
@@ -578,8 +600,7 @@ static void clocksource_select(void)
 		 * capable clocksource if the tick code is in oneshot
 		 * mode (highres or nohz)
 		 */
-		if (!(cs->flags & CLOCK_SOURCE_VALID_FOR_HRES) &&
-		    tick_oneshot_mode_active()) {
+		if (!(cs->flags & CLOCK_SOURCE_VALID_FOR_HRES) && oneshot) {
 			/* Override clocksource cannot be used. */
 			printk(KERN_WARNING "Override clocksource %s is not "
 			       "HRT compatible. Cannot switch while in "

[patch 01/15] clocksource: apb_timer: Remove unsused function

[Thomas Gleixner]

[-- Attachment #1: clocksource-apb-timer-remove-unsused-function.patch --]
[-- Type: text/plain, Size: 1436 bytes --]

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Jamie Iles <jamie@jamieiles.com>
---
 drivers/clocksource/dw_apb_timer.c |   12 ------------
 include/linux/dw_apb_timer.h       |    1 -
 2 files changed, 13 deletions(-)

Index: tip/drivers/clocksource/dw_apb_timer.c
===================================================================
--- tip.orig/drivers/clocksource/dw_apb_timer.c
+++ tip/drivers/clocksource/dw_apb_timer.c
@@ -387,15 +387,3 @@ cycle_t dw_apb_clocksource_read(struct d
 {
 	return (cycle_t)~apbt_readl(&dw_cs->timer, APBTMR_N_CURRENT_VALUE);
 }
-
-/**
- * dw_apb_clocksource_unregister() - unregister and free a clocksource.
- *
- * @dw_cs:	The clocksource to unregister/free.
- */
-void dw_apb_clocksource_unregister(struct dw_apb_clocksource *dw_cs)
-{
-	clocksource_unregister(&dw_cs->cs);
-
-	kfree(dw_cs);
-}
Index: tip/include/linux/dw_apb_timer.h
===================================================================
--- tip.orig/include/linux/dw_apb_timer.h
+++ tip/include/linux/dw_apb_timer.h
@@ -51,7 +51,6 @@ dw_apb_clocksource_init(unsigned rating,
 void dw_apb_clocksource_register(struct dw_apb_clocksource *dw_cs);
 void dw_apb_clocksource_start(struct dw_apb_clocksource *dw_cs);
 cycle_t dw_apb_clocksource_read(struct dw_apb_clocksource *dw_cs);
-void dw_apb_clocksource_unregister(struct dw_apb_clocksource *dw_cs);
 
 extern void dw_apb_timer_init(void);
 #endif /* __DW_APB_TIMER_H__ */

Re: [patch 01/15] clocksource: apb_timer: Remove unsused function

[Jamie Iles]

Hi Thomas,

On Thu, Apr 25, 2013 at 08:31:43PM -0000, Thomas Gleixner wrote:
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Jamie Iles <jamie@jamieiles.com>

Looks good, thanks for fixing this.

Acked-by: Jamie Iles <jamie@jamieiles.com>

Re: [patch 01/15] clocksource: apb_timer: Remove unsused function

[John Stultz]

On 04/25/2013 01:31 PM, Thomas Gleixner wrote:
> Signed-off-by: Thomas Gleixner<tglx@linutronix.de>
> Cc: Jamie Iles<jamie@jamieiles.com>

Acked-by: John Stultz <john.stultz@linaro.org>

thanks
-john


> ---
>   drivers/clocksource/dw_apb_timer.c |   12 ------------
>   include/linux/dw_apb_timer.h       |    1 -
>   2 files changed, 13 deletions(-)
>
> Index: tip/drivers/clocksource/dw_apb_timer.c
> ===================================================================
> --- tip.orig/drivers/clocksource/dw_apb_timer.c
> +++ tip/drivers/clocksource/dw_apb_timer.c
> @@ -387,15 +387,3 @@ cycle_t dw_apb_clocksource_read(struct d
>   {
>   	return (cycle_t)~apbt_readl(&dw_cs->timer, APBTMR_N_CURRENT_VALUE);
>   }
> -
> -/**
> - * dw_apb_clocksource_unregister() - unregister and free a clocksource.
> - *
> - * @dw_cs:	The clocksource to unregister/free.
> - */
> -void dw_apb_clocksource_unregister(struct dw_apb_clocksource *dw_cs)
> -{
> -	clocksource_unregister(&dw_cs->cs);
> -
> -	kfree(dw_cs);
> -}
> Index: tip/include/linux/dw_apb_timer.h
> ===================================================================
> --- tip.orig/include/linux/dw_apb_timer.h
> +++ tip/include/linux/dw_apb_timer.h
> @@ -51,7 +51,6 @@ dw_apb_clocksource_init(unsigned rating,
>   void dw_apb_clocksource_register(struct dw_apb_clocksource *dw_cs);
>   void dw_apb_clocksource_start(struct dw_apb_clocksource *dw_cs);
>   cycle_t dw_apb_clocksource_read(struct dw_apb_clocksource *dw_cs);
> -void dw_apb_clocksource_unregister(struct dw_apb_clocksource *dw_cs);
>   
>   extern void dw_apb_timer_init(void);
>   #endif /* __DW_APB_TIMER_H__ */
>
>

[tip:timers/core] clocksource: apb_timer: Remove unsused function

[tip-bot for Thomas Gleixner]

Commit-ID:  fc1f7d5606487ae28d6c84e95401952927d7379e
Gitweb:     http://git.kernel.org/tip/fc1f7d5606487ae28d6c84e95401952927d7379e
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 25 Apr 2013 20:31:43 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 16 May 2013 11:09:13 +0200

clocksource: apb_timer: Remove unsused function

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: John Stultz <john.stultz@linaro.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Acked-by: Jamie Iles <jamie@jamieiles.com>
Link: http://lkml.kernel.org/r/20130425143435.558006195@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/clocksource/dw_apb_timer.c | 12 ------------
 include/linux/dw_apb_timer.h       |  1 -
 2 files changed, 13 deletions(-)

diff --git a/drivers/clocksource/dw_apb_timer.c b/drivers/clocksource/dw_apb_timer.c
index 8c2a35f..e54ca10 100644
--- a/drivers/clocksource/dw_apb_timer.c
+++ b/drivers/clocksource/dw_apb_timer.c
@@ -387,15 +387,3 @@ cycle_t dw_apb_clocksource_read(struct dw_apb_clocksource *dw_cs)
 {
 	return (cycle_t)~apbt_readl(&dw_cs->timer, APBTMR_N_CURRENT_VALUE);
 }
-
-/**
- * dw_apb_clocksource_unregister() - unregister and free a clocksource.
- *
- * @dw_cs:	The clocksource to unregister/free.
- */
-void dw_apb_clocksource_unregister(struct dw_apb_clocksource *dw_cs)
-{
-	clocksource_unregister(&dw_cs->cs);
-
-	kfree(dw_cs);
-}
diff --git a/include/linux/dw_apb_timer.h b/include/linux/dw_apb_timer.h
index dd755ce..b1cd959 100644
--- a/include/linux/dw_apb_timer.h
+++ b/include/linux/dw_apb_timer.h
@@ -51,7 +51,6 @@ dw_apb_clocksource_init(unsigned rating, const char *name, void __iomem *base,
 void dw_apb_clocksource_register(struct dw_apb_clocksource *dw_cs);
 void dw_apb_clocksource_start(struct dw_apb_clocksource *dw_cs);
 cycle_t dw_apb_clocksource_read(struct dw_apb_clocksource *dw_cs);
-void dw_apb_clocksource_unregister(struct dw_apb_clocksource *dw_cs);
 
 extern void dw_apb_timer_init(void);
 #endif /* __DW_APB_TIMER_H__ */

[patch 04/15] clocksource: Add module refcount

[Thomas Gleixner]

[-- Attachment #1: clocksource-add-module-refcount.patch --]
[-- Type: text/plain, Size: 1875 bytes --]

Add a module refcount, so the current clocksource cannot be removed
unconditionally.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/clocksource.h |    3 +++
 kernel/time/timekeeping.c   |   15 ++++++++++-----
 2 files changed, 13 insertions(+), 5 deletions(-)

Index: tip/include/linux/clocksource.h
===================================================================
--- tip.orig/include/linux/clocksource.h
+++ tip/include/linux/clocksource.h
@@ -21,6 +21,7 @@
 /* clocksource cycle base type */
 typedef u64 cycle_t;
 struct clocksource;
+struct module;
 
 #ifdef CONFIG_ARCH_CLOCKSOURCE_DATA
 #include <asm/clocksource.h>
@@ -162,6 +163,7 @@ extern u64 timecounter_cyc2time(struct t
  * @suspend:		suspend function for the clocksource, if necessary
  * @resume:		resume function for the clocksource, if necessary
  * @cycle_last:		most recent cycle counter value seen by ::read()
+ * @owner:		module reference
  */
 struct clocksource {
 	/*
@@ -195,6 +197,7 @@ struct clocksource {
 	cycle_t cs_last;
 	cycle_t wd_last;
 #endif
+	struct module *owner;
 } ____cacheline_aligned;
 
 /*
Index: tip/kernel/time/timekeeping.c
===================================================================
--- tip.orig/kernel/time/timekeeping.c
+++ tip/kernel/time/timekeeping.c
@@ -627,11 +627,16 @@ static int change_clocksource(void *data
 	write_seqcount_begin(&timekeeper_seq);
 
 	timekeeping_forward_now(tk);
-	if (!new->enable || new->enable(new) == 0) {
-		old = tk->clock;
-		tk_setup_internals(tk, new);
-		if (old->disable)
-			old->disable(old);
+	if (try_module_get(new->owner)) {
+		if (!new->enable || new->enable(new) == 0) {
+			old = tk->clock;
+			tk_setup_internals(tk, new);
+			if (old->disable)
+				old->disable(old);
+			module_put(old->owner);
+		} else {
+			module_put(new->owner);
+		}
 	}
 	timekeeping_update(tk, true, true);
 

Re: [patch 04/15] clocksource: Add module refcount

[John Stultz]

On 04/25/2013 01:31 PM, Thomas Gleixner wrote:
> Add a module refcount, so the current clocksource cannot be removed
> unconditionally.
>
> Signed-off-by: Thomas Gleixner<tglx@linutronix.de>
> ---
>   include/linux/clocksource.h |    3 +++
>   kernel/time/timekeeping.c   |   15 ++++++++++-----
>   2 files changed, 13 insertions(+), 5 deletions(-)
>
> Index: tip/include/linux/clocksource.h
> ===================================================================
> --- tip.orig/include/linux/clocksource.h
> +++ tip/include/linux/clocksource.h
> @@ -21,6 +21,7 @@
>   /* clocksource cycle base type */
>   typedef u64 cycle_t;
>   struct clocksource;
> +struct module;
>   
>   #ifdef CONFIG_ARCH_CLOCKSOURCE_DATA
>   #include <asm/clocksource.h>
> @@ -162,6 +163,7 @@ extern u64 timecounter_cyc2time(struct t
>    * @suspend:		suspend function for the clocksource, if necessary
>    * @resume:		resume function for the clocksource, if necessary
>    * @cycle_last:		most recent cycle counter value seen by ::read()
> + * @owner:		module reference
>    */
>   struct clocksource {
>   	/*
> @@ -195,6 +197,7 @@ struct clocksource {
>   	cycle_t cs_last;
>   	cycle_t wd_last;
>   #endif
> +	struct module *owner;

Maybe could you add a comment about who is expected to set the owner ptr?



>   } ____cacheline_aligned;
>   
>   /*
> Index: tip/kernel/time/timekeeping.c
> ===================================================================
> --- tip.orig/kernel/time/timekeeping.c
> +++ tip/kernel/time/timekeeping.c
> @@ -627,11 +627,16 @@ static int change_clocksource(void *data
>   	write_seqcount_begin(&timekeeper_seq);
>   
>   	timekeeping_forward_now(tk);
> -	if (!new->enable || new->enable(new) == 0) {
> -		old = tk->clock;
> -		tk_setup_internals(tk, new);
> -		if (old->disable)
> -			old->disable(old);
> +	if (try_module_get(new->owner)) {

Hrm..  So this works, but the interface is a bit non-intuitive, since 
without looking up try_module_get() I'd be surprised that this would 
succeed if the owner ptr is null. Maybe deserves a comment?

> +		if (!new->enable || new->enable(new) == 0) {
> +			old = tk->clock;
> +			tk_setup_internals(tk, new);
> +			if (old->disable)
> +				old->disable(old);
> +			module_put(old->owner);
> +		} else {
> +			module_put(new->owner);
> +		}
>   	}
>   	timekeeping_update(tk, true, true);
>   

thanks
-john

[tip:timers/core] clocksource: Add module refcount

[tip-bot for Thomas Gleixner]

Commit-ID:  09ac369c825d9d593404306d59062d854b321e9b
Gitweb:     http://git.kernel.org/tip/09ac369c825d9d593404306d59062d854b321e9b
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 25 Apr 2013 20:31:44 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 16 May 2013 11:09:14 +0200

clocksource: Add module refcount

Add a module refcount, so the current clocksource cannot be removed
unconditionally.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Link: http://lkml.kernel.org/r/20130425143435.762417789@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/clocksource.h |  3 +++
 kernel/time/timekeeping.c   | 19 ++++++++++++++-----
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index aa6ba44..32a895b 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -21,6 +21,7 @@
 /* clocksource cycle base type */
 typedef u64 cycle_t;
 struct clocksource;
+struct module;
 
 #ifdef CONFIG_ARCH_CLOCKSOURCE_DATA
 #include <asm/clocksource.h>
@@ -162,6 +163,7 @@ extern u64 timecounter_cyc2time(struct timecounter *tc,
  * @suspend:		suspend function for the clocksource, if necessary
  * @resume:		resume function for the clocksource, if necessary
  * @cycle_last:		most recent cycle counter value seen by ::read()
+ * @owner:		module reference, must be set by clocksource in modules
  */
 struct clocksource {
 	/*
@@ -195,6 +197,7 @@ struct clocksource {
 	cycle_t cs_last;
 	cycle_t wd_last;
 #endif
+	struct module *owner;
 } ____cacheline_aligned;
 
 /*
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index da6e10c..933efa4 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -627,11 +627,20 @@ static int change_clocksource(void *data)
 	write_seqcount_begin(&timekeeper_seq);
 
 	timekeeping_forward_now(tk);
-	if (!new->enable || new->enable(new) == 0) {
-		old = tk->clock;
-		tk_setup_internals(tk, new);
-		if (old->disable)
-			old->disable(old);
+	/*
+	 * If the cs is in module, get a module reference. Succeeds
+	 * for built-in code (owner == NULL) as well.
+	 */
+	if (try_module_get(new->owner)) {
+		if (!new->enable || new->enable(new) == 0) {
+			old = tk->clock;
+			tk_setup_internals(tk, new);
+			if (old->disable)
+				old->disable(old);
+			module_put(old->owner);
+		} else {
+			module_put(new->owner);
+		}
 	}
 	timekeeping_update(tk, true, true);
 

[patch 03/15] clocksource: Let timekeeping_notify return success/error

[Thomas Gleixner]

[-- Attachment #1: clocksource-verify-timekeeping-notify.patch --]
[-- Type: text/plain, Size: 2259 bytes --]

timekeeping_notify() can fail due cs->enable() failure. Though the
caller does not notice and happily keeps the wrong clocksource as the
current one.

Let the caller know about failure, so the current clocksource will be
shown correctly in sysfs.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/clocksource.h |    2 +-
 kernel/time/clocksource.c   |    6 +++---
 kernel/time/timekeeping.c   |    5 +++--
 3 files changed, 7 insertions(+), 6 deletions(-)

Index: tip/include/linux/clocksource.h
===================================================================
--- tip.orig/include/linux/clocksource.h
+++ tip/include/linux/clocksource.h
@@ -321,7 +321,7 @@ static inline void __clocksource_updatef
 }
 
 
-extern void timekeeping_notify(struct clocksource *clock);
+extern int timekeeping_notify(struct clocksource *clock);
 
 extern cycle_t clocksource_mmio_readl_up(struct clocksource *);
 extern cycle_t clocksource_mmio_readl_down(struct clocksource *);
Index: tip/kernel/time/clocksource.c
===================================================================
--- tip.orig/kernel/time/clocksource.c
+++ tip/kernel/time/clocksource.c
@@ -611,10 +611,10 @@ static void clocksource_select(void)
 			best = cs;
 		break;
 	}
-	if (curr_clocksource != best) {
-		printk(KERN_INFO "Switching to clocksource %s\n", best->name);
+
+	if (curr_clocksource != best && !timekeeping_notify(best)) {
+		pr_info("Switched to clocksource %s\n", best->name);
 		curr_clocksource = best;
-		timekeeping_notify(curr_clocksource);
 	}
 }
 
Index: tip/kernel/time/timekeeping.c
===================================================================
--- tip.orig/kernel/time/timekeeping.c
+++ tip/kernel/time/timekeeping.c
@@ -648,14 +648,15 @@ static int change_clocksource(void *data
  * This function is called from clocksource.c after a new, better clock
  * source has been registered. The caller holds the clocksource_mutex.
  */
-void timekeeping_notify(struct clocksource *clock)
+int timekeeping_notify(struct clocksource *clock)
 {
 	struct timekeeper *tk = &timekeeper;
 
 	if (tk->clock == clock)
-		return;
+		return 0;
 	stop_machine(change_clocksource, clock, NULL);
 	tick_clock_notify();
+	return tk->clock == clock ? 0 : -1;
 }
 
 /**

Re: [patch 03/15] clocksource: Let timekeeping_notify return success/error

[John Stultz]

On 04/25/2013 01:31 PM, Thomas Gleixner wrote:
> timekeeping_notify() can fail due cs->enable() failure. Though the
> caller does not notice and happily keeps the wrong clocksource as the
> current one.
>
> Let the caller know about failure, so the current clocksource will be
> shown correctly in sysfs.
>
> Signed-off-by: Thomas Gleixner<tglx@linutronix.de>

Acked-by: John Stultz <john.stultz@linaro.org>

thanks
-john

[tip:timers/core] clocksource: Let timekeeping_notify return success/error

[tip-bot for Thomas Gleixner]

Commit-ID:  ba919d1caa2e624eb8c6cae1f2ce0a253e697d45
Gitweb:     http://git.kernel.org/tip/ba919d1caa2e624eb8c6cae1f2ce0a253e697d45
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 25 Apr 2013 20:31:44 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 16 May 2013 11:09:14 +0200

clocksource: Let timekeeping_notify return success/error

timekeeping_notify() can fail due cs->enable() failure. Though the
caller does not notice and happily keeps the wrong clocksource as the
current one.

Let the caller know about failure, so the current clocksource will be
shown correctly in sysfs.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: John Stultz <john.stultz@linaro.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Link: http://lkml.kernel.org/r/20130425143435.696321912@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/clocksource.h | 2 +-
 kernel/time/clocksource.c   | 6 +++---
 kernel/time/timekeeping.c   | 5 +++--
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 7279b94..aa6ba44 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -321,7 +321,7 @@ static inline void __clocksource_updatefreq_khz(struct clocksource *cs, u32 khz)
 }
 
 
-extern void timekeeping_notify(struct clocksource *clock);
+extern int timekeeping_notify(struct clocksource *clock);
 
 extern cycle_t clocksource_mmio_readl_up(struct clocksource *);
 extern cycle_t clocksource_mmio_readl_down(struct clocksource *);
diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index dda5c71..1923a34 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -611,10 +611,10 @@ static void clocksource_select(void)
 			best = cs;
 		break;
 	}
-	if (curr_clocksource != best) {
-		printk(KERN_INFO "Switching to clocksource %s\n", best->name);
+
+	if (curr_clocksource != best && !timekeeping_notify(best)) {
+		pr_info("Switched to clocksource %s\n", best->name);
 		curr_clocksource = best;
-		timekeeping_notify(curr_clocksource);
 	}
 }
 
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 98cd470..da6e10c 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -648,14 +648,15 @@ static int change_clocksource(void *data)
  * This function is called from clocksource.c after a new, better clock
  * source has been registered. The caller holds the clocksource_mutex.
  */
-void timekeeping_notify(struct clocksource *clock)
+int timekeeping_notify(struct clocksource *clock)
 {
 	struct timekeeper *tk = &timekeeper;
 
 	if (tk->clock == clock)
-		return;
+		return 0;
 	stop_machine(change_clocksource, clock, NULL);
 	tick_clock_notify();
+	return tk->clock == clock ? 0 : -1;
 }
 
 /**

[patch 05/15] clocksource: Allow clocksource select to skip current clocksource

[Thomas Gleixner]

[-- Attachment #1: clocksource-allow-clocksource-select-to-skip-current.patch --]
[-- Type: text/plain, Size: 3565 bytes --]

Preparatory patch for clocksource unbind support.

Modify clocksource_select, so it skips the current clocksource on
request and tries to find a fallback clocksource. Convert all existing
users. No functional change.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/clocksource.c |   24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

Index: tip/kernel/time/clocksource.c
===================================================================
--- tip.orig/kernel/time/clocksource.c
+++ tip/kernel/time/clocksource.c
@@ -553,7 +553,7 @@ static u64 clocksource_max_deferment(str
 
 #ifndef CONFIG_ARCH_USES_GETTIMEOFFSET
 
-static struct clocksource *clocksource_find_best(bool oneshot)
+static struct clocksource *clocksource_find_best(bool oneshot, bool skipcur)
 {
 	struct clocksource *cs;
 
@@ -566,6 +566,8 @@ static struct clocksource *clocksource_f
 	 * the best rating.
 	 */
 	list_for_each_entry(cs, &clocksource_list, list) {
+		if (skipcur && cs == curr_clocksource)
+			continue;
 		if (oneshot && !(cs->flags & CLOCK_SOURCE_VALID_FOR_HRES))
 			continue;
 		return cs;
@@ -581,18 +583,20 @@ static struct clocksource *clocksource_f
  * Select the clocksource with the best rating, or the clocksource,
  * which is selected by userspace override.
  */
-static void clocksource_select(void)
+static void clocksource_select(bool skipcur)
 {
 	bool oneshot = tick_oneshot_mode_active();
 	struct clocksource *best, *cs;
 
 	/* Find the best suitable clocksource */
-	best = clocksource_find_best(oneshot);
+	best = clocksource_find_best(oneshot, skipcur);
 	if (!best)
 		return;
 
 	/* Check for the override clocksource. */
 	list_for_each_entry(cs, &clocksource_list, list) {
+		if (skipcur && cs == curr_clocksource)
+			continue;
 		if (strcmp(cs->name, override_name) != 0)
 			continue;
 		/*
@@ -620,7 +624,7 @@ static void clocksource_select(void)
 
 #else /* !CONFIG_ARCH_USES_GETTIMEOFFSET */
 
-static inline void clocksource_select(void) { }
+static inline void clocksource_select(bool skipcur) { }
 
 #endif
 
@@ -645,7 +649,7 @@ static int __init clocksource_done_booti
 	clocksource_watchdog_kthread(NULL);
 
 	mutex_lock(&clocksource_mutex);
-	clocksource_select();
+	clocksource_select(false);
 	mutex_unlock(&clocksource_mutex);
 	return 0;
 }
@@ -739,7 +743,7 @@ int __clocksource_register_scale(struct 
 	mutex_lock(&clocksource_mutex);
 	clocksource_enqueue(cs);
 	clocksource_enqueue_watchdog(cs);
-	clocksource_select();
+	clocksource_select(false);
 	mutex_unlock(&clocksource_mutex);
 	return 0;
 }
@@ -766,7 +770,7 @@ int clocksource_register(struct clocksou
 	mutex_lock(&clocksource_mutex);
 	clocksource_enqueue(cs);
 	clocksource_enqueue_watchdog(cs);
-	clocksource_select();
+	clocksource_select(false);
 	mutex_unlock(&clocksource_mutex);
 	return 0;
 }
@@ -777,7 +781,7 @@ static void __clocksource_change_rating(
 	list_del(&cs->list);
 	cs->rating = rating;
 	clocksource_enqueue(cs);
-	clocksource_select();
+	clocksource_select(false);
 }
 
 /**
@@ -802,7 +806,7 @@ void clocksource_unregister(struct clock
 	mutex_lock(&clocksource_mutex);
 	clocksource_dequeue_watchdog(cs);
 	list_del(&cs->list);
-	clocksource_select();
+	clocksource_select(false);
 	mutex_unlock(&clocksource_mutex);
 }
 EXPORT_SYMBOL(clocksource_unregister);
@@ -858,7 +862,7 @@ static ssize_t sysfs_override_clocksourc
 	if (count > 0)
 		memcpy(override_name, buf, count);
 	override_name[count] = 0;
-	clocksource_select();
+	clocksource_select(false);
 
 	mutex_unlock(&clocksource_mutex);
 

Re: [patch 05/15] clocksource: Allow clocksource select to skip current clocksource

[John Stultz]

On 04/25/2013 01:31 PM, Thomas Gleixner wrote:
> Preparatory patch for clocksource unbind support.
>
> Modify clocksource_select, so it skips the current clocksource on
> request and tries to find a fallback clocksource. Convert all existing
> users. No functional change.
>
> Signed-off-by: Thomas Gleixner<tglx@linutronix.de>
> ---
>   kernel/time/clocksource.c |   24 ++++++++++++++----------
>   1 file changed, 14 insertions(+), 10 deletions(-)
>
> Index: tip/kernel/time/clocksource.c
> ===================================================================
> --- tip.orig/kernel/time/clocksource.c
> +++ tip/kernel/time/clocksource.c
> @@ -553,7 +553,7 @@ static u64 clocksource_max_deferment(str
>   
>   #ifndef CONFIG_ARCH_USES_GETTIMEOFFSET
>   
> -static struct clocksource *clocksource_find_best(bool oneshot)
> +static struct clocksource *clocksource_find_best(bool oneshot, bool skipcur)
>   {
>   	struct clocksource *cs;
>   
> @@ -566,6 +566,8 @@ static struct clocksource *clocksource_f
>   	 * the best rating.
>   	 */
>   	list_for_each_entry(cs, &clocksource_list, list) {
> +		if (skipcur && cs == curr_clocksource)
> +			continue;
>   		if (oneshot && !(cs->flags & CLOCK_SOURCE_VALID_FOR_HRES))
>   			continue;
>   		return cs;
> @@ -581,18 +583,20 @@ static struct clocksource *clocksource_f
>    * Select the clocksource with the best rating, or the clocksource,
>    * which is selected by userspace override.
>    */
> -static void clocksource_select(void)
> +static void clocksource_select(bool skipcur)
>   {
>   	bool oneshot = tick_oneshot_mode_active();
>   	struct clocksource *best, *cs;
>   
>   	/* Find the best suitable clocksource */
> -	best = clocksource_find_best(oneshot);
> +	best = clocksource_find_best(oneshot, skipcur);
>   	if (!best)
>   		return;
>   
>   	/* Check for the override clocksource. */
>   	list_for_each_entry(cs, &clocksource_list, list) {
> +		if (skipcur && cs == curr_clocksource)
> +			continue;
>   		if (strcmp(cs->name, override_name) != 0)
>   			continue;
>   		/*
> @@ -620,7 +624,7 @@ static void clocksource_select(void)
>   
>   #else /* !CONFIG_ARCH_USES_GETTIMEOFFSET */
>   
> -static inline void clocksource_select(void) { }
> +static inline void clocksource_select(bool skipcur) { }
>   
>   #endif

Not a major objection, but I sort of don't like the bool flag arguments 
to function that alters their behavior, since its not really very 
descriptive. I know I'll be looking at the code below thinking "hmmm.. 
clocksource_select(false).. now what does that mean?" at some point not 
too far away.

Instead, would leaving clocksource_select() alone, and introducing a 
clocksource_select_fallback() for the skipcur behavior, be a more 
descriptive choice? The code could very much use the same skipcur logic, 
in a __clocksource_select() or something function, but just helps make 
the normal usage easier to read.




>   
> @@ -645,7 +649,7 @@ static int __init clocksource_done_booti
>   	clocksource_watchdog_kthread(NULL);
>   
>   	mutex_lock(&clocksource_mutex);
> -	clocksource_select();
> +	clocksource_select(false);
>   	mutex_unlock(&clocksource_mutex);
>   	return 0;
>   }
> @@ -739,7 +743,7 @@ int __clocksource_register_scale(struct
>   	mutex_lock(&clocksource_mutex);
>   	clocksource_enqueue(cs);
>   	clocksource_enqueue_watchdog(cs);
> -	clocksource_select();
> +	clocksource_select(false);
>   	mutex_unlock(&clocksource_mutex);
>   	return 0;
>   }
> @@ -766,7 +770,7 @@ int clocksource_register(struct clocksou
>   	mutex_lock(&clocksource_mutex);
>   	clocksource_enqueue(cs);
>   	clocksource_enqueue_watchdog(cs);
> -	clocksource_select();
> +	clocksource_select(false);
>   	mutex_unlock(&clocksource_mutex);
>   	return 0;
>   }

thanks
-john

Re: [patch 05/15] clocksource: Allow clocksource select to skip current clocksource

[Thomas Gleixner]

On Mon, 29 Apr 2013, John Stultz wrote:
> On 04/25/2013 01:31 PM, Thomas Gleixner wrote:
> > Preparatory patch for clocksource unbind support.
> > 
> > Modify clocksource_select, so it skips the current clocksource on
> > request and tries to find a fallback clocksource. Convert all existing
> > users. No functional change.
> > 
> > Signed-off-by: Thomas Gleixner<tglx@linutronix.de>
> > ---
> >   kernel/time/clocksource.c |   24 ++++++++++++++----------
> >   1 file changed, 14 insertions(+), 10 deletions(-)
> > 
> > Index: tip/kernel/time/clocksource.c
> > ===================================================================
> > --- tip.orig/kernel/time/clocksource.c
> > +++ tip/kernel/time/clocksource.c
> > @@ -553,7 +553,7 @@ static u64 clocksource_max_deferment(str
> >     #ifndef CONFIG_ARCH_USES_GETTIMEOFFSET
> >   -static struct clocksource *clocksource_find_best(bool oneshot)
> > +static struct clocksource *clocksource_find_best(bool oneshot, bool
> > skipcur)
> >   {
> >   	struct clocksource *cs;
> >   @@ -566,6 +566,8 @@ static struct clocksource *clocksource_f
> >   	 * the best rating.
> >   	 */
> >   	list_for_each_entry(cs, &clocksource_list, list) {
> > +		if (skipcur && cs == curr_clocksource)
> > +			continue;
> >   		if (oneshot && !(cs->flags & CLOCK_SOURCE_VALID_FOR_HRES))
> >   			continue;
> >   		return cs;
> > @@ -581,18 +583,20 @@ static struct clocksource *clocksource_f
> >    * Select the clocksource with the best rating, or the clocksource,
> >    * which is selected by userspace override.
> >    */
> > -static void clocksource_select(void)
> > +static void clocksource_select(bool skipcur)
> >   {
> >   	bool oneshot = tick_oneshot_mode_active();
> >   	struct clocksource *best, *cs;
> >     	/* Find the best suitable clocksource */
> > -	best = clocksource_find_best(oneshot);
> > +	best = clocksource_find_best(oneshot, skipcur);
> >   	if (!best)
> >   		return;
> >     	/* Check for the override clocksource. */
> >   	list_for_each_entry(cs, &clocksource_list, list) {
> > +		if (skipcur && cs == curr_clocksource)
> > +			continue;
> >   		if (strcmp(cs->name, override_name) != 0)
> >   			continue;
> >   		/*
> > @@ -620,7 +624,7 @@ static void clocksource_select(void)
> >     #else /* !CONFIG_ARCH_USES_GETTIMEOFFSET */
> >   -static inline void clocksource_select(void) { }
> > +static inline void clocksource_select(bool skipcur) { }
> >     #endif
> 
> Not a major objection, but I sort of don't like the bool flag arguments to
> function that alters their behavior, since its not really very descriptive. I
> know I'll be looking at the code below thinking "hmmm..
> clocksource_select(false).. now what does that mean?" at some point not too
> far away.
> 
> Instead, would leaving clocksource_select() alone, and introducing a
> clocksource_select_fallback() for the skipcur behavior, be a more descriptive
> choice? The code could very much use the same skipcur logic, in a
> __clocksource_select() or something function, but just helps make the normal
> usage easier to read.

Yeah, I can do that.

[tip:timers/core] clocksource: Allow clocksource select to skip current clocksource

[tip-bot for Thomas Gleixner]

Commit-ID:  f5a2e34375a5e2b711aea488ac3ae50eeba6d57c
Gitweb:     http://git.kernel.org/tip/f5a2e34375a5e2b711aea488ac3ae50eeba6d57c
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 25 Apr 2013 20:31:45 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 16 May 2013 11:09:15 +0200

clocksource: Allow clocksource select to skip current clocksource

Preparatory patch for clocksource unbind support.

Split out code from clocksource_select and modify it, so it skips the
current clocksource on request and tries to find a fallback
clocksource. Convert all existing users. No functional change.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Link: http://lkml.kernel.org/r/20130425143435.834965397@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/clocksource.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index 1923a34..9782997 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -553,7 +553,7 @@ static u64 clocksource_max_deferment(struct clocksource *cs)
 
 #ifndef CONFIG_ARCH_USES_GETTIMEOFFSET
 
-static struct clocksource *clocksource_find_best(bool oneshot)
+static struct clocksource *clocksource_find_best(bool oneshot, bool skipcur)
 {
 	struct clocksource *cs;
 
@@ -566,6 +566,8 @@ static struct clocksource *clocksource_find_best(bool oneshot)
 	 * the best rating.
 	 */
 	list_for_each_entry(cs, &clocksource_list, list) {
+		if (skipcur && cs == curr_clocksource)
+			continue;
 		if (oneshot && !(cs->flags & CLOCK_SOURCE_VALID_FOR_HRES))
 			continue;
 		return cs;
@@ -573,26 +575,20 @@ static struct clocksource *clocksource_find_best(bool oneshot)
 	return NULL;
 }
 
-/**
- * clocksource_select - Select the best clocksource available
- *
- * Private function. Must hold clocksource_mutex when called.
- *
- * Select the clocksource with the best rating, or the clocksource,
- * which is selected by userspace override.
- */
-static void clocksource_select(void)
+static void __clocksource_select(bool skipcur)
 {
 	bool oneshot = tick_oneshot_mode_active();
 	struct clocksource *best, *cs;
 
 	/* Find the best suitable clocksource */
-	best = clocksource_find_best(oneshot);
+	best = clocksource_find_best(oneshot, skipcur);
 	if (!best)
 		return;
 
 	/* Check for the override clocksource. */
 	list_for_each_entry(cs, &clocksource_list, list) {
+		if (skipcur && cs == curr_clocksource)
+			continue;
 		if (strcmp(cs->name, override_name) != 0)
 			continue;
 		/*
@@ -618,6 +614,19 @@ static void clocksource_select(void)
 	}
 }
 
+/**
+ * clocksource_select - Select the best clocksource available
+ *
+ * Private function. Must hold clocksource_mutex when called.
+ *
+ * Select the clocksource with the best rating, or the clocksource,
+ * which is selected by userspace override.
+ */
+static void clocksource_select(void)
+{
+	return __clocksource_select(false);
+}
+
 #else /* !CONFIG_ARCH_USES_GETTIMEOFFSET */
 
 static inline void clocksource_select(void) { }

[patch 06/15] clocksource: Split out user string input

[Thomas Gleixner]

[-- Attachment #1: clocksource-split-out-user-string-input.patch --]
[-- Type: text/plain, Size: 1956 bytes --]

Split out the user string input for clocksource override. Preparatory
patch for unbind.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/clocksource.c |   35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

Index: tip/kernel/time/clocksource.c
===================================================================
--- tip.orig/kernel/time/clocksource.c
+++ tip/kernel/time/clocksource.c
@@ -174,7 +174,8 @@ clocks_calc_mult_shift(u32 *mult, u32 *s
 static struct clocksource *curr_clocksource;
 static LIST_HEAD(clocksource_list);
 static DEFINE_MUTEX(clocksource_mutex);
-static char override_name[32];
+#define CS_NAME_LEN		32
+static char override_name[CS_NAME_LEN];
 static int finished_booting;
 
 #ifdef CONFIG_CLOCKSOURCE_WATCHDOG
@@ -833,6 +834,21 @@ sysfs_show_current_clocksources(struct d
 	return count;
 }
 
+static size_t clocksource_get_uname(const char *buf, char *dst, size_t cnt)
+{
+	/* strings from sysfs write are not 0 terminated! */
+	if (!cnt || cnt >= CS_NAME_LEN)
+		return -EINVAL;
+
+	/* strip of \n: */
+	if (buf[cnt-1] == '\n')
+		cnt--;
+	if (cnt > 0)
+		memcpy(dst, buf, cnt);
+	dst[cnt] = 0;
+	return cnt;
+}
+
 /**
  * sysfs_override_clocksource - interface for manually overriding clocksource
  * @dev:	unused
@@ -847,22 +863,13 @@ static ssize_t sysfs_override_clocksourc
 					  struct device_attribute *attr,
 					  const char *buf, size_t count)
 {
-	size_t ret = count;
-
-	/* strings from sysfs write are not 0 terminated! */
-	if (count >= sizeof(override_name))
-		return -EINVAL;
-
-	/* strip of \n: */
-	if (buf[count-1] == '\n')
-		count--;
+	size_t ret;
 
 	mutex_lock(&clocksource_mutex);
 
-	if (count > 0)
-		memcpy(override_name, buf, count);
-	override_name[count] = 0;
-	clocksource_select(false);
+	ret = clocksource_get_uname(buf, override_name, count);
+	if (ret >= 0)
+		clocksource_select(false);
 
 	mutex_unlock(&clocksource_mutex);
 

Re: [patch 06/15] clocksource: Split out user string input

[John Stultz]

On Thu, Apr 25, 2013 at 1:31 PM, Thomas Gleixner <tglx@linutronix.de> wrote:

> Split out the user string input for clocksource override. Preparatory
> patch for unbind.

This patch has a bug that causes the shell to hang when \n terminated 
clocksource strings are sent to the sysfs interface.

echo hpet > /sys/devices/system/clocksource/clocksource0/current_clocksource
<shell hang>


> +static size_t clocksource_get_uname(const char *buf, char *dst, size_t
> cnt)
> +{
> +       /* strings from sysfs write are not 0 terminated! */
> +       if (!cnt || cnt >= CS_NAME_LEN)
> +               return -EINVAL;
> +
> +       /* strip of \n: */
> +       if (buf[cnt-1] == '\n')
> +               cnt--;
> +       if (cnt > 0)
> +               memcpy(dst, buf, cnt);
> +       dst[cnt] = 0;
> +       return cnt;
> +}
> +
>  /**
>   * sysfs_override_clocksource - interface for manually overriding
> clocksource
>   * @dev:       unused
> @@ -847,22 +863,13 @@ static ssize_t sysfs_override_clocksourc
>                                           struct device_attribute *attr,
>                                           const char *buf, size_t count)
>  {
> -       size_t ret = count;
> -
> -       /* strings from sysfs write are not 0 terminated! */
> -       if (count >= sizeof(override_name))
> -               return -EINVAL;
> -
> -       /* strip of \n: */
> -       if (buf[count-1] == '\n')
> -               count--;
> +       size_t ret;
>
>         mutex_lock(&clocksource_mutex);
>
> -       if (count > 0)
> -               memcpy(override_name, buf, count);
> -       override_name[count] = 0;
> -       clocksource_select(false);
> +       ret = clocksource_get_uname(buf, override_name, count);
> +       if (ret >= 0)
> +               clocksource_select(false);
>
>         mutex_unlock(&clocksource_mutex);


The change above *looks* ok, but is subtlly different then what we had 
before. Note, in the old code, we saved count to ret, and never modified 
ret before returning it.

With the new code, we modify cnt, as we process the copied clocksource 
name, and return cnt, thus we don't return to the sysfs interface the 
same count value we were given, causing the write call to loop.

So you probably want something like the following patch to fix this.

thanks
-john



diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index e9c4f04..5f6c324 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -836,6 +836,8 @@ sysfs_show_current_clocksources(struct device *dev,

  static size_t clocksource_get_uname(const char *buf, char *dst, size_t 
cnt)
  {
+	size_t ret = cnt;
+
  	/* strings from sysfs write are not 0 terminated! */
  	if (!cnt || cnt >= CS_NAME_LEN)
  		return -EINVAL;
@@ -846,7 +848,7 @@ static size_t clocksource_get_uname(const char *buf, 
char *dst, size_t cnt)
  	if (cnt > 0)
  		memcpy(dst, buf, cnt);
  	dst[cnt] = 0;
-	return cnt;
+	return ret;
  }

  /**

Re: [patch 06/15] clocksource: Split out user string input

[Thomas Gleixner]

On Mon, 29 Apr 2013, John Stultz wrote:
> On Thu, Apr 25, 2013 at 1:31 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> So you probably want something like the following patch to fix this.

Yep, stupid me.
 
> thanks
> -john
> 
> 
> 
> diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
> index e9c4f04..5f6c324 100644
> --- a/kernel/time/clocksource.c
> +++ b/kernel/time/clocksource.c
> @@ -836,6 +836,8 @@ sysfs_show_current_clocksources(struct device *dev,
> 
>  static size_t clocksource_get_uname(const char *buf, char *dst, size_t cnt)
>  {
> +	size_t ret = cnt;
> +
>  	/* strings from sysfs write are not 0 terminated! */
>  	if (!cnt || cnt >= CS_NAME_LEN)
>  		return -EINVAL;
> @@ -846,7 +848,7 @@ static size_t clocksource_get_uname(const char *buf, char
> *dst, size_t cnt)
>  	if (cnt > 0)
>  		memcpy(dst, buf, cnt);
>  	dst[cnt] = 0;
> -	return cnt;
> +	return ret;
>  }
> 
>  /**
> 

[tip:timers/core] clocksource: Split out user string input

[tip-bot for Thomas Gleixner]

Commit-ID:  29b5407819f59731c9423238fae03b756822708c
Gitweb:     http://git.kernel.org/tip/29b5407819f59731c9423238fae03b756822708c
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 25 Apr 2013 20:31:45 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 16 May 2013 11:09:15 +0200

clocksource: Split out user string input

Split out the user string input for clocksource override. Preparatory
patch for unbind.

[ jstultz: Fix an off by one error ]

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Link: http://lkml.kernel.org/r/20130425143435.895851338@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/clocksource.c | 37 +++++++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index 9782997..d7f1a45 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -174,7 +174,8 @@ clocks_calc_mult_shift(u32 *mult, u32 *shift, u32 from, u32 to, u32 maxsec)
 static struct clocksource *curr_clocksource;
 static LIST_HEAD(clocksource_list);
 static DEFINE_MUTEX(clocksource_mutex);
-static char override_name[32];
+#define CS_NAME_LEN		32
+static char override_name[CS_NAME_LEN];
 static int finished_booting;
 
 #ifdef CONFIG_CLOCKSOURCE_WATCHDOG
@@ -838,6 +839,23 @@ sysfs_show_current_clocksources(struct device *dev,
 	return count;
 }
 
+static size_t clocksource_get_uname(const char *buf, char *dst, size_t cnt)
+{
+	size_t ret = cnt;
+
+	/* strings from sysfs write are not 0 terminated! */
+	if (!cnt || cnt >= CS_NAME_LEN)
+		return -EINVAL;
+
+	/* strip of \n: */
+	if (buf[cnt-1] == '\n')
+		cnt--;
+	if (cnt > 0)
+		memcpy(dst, buf, cnt);
+	dst[cnt] = 0;
+	return ret;
+}
+
 /**
  * sysfs_override_clocksource - interface for manually overriding clocksource
  * @dev:	unused
@@ -852,22 +870,13 @@ static ssize_t sysfs_override_clocksource(struct device *dev,
 					  struct device_attribute *attr,
 					  const char *buf, size_t count)
 {
-	size_t ret = count;
-
-	/* strings from sysfs write are not 0 terminated! */
-	if (count >= sizeof(override_name))
-		return -EINVAL;
-
-	/* strip of \n: */
-	if (buf[count-1] == '\n')
-		count--;
+	size_t ret;
 
 	mutex_lock(&clocksource_mutex);
 
-	if (count > 0)
-		memcpy(override_name, buf, count);
-	override_name[count] = 0;
-	clocksource_select();
+	ret = clocksource_get_uname(buf, override_name, count);
+	if (ret >= 0)
+		clocksource_select();
 
 	mutex_unlock(&clocksource_mutex);
 

[patch 08/15] clocksource: Let clocksource_unregister() return success/error

[Thomas Gleixner]

[-- Attachment #1: clocksource-unregister-return-success-error.patch --]
[-- Type: text/plain, Size: 2951 bytes --]

The unregister call can fail, if the clocksource is the current one
and there is no replacement clocksource available. It can also fail,
if the clocksource is the watchdog clocksource and I'm not going to
provide support for this.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/clocksource.h |    2 +-
 kernel/time/clocksource.c   |   33 ++++++++++++---------------------
 2 files changed, 13 insertions(+), 22 deletions(-)

Index: tip/include/linux/clocksource.h
===================================================================
--- tip.orig/include/linux/clocksource.h
+++ tip/include/linux/clocksource.h
@@ -282,7 +282,7 @@ static inline s64 clocksource_cyc2ns(cyc
 
 
 extern int clocksource_register(struct clocksource*);
-extern void clocksource_unregister(struct clocksource*);
+extern int clocksource_unregister(struct clocksource*);
 extern void clocksource_touch_watchdog(void);
 extern struct clocksource* clocksource_get_next(void);
 extern void clocksource_change_rating(struct clocksource *cs, int rating);
Index: tip/kernel/time/clocksource.c
===================================================================
--- tip.orig/kernel/time/clocksource.c
+++ tip/kernel/time/clocksource.c
@@ -389,28 +389,17 @@ static void clocksource_enqueue_watchdog
 
 static void clocksource_dequeue_watchdog(struct clocksource *cs)
 {
-	struct clocksource *tmp;
 	unsigned long flags;
 
 	spin_lock_irqsave(&watchdog_lock, flags);
-	if (cs->flags & CLOCK_SOURCE_MUST_VERIFY) {
-		/* cs is a watched clocksource. */
-		list_del_init(&cs->wd_list);
-	} else if (cs == watchdog) {
-		/* Reset watchdog cycles */
-		clocksource_reset_watchdog();
-		/* Current watchdog is removed. Find an alternative. */
-		watchdog = NULL;
-		list_for_each_entry(tmp, &clocksource_list, list) {
-			if (tmp == cs || tmp->flags & CLOCK_SOURCE_MUST_VERIFY)
-				continue;
-			if (!watchdog || tmp->rating > watchdog->rating)
-				watchdog = tmp;
+	if (cs != watchdog) {
+		if (cs->flags & CLOCK_SOURCE_MUST_VERIFY) {
+			/* cs is a watched clocksource. */
+			list_del_init(&cs->wd_list);
+			/* Check if the watchdog timer needs to be stopped. */
+			clocksource_stop_watchdog();
 		}
 	}
-	cs->flags &= ~CLOCK_SOURCE_WATCHDOG;
-	/* Check if the watchdog timer needs to be stopped. */
-	clocksource_stop_watchdog();
 	spin_unlock_irqrestore(&watchdog_lock, flags);
 }
 
@@ -831,13 +820,15 @@ static int clocksource_unbind(struct clo
  * clocksource_unregister - remove a registered clocksource
  * @cs:	clocksource to be unregistered
  */
-void clocksource_unregister(struct clocksource *cs)
+int clocksource_unregister(struct clocksource *cs)
 {
+	int ret = 0;
+
 	mutex_lock(&clocksource_mutex);
-	clocksource_dequeue_watchdog(cs);
-	list_del(&cs->list);
-	clocksource_select(false);
+	if (!list_empty(&cs->list))
+		ret = clocksource_unbind(cs);
 	mutex_unlock(&clocksource_mutex);
+	return ret;
 }
 EXPORT_SYMBOL(clocksource_unregister);
 

Re: [patch 08/15] clocksource: Let clocksource_unregister() return success/error

[John Stultz]

On 04/25/2013 01:31 PM, Thomas Gleixner wrote:
> The unregister call can fail, if the clocksource is the current one
> and there is no replacement clocksource available. It can also fail,
> if the clocksource is the watchdog clocksource and I'm not going to
> provide support for this.
>
> Signed-off-by: Thomas Gleixner<tglx@linutronix.de>
Acked-by: John Stultz <john.stultz@linaro.org>

thanks
-john

[tip:timers/core] clocksource: Let clocksource_unregister() return success/error

[tip-bot for Thomas Gleixner]

Commit-ID:  a89c7edbe7d7aa80f507915f3dd801211b116b79
Gitweb:     http://git.kernel.org/tip/a89c7edbe7d7aa80f507915f3dd801211b116b79
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 25 Apr 2013 20:31:46 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 16 May 2013 11:09:16 +0200

clocksource: Let clocksource_unregister() return success/error

The unregister call can fail, if the clocksource is the current one
and there is no replacement clocksource available. It can also fail,
if the clocksource is the watchdog clocksource and I'm not going to
provide support for this.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Link: http://lkml.kernel.org/r/20130425143436.029915527@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/clocksource.h |  2 +-
 kernel/time/clocksource.c   | 33 ++++++++++++---------------------
 2 files changed, 13 insertions(+), 22 deletions(-)

diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 32a895b..2f39a49 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -282,7 +282,7 @@ static inline s64 clocksource_cyc2ns(cycle_t cycles, u32 mult, u32 shift)
 
 
 extern int clocksource_register(struct clocksource*);
-extern void clocksource_unregister(struct clocksource*);
+extern int clocksource_unregister(struct clocksource*);
 extern void clocksource_touch_watchdog(void);
 extern struct clocksource* clocksource_get_next(void);
 extern void clocksource_change_rating(struct clocksource *cs, int rating);
diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index 791d1ae..31b9033 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -389,28 +389,17 @@ static void clocksource_enqueue_watchdog(struct clocksource *cs)
 
 static void clocksource_dequeue_watchdog(struct clocksource *cs)
 {
-	struct clocksource *tmp;
 	unsigned long flags;
 
 	spin_lock_irqsave(&watchdog_lock, flags);
-	if (cs->flags & CLOCK_SOURCE_MUST_VERIFY) {
-		/* cs is a watched clocksource. */
-		list_del_init(&cs->wd_list);
-	} else if (cs == watchdog) {
-		/* Reset watchdog cycles */
-		clocksource_reset_watchdog();
-		/* Current watchdog is removed. Find an alternative. */
-		watchdog = NULL;
-		list_for_each_entry(tmp, &clocksource_list, list) {
-			if (tmp == cs || tmp->flags & CLOCK_SOURCE_MUST_VERIFY)
-				continue;
-			if (!watchdog || tmp->rating > watchdog->rating)
-				watchdog = tmp;
+	if (cs != watchdog) {
+		if (cs->flags & CLOCK_SOURCE_MUST_VERIFY) {
+			/* cs is a watched clocksource. */
+			list_del_init(&cs->wd_list);
+			/* Check if the watchdog timer needs to be stopped. */
+			clocksource_stop_watchdog();
 		}
 	}
-	cs->flags &= ~CLOCK_SOURCE_WATCHDOG;
-	/* Check if the watchdog timer needs to be stopped. */
-	clocksource_stop_watchdog();
 	spin_unlock_irqrestore(&watchdog_lock, flags);
 }
 
@@ -841,13 +830,15 @@ static int clocksource_unbind(struct clocksource *cs)
  * clocksource_unregister - remove a registered clocksource
  * @cs:	clocksource to be unregistered
  */
-void clocksource_unregister(struct clocksource *cs)
+int clocksource_unregister(struct clocksource *cs)
 {
+	int ret = 0;
+
 	mutex_lock(&clocksource_mutex);
-	clocksource_dequeue_watchdog(cs);
-	list_del(&cs->list);
-	clocksource_select();
+	if (!list_empty(&cs->list))
+		ret = clocksource_unbind(cs);
 	mutex_unlock(&clocksource_mutex);
+	return ret;
 }
 EXPORT_SYMBOL(clocksource_unregister);
 

[patch 07/15] clocksource: Provide unbind interface in sysfs

[Thomas Gleixner]

[-- Attachment #1: clocksource-provide-unbind-interface.patch --]
[-- Type: text/plain, Size: 4312 bytes --]

With the module refcount held for the current clocksource there is no
way to unload the module. 

Provide a sysfs interface which allows to unbind the clocksource. One
could argue that the clocksource override could be (ab)used to do so,
but the clocksource override cannot be used from the kernel itself,
while an unbind function can be used to programmatically check whether
a clocksource can be shutdown or not.

The unbind functionality uses the new skip current feature of
clocksource_select and verifies that a fallback clocksource has been
installed. If the clocksource which should be unbound is the current
clocksource and no fallback can be found, unbind returns -EBUSY.

This does not support the unbinding of a clocksource which is used as
the watchdog clocksource. No point in fostering crappy hardware.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/clocksource.c |   68 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

Index: tip/kernel/time/clocksource.c
===================================================================
--- tip.orig/kernel/time/clocksource.c
+++ tip/kernel/time/clocksource.c
@@ -440,6 +440,11 @@ static int clocksource_watchdog_kthread(
 	return 0;
 }
 
+static bool clocksource_is_watchdog(struct clocksource *cs)
+{
+	return cs == watchdog;
+}
+
 #else /* CONFIG_CLOCKSOURCE_WATCHDOG */
 
 static void clocksource_enqueue_watchdog(struct clocksource *cs)
@@ -451,6 +456,7 @@ static void clocksource_enqueue_watchdog
 static inline void clocksource_dequeue_watchdog(struct clocksource *cs) { }
 static inline void clocksource_resume_watchdog(void) { }
 static inline int clocksource_watchdog_kthread(void *data) { return 0; }
+static bool clocksource_is_watchdog(struct clocksource *cs) { return false; }
 
 #endif /* CONFIG_CLOCKSOURCE_WATCHDOG */
 
@@ -798,6 +804,29 @@ void clocksource_change_rating(struct cl
 }
 EXPORT_SYMBOL(clocksource_change_rating);
 
+/*
+ * Unbind clocksource @cs. Called with clocksource_mutex held
+ */
+static int clocksource_unbind(struct clocksource *cs)
+{
+	/*
+	 * I really can't convince myself to support this on hardware
+	 * designed by lobotomized monkeys.
+	 */
+	if (clocksource_is_watchdog(cs))
+		return -EBUSY;
+
+	if (cs == curr_clocksource) {
+		/* Select and try to install a replacement clock source */
+		clocksource_select(true);
+		if (curr_clocksource == cs)
+			return -EBUSY;
+	}
+	clocksource_dequeue_watchdog(cs);
+	list_del_init(&cs->list);
+	return 0;
+}
+
 /**
  * clocksource_unregister - remove a registered clocksource
  * @cs:	clocksource to be unregistered
@@ -877,6 +906,40 @@ static ssize_t sysfs_override_clocksourc
 }
 
 /**
+ * sysfs_unbind_current_clocksource - interface for manually unbinding clocksource
+ * @dev:	unused
+ * @attr:	unused
+ * @buf:	unused
+ * @count:	length of buffer
+ *
+ * Takes input from sysfs interface for manually unbinding a clocksource.
+ */
+static ssize_t sysfs_unbind_clocksource(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t count)
+{
+	struct clocksource *cs;
+	char name[CS_NAME_LEN];
+	size_t ret;
+
+	ret = clocksource_get_uname(buf, name, count);
+	if (ret < 0)
+		return ret;
+
+	ret = -ENODEV;
+	mutex_lock(&clocksource_mutex);
+	list_for_each_entry(cs, &clocksource_list, list) {
+		if (strcmp(cs->name, name))
+			continue;
+		ret = clocksource_unbind(cs);
+		break;
+	}
+	mutex_unlock(&clocksource_mutex);
+
+	return ret ? ret : count;
+}
+
+/**
  * sysfs_show_available_clocksources - sysfs interface for listing clocksource
  * @dev:	unused
  * @attr:	unused
@@ -918,6 +981,8 @@ sysfs_show_available_clocksources(struct
 static DEVICE_ATTR(current_clocksource, 0644, sysfs_show_current_clocksources,
 		   sysfs_override_clocksource);
 
+static DEVICE_ATTR(unbind_clocksource, 0200, NULL, sysfs_unbind_clocksource);
+
 static DEVICE_ATTR(available_clocksource, 0444,
 		   sysfs_show_available_clocksources, NULL);
 
@@ -942,6 +1007,9 @@ static int __init init_clocksource_sysfs
 				&device_clocksource,
 				&dev_attr_current_clocksource);
 	if (!error)
+		error = device_create_file(&device_clocksource,
+					   &dev_attr_unbind_clocksource);
+	if (!error)
 		error = device_create_file(
 				&device_clocksource,
 				&dev_attr_available_clocksource);

Re: [patch 07/15] clocksource: Provide unbind interface in sysfs

[John Stultz]

On 04/25/2013 01:31 PM, Thomas Gleixner wrote:
> With the module refcount held for the current clocksource there is no
> way to unload the module.
>
> Provide a sysfs interface which allows to unbind the clocksource. One
> could argue that the clocksource override could be (ab)used to do so,
> but the clocksource override cannot be used from the kernel itself,
> while an unbind function can be used to programmatically check whether
> a clocksource can be shutdown or not.
>
> The unbind functionality uses the new skip current feature of
> clocksource_select and verifies that a fallback clocksource has been
> installed. If the clocksource which should be unbound is the current
> clocksource and no fallback can be found, unbind returns -EBUSY.
>
> This does not support the unbinding of a clocksource which is used as
> the watchdog clocksource. No point in fostering crappy hardware.

So.. if the clocksource you want to unbind is the highest rated 
continuous clocksource that doesn't need a watchdog (basically what's 
likely to be in-use and required to be unbinded), its likely to be 
selected as the watchdog already.

ie: on a system that has only HPET/ACPI_PM, you can't unbind HPET, since 
its a watchdog.

Or are you really wanting to prohibit this functionality for all 
CONFIG_CLOCKSOURCE_WATCHDOG hardware, which would be easier to do via 
build time ifdefs?

thanks
-john

Re: [patch 07/15] clocksource: Provide unbind interface in sysfs

[Thomas Gleixner]

On Mon, 29 Apr 2013, John Stultz wrote:
> On 04/25/2013 01:31 PM, Thomas Gleixner wrote:
> > With the module refcount held for the current clocksource there is no
> > way to unload the module.
> > 
> > Provide a sysfs interface which allows to unbind the clocksource. One
> > could argue that the clocksource override could be (ab)used to do so,
> > but the clocksource override cannot be used from the kernel itself,
> > while an unbind function can be used to programmatically check whether
> > a clocksource can be shutdown or not.
> > 
> > The unbind functionality uses the new skip current feature of
> > clocksource_select and verifies that a fallback clocksource has been
> > installed. If the clocksource which should be unbound is the current
> > clocksource and no fallback can be found, unbind returns -EBUSY.
> > 
> > This does not support the unbinding of a clocksource which is used as
> > the watchdog clocksource. No point in fostering crappy hardware.
> 
> So.. if the clocksource you want to unbind is the highest rated continuous
> clocksource that doesn't need a watchdog (basically what's likely to be in-use
> and required to be unbinded), its likely to be selected as the watchdog
> already.
> 
> ie: on a system that has only HPET/ACPI_PM, you can't unbind HPET, since its a
> watchdog.

No. The thing is that I only prevent unbinding if it is used as the
watchdog. In the above HPET/PM scenario both are potential watchdogs,
but w/o a user it's valid to unbind one of them.

What I need to prevent is:

TSC is current clocksource and we only have ACPI_PM as watchdog and
its used. So now you try to unbind ACPI_PM then the TSC would be left
w/o a watchdog instance. That's what I'm preventing. Will reword the
changelog accordingly.

Thanks,

	tglx

Re: [patch 07/15] clocksource: Provide unbind interface in sysfs

[John Stultz]

On 05/15/2013 02:47 AM, Thomas Gleixner wrote:
> On Mon, 29 Apr 2013, John Stultz wrote:
>> On 04/25/2013 01:31 PM, Thomas Gleixner wrote:
>>> With the module refcount held for the current clocksource there is no
>>> way to unload the module.
>>>
>>> Provide a sysfs interface which allows to unbind the clocksource. One
>>> could argue that the clocksource override could be (ab)used to do so,
>>> but the clocksource override cannot be used from the kernel itself,
>>> while an unbind function can be used to programmatically check whether
>>> a clocksource can be shutdown or not.
>>>
>>> The unbind functionality uses the new skip current feature of
>>> clocksource_select and verifies that a fallback clocksource has been
>>> installed. If the clocksource which should be unbound is the current
>>> clocksource and no fallback can be found, unbind returns -EBUSY.
>>>
>>> This does not support the unbinding of a clocksource which is used as
>>> the watchdog clocksource. No point in fostering crappy hardware.
>> So.. if the clocksource you want to unbind is the highest rated continuous
>> clocksource that doesn't need a watchdog (basically what's likely to be in-use
>> and required to be unbinded), its likely to be selected as the watchdog
>> already.
>>
>> ie: on a system that has only HPET/ACPI_PM, you can't unbind HPET, since its a
>> watchdog.
> No. The thing is that I only prevent unbinding if it is used as the
> watchdog. In the above HPET/PM scenario both are potential watchdogs,
> but w/o a user it's valid to unbind one of them.
>
> What I need to prevent is:
>
> TSC is current clocksource and we only have ACPI_PM as watchdog and
> its used. So now you try to unbind ACPI_PM then the TSC would be left
> w/o a watchdog instance. That's what I'm preventing. Will reword the
> changelog accordingly.

You might double check the logic, because I feel like I actually hit 
this issue where on my vm machine w/ only hpet/acpi_pm I couldn't unbind 
the hpet.

Though its been a few weeks, so maybe I'm confusing things?
Let me know if you want me to try to reproduce it.

thanks
-john

Re: [patch 07/15] clocksource: Provide unbind interface in sysfs

[Thomas Gleixner]

On Wed, 15 May 2013, John Stultz wrote:
> On 05/15/2013 02:47 AM, Thomas Gleixner wrote:
> > TSC is current clocksource and we only have ACPI_PM as watchdog and
> > its used. So now you try to unbind ACPI_PM then the TSC would be left
> > w/o a watchdog instance. That's what I'm preventing. Will reword the
> > changelog accordingly.
> 
> You might double check the logic, because I feel like I actually hit this
> issue where on my vm machine w/ only hpet/acpi_pm I couldn't unbind the hpet.

Yes, that might be true. I'm only checking whether the clocksource is
assigned watchdog duty. So now if the TSC is not available we still
assign watchdog duty to HPET and therefor unbinding it does not work.

I really don't worry about that. The whole watchdog workaround is
horrible and all I want to prevent is wreckage of that fragile piece
of ...

Thanks,

	tglx

[tip:timers/core] clocksource: Provide unbind interface in sysfs

[tip-bot for Thomas Gleixner]

Commit-ID:  7eaeb34305dee26634f7c98ae62646da5cebe91d
Gitweb:     http://git.kernel.org/tip/7eaeb34305dee26634f7c98ae62646da5cebe91d
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 25 Apr 2013 20:31:46 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 16 May 2013 11:09:15 +0200

clocksource: Provide unbind interface in sysfs

With the module refcount held for the current clocksource there is no
way to unload the module. 

Provide a sysfs interface which allows to unbind the clocksource. One
could argue that the clocksource override could be (ab)used to do so,
but the clocksource override cannot be used from the kernel itself,
while an unbind function can be used to programmatically check whether
a clocksource can be shutdown or not.

The unbind functionality uses the new skip current feature of
clocksource_select and verifies that a fallback clocksource has been
installed. If the clocksource which should be unbound is the current
clocksource and no fallback can be found, unbind returns -EBUSY.

This does not support the unbinding of a clocksource which is used as
the watchdog clocksource. No point in fostering crappy hardware.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Link: http://lkml.kernel.org/r/20130425143435.964218245@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/clocksource.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index d7f1a45..791d1ae 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -440,6 +440,11 @@ static int clocksource_watchdog_kthread(void *data)
 	return 0;
 }
 
+static bool clocksource_is_watchdog(struct clocksource *cs)
+{
+	return cs == watchdog;
+}
+
 #else /* CONFIG_CLOCKSOURCE_WATCHDOG */
 
 static void clocksource_enqueue_watchdog(struct clocksource *cs)
@@ -451,6 +456,7 @@ static void clocksource_enqueue_watchdog(struct clocksource *cs)
 static inline void clocksource_dequeue_watchdog(struct clocksource *cs) { }
 static inline void clocksource_resume_watchdog(void) { }
 static inline int clocksource_watchdog_kthread(void *data) { return 0; }
+static bool clocksource_is_watchdog(struct clocksource *cs) { return false; }
 
 #endif /* CONFIG_CLOCKSOURCE_WATCHDOG */
 
@@ -628,6 +634,11 @@ static void clocksource_select(void)
 	return __clocksource_select(false);
 }
 
+static void clocksource_select_fallback(void)
+{
+	return __clocksource_select(true);
+}
+
 #else /* !CONFIG_ARCH_USES_GETTIMEOFFSET */
 
 static inline void clocksource_select(void) { }
@@ -803,6 +814,29 @@ void clocksource_change_rating(struct clocksource *cs, int rating)
 }
 EXPORT_SYMBOL(clocksource_change_rating);
 
+/*
+ * Unbind clocksource @cs. Called with clocksource_mutex held
+ */
+static int clocksource_unbind(struct clocksource *cs)
+{
+	/*
+	 * I really can't convince myself to support this on hardware
+	 * designed by lobotomized monkeys.
+	 */
+	if (clocksource_is_watchdog(cs))
+		return -EBUSY;
+
+	if (cs == curr_clocksource) {
+		/* Select and try to install a replacement clock source */
+		clocksource_select_fallback();
+		if (curr_clocksource == cs)
+			return -EBUSY;
+	}
+	clocksource_dequeue_watchdog(cs);
+	list_del_init(&cs->list);
+	return 0;
+}
+
 /**
  * clocksource_unregister - remove a registered clocksource
  * @cs:	clocksource to be unregistered
@@ -884,6 +918,40 @@ static ssize_t sysfs_override_clocksource(struct device *dev,
 }
 
 /**
+ * sysfs_unbind_current_clocksource - interface for manually unbinding clocksource
+ * @dev:	unused
+ * @attr:	unused
+ * @buf:	unused
+ * @count:	length of buffer
+ *
+ * Takes input from sysfs interface for manually unbinding a clocksource.
+ */
+static ssize_t sysfs_unbind_clocksource(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t count)
+{
+	struct clocksource *cs;
+	char name[CS_NAME_LEN];
+	size_t ret;
+
+	ret = clocksource_get_uname(buf, name, count);
+	if (ret < 0)
+		return ret;
+
+	ret = -ENODEV;
+	mutex_lock(&clocksource_mutex);
+	list_for_each_entry(cs, &clocksource_list, list) {
+		if (strcmp(cs->name, name))
+			continue;
+		ret = clocksource_unbind(cs);
+		break;
+	}
+	mutex_unlock(&clocksource_mutex);
+
+	return ret ? ret : count;
+}
+
+/**
  * sysfs_show_available_clocksources - sysfs interface for listing clocksource
  * @dev:	unused
  * @attr:	unused
@@ -925,6 +993,8 @@ sysfs_show_available_clocksources(struct device *dev,
 static DEVICE_ATTR(current_clocksource, 0644, sysfs_show_current_clocksources,
 		   sysfs_override_clocksource);
 
+static DEVICE_ATTR(unbind_clocksource, 0200, NULL, sysfs_unbind_clocksource);
+
 static DEVICE_ATTR(available_clocksource, 0444,
 		   sysfs_show_available_clocksources, NULL);
 
@@ -949,6 +1019,9 @@ static int __init init_clocksource_sysfs(void)
 				&device_clocksource,
 				&dev_attr_current_clocksource);
 	if (!error)
+		error = device_create_file(&device_clocksource,
+					   &dev_attr_unbind_clocksource);
+	if (!error)
 		error = device_create_file(
 				&device_clocksource,
 				&dev_attr_available_clocksource);

[patch 09/15] clockevents: Get rid of the notifier chain

[Thomas Gleixner]

[-- Attachment #1: clockevents-get-rid-of-the-notifier.patch --]
[-- Type: text/plain, Size: 7814 bytes --]

7+ years and still a single user. Kill it.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/clockchips.h   |    1 -
 kernel/time/clockevents.c    |   35 +++--------------------------------
 kernel/time/tick-broadcast.c |    5 ++---
 kernel/time/tick-common.c    |   30 +++++-------------------------
 kernel/time/tick-internal.h  |    7 ++++---
 5 files changed, 14 insertions(+), 64 deletions(-)

Index: tip/include/linux/clockchips.h
===================================================================
--- tip.orig/include/linux/clockchips.h
+++ tip/include/linux/clockchips.h
@@ -150,7 +150,6 @@ extern void clockevents_exchange_device(
 					struct clock_event_device *new);
 extern void clockevents_set_mode(struct clock_event_device *dev,
 				 enum clock_event_mode mode);
-extern int clockevents_register_notifier(struct notifier_block *nb);
 extern int clockevents_program_event(struct clock_event_device *dev,
 				     ktime_t expires, bool force);
 
Index: tip/kernel/time/clockevents.c
===================================================================
--- tip.orig/kernel/time/clockevents.c
+++ tip/kernel/time/clockevents.c
@@ -15,7 +15,6 @@
 #include <linux/hrtimer.h>
 #include <linux/init.h>
 #include <linux/module.h>
-#include <linux/notifier.h>
 #include <linux/smp.h>
 
 #include "tick-internal.h"
@@ -23,10 +22,6 @@
 /* The registered clock event devices */
 static LIST_HEAD(clockevent_devices);
 static LIST_HEAD(clockevents_released);
-
-/* Notification for clock events */
-static RAW_NOTIFIER_HEAD(clockevents_chain);
-
 /* Protection for the above */
 static DEFINE_RAW_SPINLOCK(clockevents_lock);
 
@@ -232,30 +227,6 @@ int clockevents_program_event(struct clo
 	return (rc && force) ? clockevents_program_min_delta(dev) : rc;
 }
 
-/**
- * clockevents_register_notifier - register a clock events change listener
- */
-int clockevents_register_notifier(struct notifier_block *nb)
-{
-	unsigned long flags;
-	int ret;
-
-	raw_spin_lock_irqsave(&clockevents_lock, flags);
-	ret = raw_notifier_chain_register(&clockevents_chain, nb);
-	raw_spin_unlock_irqrestore(&clockevents_lock, flags);
-
-	return ret;
-}
-
-/*
- * Notify about a clock event change. Called with clockevents_lock
- * held.
- */
-static void clockevents_do_notify(unsigned long reason, void *dev)
-{
-	raw_notifier_call_chain(&clockevents_chain, reason, dev);
-}
-
 /*
  * Called after a notify add to make devices available which were
  * released from the notifier call.
@@ -269,7 +240,7 @@ static void clockevents_notify_released(
 				 struct clock_event_device, list);
 		list_del(&dev->list);
 		list_add(&dev->list, &clockevent_devices);
-		clockevents_do_notify(CLOCK_EVT_NOTIFY_ADD, dev);
+		tick_check_new_device(dev);
 	}
 }
 
@@ -290,7 +261,7 @@ void clockevents_register_device(struct 
 	raw_spin_lock_irqsave(&clockevents_lock, flags);
 
 	list_add(&dev->list, &clockevent_devices);
-	clockevents_do_notify(CLOCK_EVT_NOTIFY_ADD, dev);
+	tick_check_new_device(dev);
 	clockevents_notify_released();
 
 	raw_spin_unlock_irqrestore(&clockevents_lock, flags);
@@ -433,7 +404,7 @@ void clockevents_notify(unsigned long re
 	int cpu;
 
 	raw_spin_lock_irqsave(&clockevents_lock, flags);
-	clockevents_do_notify(reason, arg);
+	tick_notify(reason, arg);
 
 	switch (reason) {
 	case CLOCK_EVT_NOTIFY_CPU_DEAD:
Index: tip/kernel/time/tick-broadcast.c
===================================================================
--- tip.orig/kernel/time/tick-broadcast.c
+++ tip/kernel/time/tick-broadcast.c
@@ -64,7 +64,7 @@ static void tick_broadcast_start_periodi
 /*
  * Check, if the device can be utilized as broadcast device:
  */
-int tick_check_broadcast_device(struct clock_event_device *dev)
+void tick_install_broadcast_device(struct clock_event_device *dev)
 {
 	struct clock_event_device *cur = tick_broadcast_device.evtdev;
 
@@ -72,7 +72,7 @@ int tick_check_broadcast_device(struct c
 	    (tick_broadcast_device.evtdev &&
 	     tick_broadcast_device.evtdev->rating >= dev->rating) ||
 	     (dev->features & CLOCK_EVT_FEAT_C3STOP))
-		return 0;
+		return;
 
 	clockevents_exchange_device(tick_broadcast_device.evtdev, dev);
 	if (cur)
@@ -90,7 +90,6 @@ int tick_check_broadcast_device(struct c
 	 */
 	if (dev->features & CLOCK_EVT_FEAT_ONESHOT)
 		tick_clock_notify();
-	return 1;
 }
 
 /*
Index: tip/kernel/time/tick-common.c
===================================================================
--- tip.orig/kernel/time/tick-common.c
+++ tip/kernel/time/tick-common.c
@@ -205,11 +205,11 @@ static void tick_setup_device(struct tic
 /*
  * Check, if the new registered device should be used.
  */
-static int tick_check_new_device(struct clock_event_device *newdev)
+void tick_check_new_device(struct clock_event_device *newdev)
 {
 	struct clock_event_device *curdev;
 	struct tick_device *td;
-	int cpu, ret = NOTIFY_OK;
+	int cpu;
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&tick_device_lock, flags);
@@ -272,18 +272,14 @@ static int tick_check_new_device(struct 
 		tick_oneshot_notify();
 
 	raw_spin_unlock_irqrestore(&tick_device_lock, flags);
-	return NOTIFY_STOP;
+	return;
 
 out_bc:
 	/*
 	 * Can the new device be used as a broadcast device ?
 	 */
-	if (tick_check_broadcast_device(newdev))
-		ret = NOTIFY_STOP;
-
+	tick_install_broadcast_device(newdev);
 	raw_spin_unlock_irqrestore(&tick_device_lock, flags);
-
-	return ret;
 }
 
 /*
@@ -357,17 +353,10 @@ static void tick_resume(void)
 	raw_spin_unlock_irqrestore(&tick_device_lock, flags);
 }
 
-/*
- * Notification about clock event devices
- */
-static int tick_notify(struct notifier_block *nb, unsigned long reason,
-			       void *dev)
+void tick_notify(unsigned long reason, void *dev)
 {
 	switch (reason) {
 
-	case CLOCK_EVT_NOTIFY_ADD:
-		return tick_check_new_device(dev);
-
 	case CLOCK_EVT_NOTIFY_BROADCAST_ON:
 	case CLOCK_EVT_NOTIFY_BROADCAST_OFF:
 	case CLOCK_EVT_NOTIFY_BROADCAST_FORCE:
@@ -401,21 +390,12 @@ static int tick_notify(struct notifier_b
 	default:
 		break;
 	}
-
-	return NOTIFY_OK;
 }
 
-static struct notifier_block tick_notifier = {
-	.notifier_call = tick_notify,
-};
-
 /**
  * tick_init - initialize the tick control
- *
- * Register the notifier with the clockevents framework
  */
 void __init tick_init(void)
 {
-	clockevents_register_notifier(&tick_notifier);
 	tick_broadcast_init();
 }
Index: tip/kernel/time/tick-internal.h
===================================================================
--- tip.orig/kernel/time/tick-internal.h
+++ tip/kernel/time/tick-internal.h
@@ -18,6 +18,8 @@ extern int tick_do_timer_cpu __read_most
 
 extern void tick_setup_periodic(struct clock_event_device *dev, int broadcast);
 extern void tick_handle_periodic(struct clock_event_device *dev);
+extern void tick_notify(unsigned long reason, void *dev);
+extern void tick_check_new_device(struct clock_event_device *dev);
 
 extern void clockevents_shutdown(struct clock_event_device *dev);
 
@@ -90,7 +92,7 @@ static inline bool tick_broadcast_onesho
  */
 #ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 extern int tick_device_uses_broadcast(struct clock_event_device *dev, int cpu);
-extern int tick_check_broadcast_device(struct clock_event_device *dev);
+extern void tick_install_broadcast_device(struct clock_event_device *dev);
 extern int tick_is_broadcast_device(struct clock_event_device *dev);
 extern void tick_broadcast_on_off(unsigned long reason, int *oncpu);
 extern void tick_shutdown_broadcast(unsigned int *cpup);
@@ -102,9 +104,8 @@ tick_set_periodic_handler(struct clock_e
 
 #else /* !BROADCAST */
 
-static inline int tick_check_broadcast_device(struct clock_event_device *dev)
+static inline void tick_install_broadcast_device(struct clock_event_device *dev)
 {
-	return 0;
 }
 
 static inline int tick_is_broadcast_device(struct clock_event_device *dev)

[tip:timers/core] clockevents: Get rid of the notifier chain

[tip-bot for Thomas Gleixner]

Commit-ID:  7172a286ced0c1f4f239a0fa09db54ed37d3ead2
Gitweb:     http://git.kernel.org/tip/7172a286ced0c1f4f239a0fa09db54ed37d3ead2
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 25 Apr 2013 20:31:47 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 16 May 2013 11:09:16 +0200

clockevents: Get rid of the notifier chain

7+ years and still a single user. Kill it.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Link: http://lkml.kernel.org/r/20130425143436.098520211@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/clockchips.h   |  1 -
 kernel/time/clockevents.c    | 35 +++--------------------------------
 kernel/time/tick-broadcast.c |  5 ++---
 kernel/time/tick-common.c    | 30 +++++-------------------------
 kernel/time/tick-internal.h  |  7 ++++---
 5 files changed, 14 insertions(+), 64 deletions(-)

diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h
index 963d714..2f498f6 100644
--- a/include/linux/clockchips.h
+++ b/include/linux/clockchips.h
@@ -150,7 +150,6 @@ extern void clockevents_exchange_device(struct clock_event_device *old,
 					struct clock_event_device *new);
 extern void clockevents_set_mode(struct clock_event_device *dev,
 				 enum clock_event_mode mode);
-extern int clockevents_register_notifier(struct notifier_block *nb);
 extern int clockevents_program_event(struct clock_event_device *dev,
 				     ktime_t expires, bool force);
 
diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index c6d6400..dd70b48 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -15,7 +15,6 @@
 #include <linux/hrtimer.h>
 #include <linux/init.h>
 #include <linux/module.h>
-#include <linux/notifier.h>
 #include <linux/smp.h>
 
 #include "tick-internal.h"
@@ -23,10 +22,6 @@
 /* The registered clock event devices */
 static LIST_HEAD(clockevent_devices);
 static LIST_HEAD(clockevents_released);
-
-/* Notification for clock events */
-static RAW_NOTIFIER_HEAD(clockevents_chain);
-
 /* Protection for the above */
 static DEFINE_RAW_SPINLOCK(clockevents_lock);
 
@@ -232,30 +227,6 @@ int clockevents_program_event(struct clock_event_device *dev, ktime_t expires,
 	return (rc && force) ? clockevents_program_min_delta(dev) : rc;
 }
 
-/**
- * clockevents_register_notifier - register a clock events change listener
- */
-int clockevents_register_notifier(struct notifier_block *nb)
-{
-	unsigned long flags;
-	int ret;
-
-	raw_spin_lock_irqsave(&clockevents_lock, flags);
-	ret = raw_notifier_chain_register(&clockevents_chain, nb);
-	raw_spin_unlock_irqrestore(&clockevents_lock, flags);
-
-	return ret;
-}
-
-/*
- * Notify about a clock event change. Called with clockevents_lock
- * held.
- */
-static void clockevents_do_notify(unsigned long reason, void *dev)
-{
-	raw_notifier_call_chain(&clockevents_chain, reason, dev);
-}
-
 /*
  * Called after a notify add to make devices available which were
  * released from the notifier call.
@@ -269,7 +240,7 @@ static void clockevents_notify_released(void)
 				 struct clock_event_device, list);
 		list_del(&dev->list);
 		list_add(&dev->list, &clockevent_devices);
-		clockevents_do_notify(CLOCK_EVT_NOTIFY_ADD, dev);
+		tick_check_new_device(dev);
 	}
 }
 
@@ -290,7 +261,7 @@ void clockevents_register_device(struct clock_event_device *dev)
 	raw_spin_lock_irqsave(&clockevents_lock, flags);
 
 	list_add(&dev->list, &clockevent_devices);
-	clockevents_do_notify(CLOCK_EVT_NOTIFY_ADD, dev);
+	tick_check_new_device(dev);
 	clockevents_notify_released();
 
 	raw_spin_unlock_irqrestore(&clockevents_lock, flags);
@@ -433,7 +404,7 @@ void clockevents_notify(unsigned long reason, void *arg)
 	int cpu;
 
 	raw_spin_lock_irqsave(&clockevents_lock, flags);
-	clockevents_do_notify(reason, arg);
+	tick_notify(reason, arg);
 
 	switch (reason) {
 	case CLOCK_EVT_NOTIFY_CPU_DEAD:
diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index 24938d5..3500caa 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -64,7 +64,7 @@ static void tick_broadcast_start_periodic(struct clock_event_device *bc)
 /*
  * Check, if the device can be utilized as broadcast device:
  */
-int tick_check_broadcast_device(struct clock_event_device *dev)
+void tick_install_broadcast_device(struct clock_event_device *dev)
 {
 	struct clock_event_device *cur = tick_broadcast_device.evtdev;
 
@@ -72,7 +72,7 @@ int tick_check_broadcast_device(struct clock_event_device *dev)
 	    (tick_broadcast_device.evtdev &&
 	     tick_broadcast_device.evtdev->rating >= dev->rating) ||
 	     (dev->features & CLOCK_EVT_FEAT_C3STOP))
-		return 0;
+		return;
 
 	clockevents_exchange_device(tick_broadcast_device.evtdev, dev);
 	if (cur)
@@ -90,7 +90,6 @@ int tick_check_broadcast_device(struct clock_event_device *dev)
 	 */
 	if (dev->features & CLOCK_EVT_FEAT_ONESHOT)
 		tick_clock_notify();
-	return 1;
 }
 
 /*
diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index 5d3fb10..dbf4e18 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -208,11 +208,11 @@ static void tick_setup_device(struct tick_device *td,
 /*
  * Check, if the new registered device should be used.
  */
-static int tick_check_new_device(struct clock_event_device *newdev)
+void tick_check_new_device(struct clock_event_device *newdev)
 {
 	struct clock_event_device *curdev;
 	struct tick_device *td;
-	int cpu, ret = NOTIFY_OK;
+	int cpu;
 	unsigned long flags;
 
 	raw_spin_lock_irqsave(&tick_device_lock, flags);
@@ -275,18 +275,14 @@ static int tick_check_new_device(struct clock_event_device *newdev)
 		tick_oneshot_notify();
 
 	raw_spin_unlock_irqrestore(&tick_device_lock, flags);
-	return NOTIFY_STOP;
+	return;
 
 out_bc:
 	/*
 	 * Can the new device be used as a broadcast device ?
 	 */
-	if (tick_check_broadcast_device(newdev))
-		ret = NOTIFY_STOP;
-
+	tick_install_broadcast_device(newdev);
 	raw_spin_unlock_irqrestore(&tick_device_lock, flags);
-
-	return ret;
 }
 
 /*
@@ -360,17 +356,10 @@ static void tick_resume(void)
 	raw_spin_unlock_irqrestore(&tick_device_lock, flags);
 }
 
-/*
- * Notification about clock event devices
- */
-static int tick_notify(struct notifier_block *nb, unsigned long reason,
-			       void *dev)
+void tick_notify(unsigned long reason, void *dev)
 {
 	switch (reason) {
 
-	case CLOCK_EVT_NOTIFY_ADD:
-		return tick_check_new_device(dev);
-
 	case CLOCK_EVT_NOTIFY_BROADCAST_ON:
 	case CLOCK_EVT_NOTIFY_BROADCAST_OFF:
 	case CLOCK_EVT_NOTIFY_BROADCAST_FORCE:
@@ -404,21 +393,12 @@ static int tick_notify(struct notifier_block *nb, unsigned long reason,
 	default:
 		break;
 	}
-
-	return NOTIFY_OK;
 }
 
-static struct notifier_block tick_notifier = {
-	.notifier_call = tick_notify,
-};
-
 /**
  * tick_init - initialize the tick control
- *
- * Register the notifier with the clockevents framework
  */
 void __init tick_init(void)
 {
-	clockevents_register_notifier(&tick_notifier);
 	tick_broadcast_init();
 }
diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h
index f0299ea..60742fe 100644
--- a/kernel/time/tick-internal.h
+++ b/kernel/time/tick-internal.h
@@ -18,6 +18,8 @@ extern int tick_do_timer_cpu __read_mostly;
 
 extern void tick_setup_periodic(struct clock_event_device *dev, int broadcast);
 extern void tick_handle_periodic(struct clock_event_device *dev);
+extern void tick_notify(unsigned long reason, void *dev);
+extern void tick_check_new_device(struct clock_event_device *dev);
 
 extern void clockevents_shutdown(struct clock_event_device *dev);
 
@@ -90,7 +92,7 @@ static inline bool tick_broadcast_oneshot_available(void) { return false; }
  */
 #ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 extern int tick_device_uses_broadcast(struct clock_event_device *dev, int cpu);
-extern int tick_check_broadcast_device(struct clock_event_device *dev);
+extern void tick_install_broadcast_device(struct clock_event_device *dev);
 extern int tick_is_broadcast_device(struct clock_event_device *dev);
 extern void tick_broadcast_on_off(unsigned long reason, int *oncpu);
 extern void tick_shutdown_broadcast(unsigned int *cpup);
@@ -102,9 +104,8 @@ tick_set_periodic_handler(struct clock_event_device *dev, int broadcast);
 
 #else /* !BROADCAST */
 
-static inline int tick_check_broadcast_device(struct clock_event_device *dev)
+static inline void tick_install_broadcast_device(struct clock_event_device *dev)
 {
-	return 0;
 }
 
 static inline int tick_is_broadcast_device(struct clock_event_device *dev)

[patch 11/15] clockevents: Move the tick_notify() switch case to clockevents_notify()

[Thomas Gleixner]

[-- Attachment #1: clockevents-move-the-external-notifier-to-clockevents.patch --]
[-- Type: text/plain, Size: 4392 bytes --]

No need to call another function and have duplicated cases.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/clockevents.c   |   28 +++++++++++++++++++++++-
 kernel/time/tick-common.c   |   50 +++-----------------------------------------
 kernel/time/tick-internal.h |    5 +++-
 3 files changed, 35 insertions(+), 48 deletions(-)

Index: tip/kernel/time/clockevents.c
===================================================================
--- tip.orig/kernel/time/clockevents.c
+++ tip/kernel/time/clockevents.c
@@ -404,10 +404,36 @@ void clockevents_notify(unsigned long re
 	int cpu;
 
 	raw_spin_lock_irqsave(&clockevents_lock, flags);
-	tick_notify(reason, arg);
 
 	switch (reason) {
+	case CLOCK_EVT_NOTIFY_BROADCAST_ON:
+	case CLOCK_EVT_NOTIFY_BROADCAST_OFF:
+	case CLOCK_EVT_NOTIFY_BROADCAST_FORCE:
+		tick_broadcast_on_off(reason, arg);
+		break;
+
+	case CLOCK_EVT_NOTIFY_BROADCAST_ENTER:
+	case CLOCK_EVT_NOTIFY_BROADCAST_EXIT:
+		tick_broadcast_oneshot_control(reason);
+		break;
+
+	case CLOCK_EVT_NOTIFY_CPU_DYING:
+		tick_handover_do_timer(arg);
+		break;
+
+	case CLOCK_EVT_NOTIFY_SUSPEND:
+		tick_suspend();
+		tick_suspend_broadcast();
+		break;
+
+	case CLOCK_EVT_NOTIFY_RESUME:
+		tick_resume();
+		break;
+
 	case CLOCK_EVT_NOTIFY_CPU_DEAD:
+		tick_shutdown_broadcast_oneshot(arg);
+		tick_shutdown_broadcast(arg);
+		tick_shutdown(arg);
 		/*
 		 * Unregister the clock event devices which were
 		 * released from the users in the notify chain.
Index: tip/kernel/time/tick-common.c
===================================================================
--- tip.orig/kernel/time/tick-common.c
+++ tip/kernel/time/tick-common.c
@@ -281,7 +281,7 @@ out_bc:
  *
  * Called with interrupts disabled.
  */
-static void tick_handover_do_timer(int *cpup)
+void tick_handover_do_timer(int *cpup)
 {
 	if (*cpup == tick_do_timer_cpu) {
 		int cpu = cpumask_first(cpu_online_mask);
@@ -298,7 +298,7 @@ static void tick_handover_do_timer(int *
  * access the hardware device itself.
  * We just set the mode and remove it from the lists.
  */
-static void tick_shutdown(unsigned int *cpup)
+void tick_shutdown(unsigned int *cpup)
 {
 	struct tick_device *td = &per_cpu(tick_cpu_device, *cpup);
 	struct clock_event_device *dev = td->evtdev;
@@ -316,14 +316,14 @@ static void tick_shutdown(unsigned int *
 	}
 }
 
-static void tick_suspend(void)
+void tick_suspend(void)
 {
 	struct tick_device *td = &__get_cpu_var(tick_cpu_device);
 
 	clockevents_shutdown(td->evtdev);
 }
 
-static void tick_resume(void)
+void tick_resume(void)
 {
 	struct tick_device *td = &__get_cpu_var(tick_cpu_device);
 	int broadcast = tick_resume_broadcast();
@@ -338,48 +338,6 @@ static void tick_resume(void)
 	}
 }
 
-/*
- * Called with clockevents_lock held and interrupts disabled
- */
-void tick_notify(unsigned long reason, void *dev)
-{
-	switch (reason) {
-
-	case CLOCK_EVT_NOTIFY_BROADCAST_ON:
-	case CLOCK_EVT_NOTIFY_BROADCAST_OFF:
-	case CLOCK_EVT_NOTIFY_BROADCAST_FORCE:
-		tick_broadcast_on_off(reason, dev);
-		break;
-
-	case CLOCK_EVT_NOTIFY_BROADCAST_ENTER:
-	case CLOCK_EVT_NOTIFY_BROADCAST_EXIT:
-		tick_broadcast_oneshot_control(reason);
-		break;
-
-	case CLOCK_EVT_NOTIFY_CPU_DYING:
-		tick_handover_do_timer(dev);
-		break;
-
-	case CLOCK_EVT_NOTIFY_CPU_DEAD:
-		tick_shutdown_broadcast_oneshot(dev);
-		tick_shutdown_broadcast(dev);
-		tick_shutdown(dev);
-		break;
-
-	case CLOCK_EVT_NOTIFY_SUSPEND:
-		tick_suspend();
-		tick_suspend_broadcast();
-		break;
-
-	case CLOCK_EVT_NOTIFY_RESUME:
-		tick_resume();
-		break;
-
-	default:
-		break;
-	}
-}
-
 /**
  * tick_init - initialize the tick control
  */
Index: tip/kernel/time/tick-internal.h
===================================================================
--- tip.orig/kernel/time/tick-internal.h
+++ tip/kernel/time/tick-internal.h
@@ -18,8 +18,11 @@ extern int tick_do_timer_cpu __read_most
 
 extern void tick_setup_periodic(struct clock_event_device *dev, int broadcast);
 extern void tick_handle_periodic(struct clock_event_device *dev);
-extern void tick_notify(unsigned long reason, void *dev);
 extern void tick_check_new_device(struct clock_event_device *dev);
+extern void tick_handover_do_timer(int *cpup);
+extern void tick_shutdown(unsigned int *cpup);
+extern void tick_suspend(void);
+extern void tick_resume(void);
 
 extern void clockevents_shutdown(struct clock_event_device *dev);
 

[tip:timers/core] clockevents: Move the tick_notify() switch case to clockevents_notify()

[tip-bot for Thomas Gleixner]

Commit-ID:  8c53daf63f56791ed47fc585206ef3049489612f
Gitweb:     http://git.kernel.org/tip/8c53daf63f56791ed47fc585206ef3049489612f
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 25 Apr 2013 20:31:48 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 16 May 2013 11:09:16 +0200

clockevents: Move the tick_notify() switch case to clockevents_notify()

No need to call another function and have duplicated cases.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Link: http://lkml.kernel.org/r/20130425143436.235746557@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/clockevents.c   | 28 ++++++++++++++++++++++++-
 kernel/time/tick-common.c   | 50 ++++-----------------------------------------
 kernel/time/tick-internal.h |  5 ++++-
 3 files changed, 35 insertions(+), 48 deletions(-)

diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index dd70b48..0e3a844 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -404,10 +404,36 @@ void clockevents_notify(unsigned long reason, void *arg)
 	int cpu;
 
 	raw_spin_lock_irqsave(&clockevents_lock, flags);
-	tick_notify(reason, arg);
 
 	switch (reason) {
+	case CLOCK_EVT_NOTIFY_BROADCAST_ON:
+	case CLOCK_EVT_NOTIFY_BROADCAST_OFF:
+	case CLOCK_EVT_NOTIFY_BROADCAST_FORCE:
+		tick_broadcast_on_off(reason, arg);
+		break;
+
+	case CLOCK_EVT_NOTIFY_BROADCAST_ENTER:
+	case CLOCK_EVT_NOTIFY_BROADCAST_EXIT:
+		tick_broadcast_oneshot_control(reason);
+		break;
+
+	case CLOCK_EVT_NOTIFY_CPU_DYING:
+		tick_handover_do_timer(arg);
+		break;
+
+	case CLOCK_EVT_NOTIFY_SUSPEND:
+		tick_suspend();
+		tick_suspend_broadcast();
+		break;
+
+	case CLOCK_EVT_NOTIFY_RESUME:
+		tick_resume();
+		break;
+
 	case CLOCK_EVT_NOTIFY_CPU_DEAD:
+		tick_shutdown_broadcast_oneshot(arg);
+		tick_shutdown_broadcast(arg);
+		tick_shutdown(arg);
 		/*
 		 * Unregister the clock event devices which were
 		 * released from the users in the notify chain.
diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index 170a4bd..84c7cfc 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -284,7 +284,7 @@ out_bc:
  *
  * Called with interrupts disabled.
  */
-static void tick_handover_do_timer(int *cpup)
+void tick_handover_do_timer(int *cpup)
 {
 	if (*cpup == tick_do_timer_cpu) {
 		int cpu = cpumask_first(cpu_online_mask);
@@ -301,7 +301,7 @@ static void tick_handover_do_timer(int *cpup)
  * access the hardware device itself.
  * We just set the mode and remove it from the lists.
  */
-static void tick_shutdown(unsigned int *cpup)
+void tick_shutdown(unsigned int *cpup)
 {
 	struct tick_device *td = &per_cpu(tick_cpu_device, *cpup);
 	struct clock_event_device *dev = td->evtdev;
@@ -319,14 +319,14 @@ static void tick_shutdown(unsigned int *cpup)
 	}
 }
 
-static void tick_suspend(void)
+void tick_suspend(void)
 {
 	struct tick_device *td = &__get_cpu_var(tick_cpu_device);
 
 	clockevents_shutdown(td->evtdev);
 }
 
-static void tick_resume(void)
+void tick_resume(void)
 {
 	struct tick_device *td = &__get_cpu_var(tick_cpu_device);
 	int broadcast = tick_resume_broadcast();
@@ -341,48 +341,6 @@ static void tick_resume(void)
 	}
 }
 
-/*
- * Called with clockevents_lock held and interrupts disabled
- */
-void tick_notify(unsigned long reason, void *dev)
-{
-	switch (reason) {
-
-	case CLOCK_EVT_NOTIFY_BROADCAST_ON:
-	case CLOCK_EVT_NOTIFY_BROADCAST_OFF:
-	case CLOCK_EVT_NOTIFY_BROADCAST_FORCE:
-		tick_broadcast_on_off(reason, dev);
-		break;
-
-	case CLOCK_EVT_NOTIFY_BROADCAST_ENTER:
-	case CLOCK_EVT_NOTIFY_BROADCAST_EXIT:
-		tick_broadcast_oneshot_control(reason);
-		break;
-
-	case CLOCK_EVT_NOTIFY_CPU_DYING:
-		tick_handover_do_timer(dev);
-		break;
-
-	case CLOCK_EVT_NOTIFY_CPU_DEAD:
-		tick_shutdown_broadcast_oneshot(dev);
-		tick_shutdown_broadcast(dev);
-		tick_shutdown(dev);
-		break;
-
-	case CLOCK_EVT_NOTIFY_SUSPEND:
-		tick_suspend();
-		tick_suspend_broadcast();
-		break;
-
-	case CLOCK_EVT_NOTIFY_RESUME:
-		tick_resume();
-		break;
-
-	default:
-		break;
-	}
-}
-
 /**
  * tick_init - initialize the tick control
  */
diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h
index 60742fe..06bfc88 100644
--- a/kernel/time/tick-internal.h
+++ b/kernel/time/tick-internal.h
@@ -18,8 +18,11 @@ extern int tick_do_timer_cpu __read_mostly;
 
 extern void tick_setup_periodic(struct clock_event_device *dev, int broadcast);
 extern void tick_handle_periodic(struct clock_event_device *dev);
-extern void tick_notify(unsigned long reason, void *dev);
 extern void tick_check_new_device(struct clock_event_device *dev);
+extern void tick_handover_do_timer(int *cpup);
+extern void tick_shutdown(unsigned int *cpup);
+extern void tick_suspend(void);
+extern void tick_resume(void);
 
 extern void clockevents_shutdown(struct clock_event_device *dev);
 

[patch 10/15] clockevents: Simplify locking

[Thomas Gleixner]

[-- Attachment #1: clockevents-simplify-locking.patch --]
[-- Type: text/plain, Size: 3136 bytes --]

Now that the notifier chain is gone there are no other users and it's
pointless to nest tick_device_lock inside of clockevents_lock because
there is no other use case.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/tick-common.c |   22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

Index: tip/kernel/time/tick-common.c
===================================================================
--- tip.orig/kernel/time/tick-common.c
+++ tip/kernel/time/tick-common.c
@@ -33,7 +33,6 @@ DEFINE_PER_CPU(struct tick_device, tick_
 ktime_t tick_next_period;
 ktime_t tick_period;
 int tick_do_timer_cpu __read_mostly = TICK_DO_TIMER_BOOT;
-static DEFINE_RAW_SPINLOCK(tick_device_lock);
 
 /*
  * Debugging: see timer_list.c
@@ -203,16 +202,14 @@ static void tick_setup_device(struct tic
 }
 
 /*
- * Check, if the new registered device should be used.
+ * Check, if the new registered device should be used. Called with
+ * clockevents_lock held and interrupts disabled.
  */
 void tick_check_new_device(struct clock_event_device *newdev)
 {
 	struct clock_event_device *curdev;
 	struct tick_device *td;
 	int cpu;
-	unsigned long flags;
-
-	raw_spin_lock_irqsave(&tick_device_lock, flags);
 
 	cpu = smp_processor_id();
 	if (!cpumask_test_cpu(cpu, newdev->cpumask))
@@ -270,8 +267,6 @@ void tick_check_new_device(struct clock_
 	tick_setup_device(td, newdev, cpu, cpumask_of(cpu));
 	if (newdev->features & CLOCK_EVT_FEAT_ONESHOT)
 		tick_oneshot_notify();
-
-	raw_spin_unlock_irqrestore(&tick_device_lock, flags);
 	return;
 
 out_bc:
@@ -279,7 +274,6 @@ out_bc:
 	 * Can the new device be used as a broadcast device ?
 	 */
 	tick_install_broadcast_device(newdev);
-	raw_spin_unlock_irqrestore(&tick_device_lock, flags);
 }
 
 /*
@@ -308,9 +302,7 @@ static void tick_shutdown(unsigned int *
 {
 	struct tick_device *td = &per_cpu(tick_cpu_device, *cpup);
 	struct clock_event_device *dev = td->evtdev;
-	unsigned long flags;
 
-	raw_spin_lock_irqsave(&tick_device_lock, flags);
 	td->mode = TICKDEV_MODE_PERIODIC;
 	if (dev) {
 		/*
@@ -322,26 +314,20 @@ static void tick_shutdown(unsigned int *
 		dev->event_handler = clockevents_handle_noop;
 		td->evtdev = NULL;
 	}
-	raw_spin_unlock_irqrestore(&tick_device_lock, flags);
 }
 
 static void tick_suspend(void)
 {
 	struct tick_device *td = &__get_cpu_var(tick_cpu_device);
-	unsigned long flags;
 
-	raw_spin_lock_irqsave(&tick_device_lock, flags);
 	clockevents_shutdown(td->evtdev);
-	raw_spin_unlock_irqrestore(&tick_device_lock, flags);
 }
 
 static void tick_resume(void)
 {
 	struct tick_device *td = &__get_cpu_var(tick_cpu_device);
-	unsigned long flags;
 	int broadcast = tick_resume_broadcast();
 
-	raw_spin_lock_irqsave(&tick_device_lock, flags);
 	clockevents_set_mode(td->evtdev, CLOCK_EVT_MODE_RESUME);
 
 	if (!broadcast) {
@@ -350,9 +336,11 @@ static void tick_resume(void)
 		else
 			tick_resume_oneshot();
 	}
-	raw_spin_unlock_irqrestore(&tick_device_lock, flags);
 }
 
+/*
+ * Called with clockevents_lock held and interrupts disabled
+ */
 void tick_notify(unsigned long reason, void *dev)
 {
 	switch (reason) {

[tip:timers/core] clockevents: Simplify locking

[tip-bot for Thomas Gleixner]

Commit-ID:  7126cac426137633e470167524e7bcb590fd49b3
Gitweb:     http://git.kernel.org/tip/7126cac426137633e470167524e7bcb590fd49b3
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 25 Apr 2013 20:31:48 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 16 May 2013 11:09:16 +0200

clockevents: Simplify locking

Now that the notifier chain is gone there are no other users and it's
pointless to nest tick_device_lock inside of clockevents_lock because
there is no other use case.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Link: http://lkml.kernel.org/r/20130425143436.162888472@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/tick-common.c | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index dbf4e18..170a4bd 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -33,7 +33,6 @@ DEFINE_PER_CPU(struct tick_device, tick_cpu_device);
 ktime_t tick_next_period;
 ktime_t tick_period;
 int tick_do_timer_cpu __read_mostly = TICK_DO_TIMER_BOOT;
-static DEFINE_RAW_SPINLOCK(tick_device_lock);
 
 /*
  * Debugging: see timer_list.c
@@ -206,16 +205,14 @@ static void tick_setup_device(struct tick_device *td,
 }
 
 /*
- * Check, if the new registered device should be used.
+ * Check, if the new registered device should be used. Called with
+ * clockevents_lock held and interrupts disabled.
  */
 void tick_check_new_device(struct clock_event_device *newdev)
 {
 	struct clock_event_device *curdev;
 	struct tick_device *td;
 	int cpu;
-	unsigned long flags;
-
-	raw_spin_lock_irqsave(&tick_device_lock, flags);
 
 	cpu = smp_processor_id();
 	if (!cpumask_test_cpu(cpu, newdev->cpumask))
@@ -273,8 +270,6 @@ void tick_check_new_device(struct clock_event_device *newdev)
 	tick_setup_device(td, newdev, cpu, cpumask_of(cpu));
 	if (newdev->features & CLOCK_EVT_FEAT_ONESHOT)
 		tick_oneshot_notify();
-
-	raw_spin_unlock_irqrestore(&tick_device_lock, flags);
 	return;
 
 out_bc:
@@ -282,7 +277,6 @@ out_bc:
 	 * Can the new device be used as a broadcast device ?
 	 */
 	tick_install_broadcast_device(newdev);
-	raw_spin_unlock_irqrestore(&tick_device_lock, flags);
 }
 
 /*
@@ -311,9 +305,7 @@ static void tick_shutdown(unsigned int *cpup)
 {
 	struct tick_device *td = &per_cpu(tick_cpu_device, *cpup);
 	struct clock_event_device *dev = td->evtdev;
-	unsigned long flags;
 
-	raw_spin_lock_irqsave(&tick_device_lock, flags);
 	td->mode = TICKDEV_MODE_PERIODIC;
 	if (dev) {
 		/*
@@ -325,26 +317,20 @@ static void tick_shutdown(unsigned int *cpup)
 		dev->event_handler = clockevents_handle_noop;
 		td->evtdev = NULL;
 	}
-	raw_spin_unlock_irqrestore(&tick_device_lock, flags);
 }
 
 static void tick_suspend(void)
 {
 	struct tick_device *td = &__get_cpu_var(tick_cpu_device);
-	unsigned long flags;
 
-	raw_spin_lock_irqsave(&tick_device_lock, flags);
 	clockevents_shutdown(td->evtdev);
-	raw_spin_unlock_irqrestore(&tick_device_lock, flags);
 }
 
 static void tick_resume(void)
 {
 	struct tick_device *td = &__get_cpu_var(tick_cpu_device);
-	unsigned long flags;
 	int broadcast = tick_resume_broadcast();
 
-	raw_spin_lock_irqsave(&tick_device_lock, flags);
 	clockevents_set_mode(td->evtdev, CLOCK_EVT_MODE_RESUME);
 
 	if (!broadcast) {
@@ -353,9 +339,11 @@ static void tick_resume(void)
 		else
 			tick_resume_oneshot();
 	}
-	raw_spin_unlock_irqrestore(&tick_device_lock, flags);
 }
 
+/*
+ * Called with clockevents_lock held and interrupts disabled
+ */
 void tick_notify(unsigned long reason, void *dev)
 {
 	switch (reason) {

[patch 13/15] clockevents: Provide sysfs interface

[Thomas Gleixner]

[-- Attachment #1: clockevents-provide-sysfs-interface.patch --]
[-- Type: text/plain, Size: 2879 bytes --]

Provide a simple sysfs interface for the clockevent devices. Show the
current active clockevent device.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/clockevents.c |   86 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)

Index: tip/kernel/time/clockevents.c
===================================================================
--- tip.orig/kernel/time/clockevents.c
+++ tip/kernel/time/clockevents.c
@@ -16,6 +16,7 @@
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/smp.h>
+#include <linux/device.h>
 
 #include "tick-internal.h"
 
@@ -460,4 +461,89 @@ void clockevents_notify(unsigned long re
 	raw_spin_unlock_irqrestore(&clockevents_lock, flags);
 }
 EXPORT_SYMBOL_GPL(clockevents_notify);
+
+#ifdef CONFIG_SYSFS
+struct bus_type clockevents_subsys = {
+	.name		= "clockevents",
+	.dev_name       = "clockevent",
+};
+
+static DEFINE_PER_CPU(struct device, tick_percpu_dev);
+static struct tick_device *tick_get_tick_dev(struct device *dev);
+
+static ssize_t sysfs_show_current_tick_dev(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf)
+{
+	struct tick_device *td;
+	ssize_t count = 0;
+
+	raw_spin_lock_irq(&clockevents_lock);
+	td = tick_get_tick_dev(dev);
+	if (td && td->evtdev)
+		count = snprintf(buf, PAGE_SIZE, "%s\n", td->evtdev->name);
+	raw_spin_unlock_irq(&clockevents_lock);
+	return count;
+}
+static DEVICE_ATTR(current_device, 0444, sysfs_show_current_tick_dev, NULL);
+
+#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
+static struct device tick_bc_dev = {
+	.init_name	= "broadcast",
+	.id		= 0,
+	.bus		= &clockevents_subsys,
+};
+
+static struct tick_device *tick_get_tick_dev(struct device *dev)
+{
+	return dev == &tick_bc_dev ? tick_get_broadcast_device() :
+		&per_cpu(tick_cpu_device, dev->id);
+}
+
+static int tick_broadcast_init_sysfs(void)
+{
+	int err = device_register(&tick_bc_dev);
+
+	if (!err)
+		err = device_create_file(&tick_bc_dev, &dev_attr_current_device);
+	return err;
+}
+#else
+static struct tick_device *tick_get_tick_dev(struct device *dev)
+{
+	return &per_cpu(tick_cpu_device, dev->id);
+}
+static inline int tick_broadcast_init_sysfs(void) { return 0; }
 #endif
+
+int __init tick_init_sysfs(void)
+{
+	int cpu;
+
+	for_each_possible_cpu(cpu) {
+		struct device *dev = &per_cpu(tick_percpu_dev, cpu);
+		int err;
+
+		dev->id = cpu;
+		dev->bus = &clockevents_subsys;
+		err = device_register(dev);
+		if (!err)
+			err = device_create_file(dev, &dev_attr_current_device);
+		if (err)
+			return err;
+	}
+	return tick_broadcast_init_sysfs();
+}
+
+static int __init clockevents_init_sysfs(void)
+{
+	int err = subsys_system_register(&clockevents_subsys, NULL);
+
+	if (!err)
+		err = tick_init_sysfs();
+	return err;
+}
+device_initcall(clockevents_init_sysfs);
+#endif /* SYSFS */
+
+#endif /* GENERIC_CLOCK_EVENTS */

Re: [patch 13/15] clockevents: Provide sysfs interface

[Stephen Boyd]

On 04/25, Thomas Gleixner wrote:
> Provide a simple sysfs interface for the clockevent devices. Show the
> current active clockevent device.
> 

Neat. Does this do anything about clockevents that aren't in use
for the tick devices or broadcast device?

> Index: tip/kernel/time/clockevents.c
> ===================================================================
> --- tip.orig/kernel/time/clockevents.c
> +++ tip/kernel/time/clockevents.c
> @@ -460,4 +461,89 @@ void clockevents_notify(unsigned long re
>  	raw_spin_unlock_irqrestore(&clockevents_lock, flags);
>  }
>  EXPORT_SYMBOL_GPL(clockevents_notify);
> +
> +#ifdef CONFIG_SYSFS
> +struct bus_type clockevents_subsys = {
> +	.name		= "clockevents",
> +	.dev_name       = "clockevent",
> +};
> +
> +static DEFINE_PER_CPU(struct device, tick_percpu_dev);
> +static struct tick_device *tick_get_tick_dev(struct device *dev);
> +
> +static ssize_t sysfs_show_current_tick_dev(struct device *dev,
> +					   struct device_attribute *attr,
> +					   char *buf)
> +{
> +	struct tick_device *td;
> +	ssize_t count = 0;
> +
> +	raw_spin_lock_irq(&clockevents_lock);
> +	td = tick_get_tick_dev(dev);
> +	if (td && td->evtdev)
> +		count = snprintf(buf, PAGE_SIZE, "%s\n", td->evtdev->name);
> +	raw_spin_unlock_irq(&clockevents_lock);
> +	return count;
> +}
> +static DEVICE_ATTR(current_device, 0444, sysfs_show_current_tick_dev, NULL);
> +
> +#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
> +static struct device tick_bc_dev = {
> +	.init_name	= "broadcast",
> +	.id		= 0,
> +	.bus		= &clockevents_subsys,
> +};
> +
> +static struct tick_device *tick_get_tick_dev(struct device *dev)
> +{
> +	return dev == &tick_bc_dev ? tick_get_broadcast_device() :
> +		&per_cpu(tick_cpu_device, dev->id);
> +}
> +
> +static int tick_broadcast_init_sysfs(void)

__init?

> +{
> +	int err = device_register(&tick_bc_dev);
> +
> +	if (!err)
> +		err = device_create_file(&tick_bc_dev, &dev_attr_current_device);
> +	return err;
> +}
> +#else
> +static struct tick_device *tick_get_tick_dev(struct device *dev)
> +{
> +	return &per_cpu(tick_cpu_device, dev->id);
> +}
> +static inline int tick_broadcast_init_sysfs(void) { return 0; }
>  #endif
> +
> +int __init tick_init_sysfs(void)

static?

> +{
> +	int cpu;
> +
> +	for_each_possible_cpu(cpu) {
> +		struct device *dev = &per_cpu(tick_percpu_dev, cpu);
> +		int err;
> +
> +		dev->id = cpu;
> +		dev->bus = &clockevents_subsys;
> +		err = device_register(dev);
> +		if (!err)
> +			err = device_create_file(dev, &dev_attr_current_device);
> +		if (err)
> +			return err;
> +	}
> +	return tick_broadcast_init_sysfs();
> +}

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [patch 13/15] clockevents: Provide sysfs interface

[Thomas Gleixner]

On Fri, 26 Apr 2013, Stephen Boyd wrote:

> On 04/25, Thomas Gleixner wrote:
> > Provide a simple sysfs interface for the clockevent devices. Show the
> > current active clockevent device.
> > 
> 
> Neat. Does this do anything about clockevents that aren't in use
> for the tick devices or broadcast device?

No, but that would be simple to add.
 
> > +static int tick_broadcast_init_sysfs(void)
> 
> __init?

Yes
 
> > +{
> > +	int err = device_register(&tick_bc_dev);
> > +
> > +	if (!err)
> > +		err = device_create_file(&tick_bc_dev, &dev_attr_current_device);
> > +	return err;
> > +}
> > +#else
> > +static struct tick_device *tick_get_tick_dev(struct device *dev)
> > +{
> > +	return &per_cpu(tick_cpu_device, dev->id);
> > +}
> > +static inline int tick_broadcast_init_sysfs(void) { return 0; }
> >  #endif
> > +
> > +int __init tick_init_sysfs(void)
> 
> static?

Yes
 
> > +{
> > +	int cpu;
> > +
> > +	for_each_possible_cpu(cpu) {
> > +		struct device *dev = &per_cpu(tick_percpu_dev, cpu);
> > +		int err;
> > +
> > +		dev->id = cpu;
> > +		dev->bus = &clockevents_subsys;
> > +		err = device_register(dev);
> > +		if (!err)
> > +			err = device_create_file(dev, &dev_attr_current_device);
> > +		if (err)
> > +			return err;
> > +	}
> > +	return tick_broadcast_init_sysfs();
> > +}
> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by The Linux Foundation
> 

Re: [patch 13/15] clockevents: Provide sysfs interface

[Stephen Boyd]

On 05/15/13 02:50, Thomas Gleixner wrote:
> On Fri, 26 Apr 2013, Stephen Boyd wrote:
>
>> On 04/25, Thomas Gleixner wrote:
>>> Provide a simple sysfs interface for the clockevent devices. Show the
>>> current active clockevent device.
>>>
>> Neat. Does this do anything about clockevents that aren't in use
>> for the tick devices or broadcast device?
> No, but that would be simple to add.

What would it look like? Right now I think we have clockevent0,1,2,3,
etc. for all the cpus and a broadcast0 device. The broadcast0 device is
present even if we don't actually have a broadcast device in the system
(i.e. it says <null> for its name).

Would we add unused_clockevent0,1,2? Maybe we should have clockevent
devices for each physical evtdev and then symlinks for clockevent0,1,2,3
and broadcast0 that point to the physical clockevent device?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[tip:timers/core] clockevents: Provide sysfs interface

[tip-bot for Thomas Gleixner]

Commit-ID:  501f867064e95f9a6f540e60705be0937280e7ec
Gitweb:     http://git.kernel.org/tip/501f867064e95f9a6f540e60705be0937280e7ec
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 25 Apr 2013 20:31:49 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 16 May 2013 11:09:17 +0200

clockevents: Provide sysfs interface

Provide a simple sysfs interface for the clockevent devices. Show the
current active clockevent device.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Link: http://lkml.kernel.org/r/20130425143436.371634778@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/clockevents.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index 89e394c..0a23f4f 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -16,6 +16,7 @@
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/smp.h>
+#include <linux/device.h>
 
 #include "tick-internal.h"
 
@@ -460,4 +461,89 @@ void clockevents_notify(unsigned long reason, void *arg)
 	raw_spin_unlock_irqrestore(&clockevents_lock, flags);
 }
 EXPORT_SYMBOL_GPL(clockevents_notify);
+
+#ifdef CONFIG_SYSFS
+struct bus_type clockevents_subsys = {
+	.name		= "clockevents",
+	.dev_name       = "clockevent",
+};
+
+static DEFINE_PER_CPU(struct device, tick_percpu_dev);
+static struct tick_device *tick_get_tick_dev(struct device *dev);
+
+static ssize_t sysfs_show_current_tick_dev(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf)
+{
+	struct tick_device *td;
+	ssize_t count = 0;
+
+	raw_spin_lock_irq(&clockevents_lock);
+	td = tick_get_tick_dev(dev);
+	if (td && td->evtdev)
+		count = snprintf(buf, PAGE_SIZE, "%s\n", td->evtdev->name);
+	raw_spin_unlock_irq(&clockevents_lock);
+	return count;
+}
+static DEVICE_ATTR(current_device, 0444, sysfs_show_current_tick_dev, NULL);
+
+#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
+static struct device tick_bc_dev = {
+	.init_name	= "broadcast",
+	.id		= 0,
+	.bus		= &clockevents_subsys,
+};
+
+static struct tick_device *tick_get_tick_dev(struct device *dev)
+{
+	return dev == &tick_bc_dev ? tick_get_broadcast_device() :
+		&per_cpu(tick_cpu_device, dev->id);
+}
+
+static __init int tick_broadcast_init_sysfs(void)
+{
+	int err = device_register(&tick_bc_dev);
+
+	if (!err)
+		err = device_create_file(&tick_bc_dev, &dev_attr_current_device);
+	return err;
+}
+#else
+static struct tick_device *tick_get_tick_dev(struct device *dev)
+{
+	return &per_cpu(tick_cpu_device, dev->id);
+}
+static inline int tick_broadcast_init_sysfs(void) { return 0; }
 #endif
+
+static int __init tick_init_sysfs(void)
+{
+	int cpu;
+
+	for_each_possible_cpu(cpu) {
+		struct device *dev = &per_cpu(tick_percpu_dev, cpu);
+		int err;
+
+		dev->id = cpu;
+		dev->bus = &clockevents_subsys;
+		err = device_register(dev);
+		if (!err)
+			err = device_create_file(dev, &dev_attr_current_device);
+		if (err)
+			return err;
+	}
+	return tick_broadcast_init_sysfs();
+}
+
+static int __init clockevents_init_sysfs(void)
+{
+	int err = subsys_system_register(&clockevents_subsys, NULL);
+
+	if (!err)
+		err = tick_init_sysfs();
+	return err;
+}
+device_initcall(clockevents_init_sysfs);
+#endif /* SYSFS */
+
+#endif /* GENERIC_CLOCK_EVENTS */

[patch 12/15] clockevents: Add module refcount

[Thomas Gleixner]

[-- Attachment #1: clockevents-add-module-refcount.patch --]
[-- Type: text/plain, Size: 2937 bytes --]

We want to be able to remove clockevent modules as well. Add a
refcount so we don't remove a module with an active clock event
device.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/clockchips.h   |    3 +++
 kernel/time/clockevents.c    |    1 +
 kernel/time/tick-broadcast.c |    3 +++
 kernel/time/tick-common.c    |    4 ++++
 4 files changed, 11 insertions(+)

Index: tip/include/linux/clockchips.h
===================================================================
--- tip.orig/include/linux/clockchips.h
+++ tip/include/linux/clockchips.h
@@ -16,6 +16,7 @@
 #include <linux/notifier.h>
 
 struct clock_event_device;
+struct module;
 
 /* Clock event mode commands */
 enum clock_event_mode {
@@ -83,6 +84,7 @@ enum clock_event_nofitiers {
  * @irq:		IRQ number (only for non CPU local devices)
  * @cpumask:		cpumask to indicate for which CPUs this device works
  * @list:		list head for the management code
+ * @owner:		module reference
  */
 struct clock_event_device {
 	void			(*event_handler)(struct clock_event_device *);
@@ -112,6 +114,7 @@ struct clock_event_device {
 	int			irq;
 	const struct cpumask	*cpumask;
 	struct list_head	list;
+	struct module		*owner;
 } ____cacheline_aligned;
 
 /*
Index: tip/kernel/time/clockevents.c
===================================================================
--- tip.orig/kernel/time/clockevents.c
+++ tip/kernel/time/clockevents.c
@@ -357,6 +357,7 @@ void clockevents_exchange_device(struct 
 	 * released list and do a notify add later.
 	 */
 	if (old) {
+		module_put(old->owner);
 		clockevents_set_mode(old, CLOCK_EVT_MODE_UNUSED);
 		list_del(&old->list);
 		list_add(&old->list, &clockevents_released);
Index: tip/kernel/time/tick-broadcast.c
===================================================================
--- tip.orig/kernel/time/tick-broadcast.c
+++ tip/kernel/time/tick-broadcast.c
@@ -19,6 +19,7 @@
 #include <linux/profile.h>
 #include <linux/sched.h>
 #include <linux/smp.h>
+#include <linux/module.h>
 
 #include "tick-internal.h"
 
@@ -73,6 +74,8 @@ void tick_install_broadcast_device(struc
 	     tick_broadcast_device.evtdev->rating >= dev->rating) ||
 	     (dev->features & CLOCK_EVT_FEAT_C3STOP))
 		return;
+	if (!try_module_get(dev->owner))
+		return;
 
 	clockevents_exchange_device(tick_broadcast_device.evtdev, dev);
 	if (cur)
Index: tip/kernel/time/tick-common.c
===================================================================
--- tip.orig/kernel/time/tick-common.c
+++ tip/kernel/time/tick-common.c
@@ -18,6 +18,7 @@
 #include <linux/percpu.h>
 #include <linux/profile.h>
 #include <linux/sched.h>
+#include <linux/module.h>
 
 #include <asm/irq_regs.h>
 
@@ -254,6 +255,9 @@ void tick_check_new_device(struct clock_
 			goto out_bc;
 	}
 
+	if (!try_module_get(newdev->owner))
+		return;
+
 	/*
 	 * Replace the eventually existing device by the new
 	 * device. If the current device is the broadcast device, do

[tip:timers/core] clockevents: Add module refcount

[tip-bot for Thomas Gleixner]

Commit-ID:  ccf33d6880f39a35158fff66db13000ae4943fac
Gitweb:     http://git.kernel.org/tip/ccf33d6880f39a35158fff66db13000ae4943fac
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 25 Apr 2013 20:31:49 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 16 May 2013 11:09:17 +0200

clockevents: Add module refcount

We want to be able to remove clockevent modules as well. Add a
refcount so we don't remove a module with an active clock event
device.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Link: http://lkml.kernel.org/r/20130425143436.307435149@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/clockchips.h   | 3 +++
 kernel/time/clockevents.c    | 1 +
 kernel/time/tick-broadcast.c | 3 +++
 kernel/time/tick-common.c    | 4 ++++
 4 files changed, 11 insertions(+)

diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h
index 2f498f6..ae1193b 100644
--- a/include/linux/clockchips.h
+++ b/include/linux/clockchips.h
@@ -30,6 +30,7 @@ enum clock_event_nofitiers {
 #include <linux/notifier.h>
 
 struct clock_event_device;
+struct module;
 
 /* Clock event mode commands */
 enum clock_event_mode {
@@ -83,6 +84,7 @@ enum clock_event_mode {
  * @irq:		IRQ number (only for non CPU local devices)
  * @cpumask:		cpumask to indicate for which CPUs this device works
  * @list:		list head for the management code
+ * @owner:		module reference
  */
 struct clock_event_device {
 	void			(*event_handler)(struct clock_event_device *);
@@ -112,6 +114,7 @@ struct clock_event_device {
 	int			irq;
 	const struct cpumask	*cpumask;
 	struct list_head	list;
+	struct module		*owner;
 } ____cacheline_aligned;
 
 /*
diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index 0e3a844..89e394c 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -357,6 +357,7 @@ void clockevents_exchange_device(struct clock_event_device *old,
 	 * released list and do a notify add later.
 	 */
 	if (old) {
+		module_put(old->owner);
 		clockevents_set_mode(old, CLOCK_EVT_MODE_UNUSED);
 		list_del(&old->list);
 		list_add(&old->list, &clockevents_released);
diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index 3500caa..0e374cd 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -19,6 +19,7 @@
 #include <linux/profile.h>
 #include <linux/sched.h>
 #include <linux/smp.h>
+#include <linux/module.h>
 
 #include "tick-internal.h"
 
@@ -73,6 +74,8 @@ void tick_install_broadcast_device(struct clock_event_device *dev)
 	     tick_broadcast_device.evtdev->rating >= dev->rating) ||
 	     (dev->features & CLOCK_EVT_FEAT_C3STOP))
 		return;
+	if (!try_module_get(dev->owner))
+		return;
 
 	clockevents_exchange_device(tick_broadcast_device.evtdev, dev);
 	if (cur)
diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index 84c7cfc..433a1e1 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -18,6 +18,7 @@
 #include <linux/percpu.h>
 #include <linux/profile.h>
 #include <linux/sched.h>
+#include <linux/module.h>
 
 #include <asm/irq_regs.h>
 
@@ -257,6 +258,9 @@ void tick_check_new_device(struct clock_event_device *newdev)
 			goto out_bc;
 	}
 
+	if (!try_module_get(newdev->owner))
+		return;
+
 	/*
 	 * Replace the eventually existing device by the new
 	 * device. If the current device is the broadcast device, do

[patch 15/15] clockevents: Implement unbind functionality

[Thomas Gleixner]

[-- Attachment #1: clockevents-implement-unbind.patch --]
[-- Type: text/plain, Size: 9335 bytes --]

Provide a sysfs interface to allow unbinding of clockevent
devices. The device is unbound if it is unused or if there is a
replacement device available. Unbinding of broadcast devices is not
supported as we don't want to foster that nonsense. If no replacement
device is available the unbind returns -EBUSY. Unbind is available
from the kernel and through sysfs, which is necessary to drop the
module refcount.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/clockchips.h  |    1 +
 kernel/time/clockevents.c   |  125 ++++++++++++++++++++++++++++++++++++++++++++
 kernel/time/clocksource.c   |    9 +--
 kernel/time/tick-common.c   |   24 ++++++++
 kernel/time/tick-internal.h |    7 ++
 5 files changed, 162 insertions(+), 4 deletions(-)

Index: tip/include/linux/clockchips.h
===================================================================
--- tip.orig/include/linux/clockchips.h
+++ tip/include/linux/clockchips.h
@@ -141,6 +141,7 @@ static inline unsigned long div_sc(unsig
 extern u64 clockevent_delta2ns(unsigned long latch,
 			       struct clock_event_device *evt);
 extern void clockevents_register_device(struct clock_event_device *dev);
+extern int clockevents_unbind_device(struct clock_event_device *ced, int cpu);
 
 extern void clockevents_config(struct clock_event_device *dev, u32 freq);
 extern void clockevents_config_and_register(struct clock_event_device *dev,
Index: tip/kernel/time/clockevents.c
===================================================================
--- tip.orig/kernel/time/clockevents.c
+++ tip/kernel/time/clockevents.c
@@ -25,6 +25,13 @@ static LIST_HEAD(clockevent_devices);
 static LIST_HEAD(clockevents_released);
 /* Protection for the above */
 static DEFINE_RAW_SPINLOCK(clockevents_lock);
+/* Protection for unbind operations */
+static DEFINE_MUTEX(clockevents_mutex);
+
+struct ce_unbind {
+	struct clock_event_device *ce;
+	int res;
+};
 
 /**
  * clockevents_delta2ns - Convert a latch value (device ticks) to nanoseconds
@@ -245,6 +252,90 @@ static void clockevents_notify_released(
 	}
 }
 
+/*
+ * Try to install a replacement clock event device
+ */
+static int clockevents_replace(struct clock_event_device *ced)
+{
+	struct clock_event_device *dev, *newdev = NULL;
+
+	list_for_each_entry(dev, &clockevent_devices, list) {
+		if (dev == ced || dev->mode != CLOCK_EVT_MODE_UNUSED)
+			continue;
+
+		if (!tick_check_replacement(newdev, dev))
+			continue;
+
+		if (!try_module_get(dev->owner))
+			continue;
+
+		if (newdev)
+			module_put(newdev->owner);
+		newdev = dev;
+	}
+	if (newdev) {
+		tick_install_replacement(newdev);
+		list_del_init(&ced->list);
+	}
+	return newdev ? 0 : -EBUSY;
+}
+
+/*
+ * Called with clockevents_mutex and clockevents_lock held
+ */
+static int __clockevents_try_unbind(struct clock_event_device *ced, int cpu)
+{
+	/* Fast track. Device is unused */
+	if (ced->mode == CLOCK_EVT_MODE_UNUSED) {
+		list_del_init(&ced->list);
+		return 0;
+	}
+
+	return ced == per_cpu(tick_cpu_device, cpu).evtdev ? -EAGAIN : -EBUSY;
+}
+
+/*
+ * SMP function call to unbind a device
+ */
+static void __clockevents_unbind(void *arg)
+{
+	struct ce_unbind *cu = arg;
+	int res;
+
+	raw_spin_lock(&clockevents_lock);
+	res = __clockevents_try_unbind(cu->ce, smp_processor_id());
+	if (res == -EAGAIN)
+		res = clockevents_replace(cu->ce);
+	cu->res = res;
+	raw_spin_unlock(&clockevents_lock);
+}
+
+/*
+ * Issues smp function call to unbind a per cpu device. Called with
+ * clockevents_mutex held.
+ */
+static int clockevents_unbind(struct clock_event_device *ced, int cpu)
+{
+	struct ce_unbind cu = { .ce = ced, .res = -ENODEV };
+
+	smp_call_function_single(cpu, __clockevents_unbind, &cu, 1);
+	return cu.res;
+}
+
+/*
+ * Unbind a clockevents device.
+ */
+int clockevents_unbind_device(struct clock_event_device *ced, int cpu)
+{
+	int ret;
+
+	mutex_lock(&clockevents_mutex);
+	ret = clockevents_unbind(ced, cpu);
+	mutex_unlock(&clockevents_mutex);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(clockevents_unbind);
+
 /**
  * clockevents_register_device - register a clock event device
  * @dev:	device to register
@@ -487,6 +578,38 @@ static ssize_t sysfs_show_current_tick_d
 }
 static DEVICE_ATTR(current_device, 0444, sysfs_show_current_tick_dev, NULL);
 
+/* We don't support the abomination of removable broadcast devices */
+static ssize_t sysfs_unbind_tick_dev(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	char name[CS_NAME_LEN];
+	size_t ret = sysfs_get_uname(buf, name, count);
+	struct clock_event_device *ce;
+
+	if (ret < 0)
+		return ret;
+
+	ret = -ENODEV;
+	mutex_lock(&clockevents_mutex);
+	raw_spin_lock_irq(&clockevents_lock);
+	list_for_each_entry(ce, &clockevent_devices, list) {
+		if (!strcmp(ce->name, name)) {
+			ret = __clockevents_try_unbind(ce, dev->id);
+			break;
+		}
+	}
+	raw_spin_unlock_irq(&clockevents_lock);
+	/*
+	 * We hold clockevents_mutex, so ce can't go away
+	 */
+	if (ret == -EAGAIN)
+		ret = clockevents_unbind(ce, dev->id);
+	mutex_unlock(&clockevents_mutex);
+	return ret ? ret : count;
+}
+static DEVICE_ATTR(unbind_device, 0200, NULL, sysfs_unbind_tick_dev);
+
 #ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 static struct device tick_bc_dev = {
 	.init_name	= "broadcast",
@@ -529,6 +652,8 @@ int __init tick_init_sysfs(void)
 		err = device_register(dev);
 		if (!err)
 			err = device_create_file(dev, &dev_attr_current_device);
+		if (!err)
+			err = device_create_file(dev, &dev_attr_unbind_device);
 		if (err)
 			return err;
 	}
Index: tip/kernel/time/clocksource.c
===================================================================
--- tip.orig/kernel/time/clocksource.c
+++ tip/kernel/time/clocksource.c
@@ -31,6 +31,8 @@
 #include <linux/tick.h>
 #include <linux/kthread.h>
 
+#include "tick-internal.h"
+
 void timecounter_init(struct timecounter *tc,
 		      const struct cyclecounter *cc,
 		      u64 start_tstamp)
@@ -174,7 +176,6 @@ clocks_calc_mult_shift(u32 *mult, u32 *s
 static struct clocksource *curr_clocksource;
 static LIST_HEAD(clocksource_list);
 static DEFINE_MUTEX(clocksource_mutex);
-#define CS_NAME_LEN		32
 static char override_name[CS_NAME_LEN];
 static int finished_booting;
 
@@ -854,7 +855,7 @@ sysfs_show_current_clocksources(struct d
 	return count;
 }
 
-static size_t clocksource_get_uname(const char *buf, char *dst, size_t cnt)
+size_t sysfs_get_uname(const char *buf, char *dst, size_t cnt)
 {
 	/* strings from sysfs write are not 0 terminated! */
 	if (!cnt || cnt >= CS_NAME_LEN)
@@ -887,7 +888,7 @@ static ssize_t sysfs_override_clocksourc
 
 	mutex_lock(&clocksource_mutex);
 
-	ret = clocksource_get_uname(buf, override_name, count);
+	ret = sysfs_get_uname(buf, override_name, count);
 	if (ret >= 0)
 		clocksource_select(false);
 
@@ -913,7 +914,7 @@ static ssize_t sysfs_unbind_clocksource(
 	char name[CS_NAME_LEN];
 	size_t ret;
 
-	ret = clocksource_get_uname(buf, name, count);
+	ret = sysfs_get_uname(buf, name, count);
 	if (ret < 0)
 		return ret;
 
Index: tip/kernel/time/tick-common.c
===================================================================
--- tip.orig/kernel/time/tick-common.c
+++ tip/kernel/time/tick-common.c
@@ -202,6 +202,17 @@ static void tick_setup_device(struct tic
 		tick_setup_oneshot(newdev, handler, next_event);
 }
 
+void tick_install_replacement(struct clock_event_device *newdev)
+{
+	struct tick_device *td = &__get_cpu_var(tick_cpu_device);
+	int cpu = smp_processor_id();
+
+	clockevents_exchange_device(td->evtdev, newdev);
+	tick_setup_device(td, newdev, cpu, cpumask_of(cpu));
+	if (newdev->features & CLOCK_EVT_FEAT_ONESHOT)
+		tick_oneshot_notify();
+}
+
 static bool tick_check_percpu(struct clock_event_device *curdev,
 			      struct clock_event_device *newdev, int cpu)
 {
@@ -234,6 +245,19 @@ static bool tick_check_preferred(struct 
 }
 
 /*
+ * Check whether the new device is a better fit than curdev. curdev
+ * can be NULL !
+ */
+bool tick_check_replacement(struct clock_event_device *curdev,
+			    struct clock_event_device *newdev)
+{
+	if (tick_check_percpu(curdev, newdev, smp_processor_id()))
+		return false;
+
+	return tick_check_preferred(curdev, newdev);
+}
+
+/*
  * Check, if the new registered device should be used. Called with
  * clockevents_lock held and interrupts disabled.
  */
Index: tip/kernel/time/tick-internal.h
===================================================================
--- tip.orig/kernel/time/tick-internal.h
+++ tip/kernel/time/tick-internal.h
@@ -11,6 +11,8 @@ extern seqlock_t jiffies_lock;
 #define TICK_DO_TIMER_NONE	-1
 #define TICK_DO_TIMER_BOOT	-2
 
+#define CS_NAME_LEN	32
+
 DECLARE_PER_CPU(struct tick_device, tick_cpu_device);
 extern ktime_t tick_next_period;
 extern ktime_t tick_period;
@@ -23,9 +25,14 @@ extern void tick_handover_do_timer(int *
 extern void tick_shutdown(unsigned int *cpup);
 extern void tick_suspend(void);
 extern void tick_resume(void);
+extern bool tick_check_replacement(struct clock_event_device *curdev,
+				   struct clock_event_device *newdev);
+extern void tick_install_replacement(struct clock_event_device *dev);
 
 extern void clockevents_shutdown(struct clock_event_device *dev);
 
+extern size_t sysfs_get_uname(const char *buf, char *dst, size_t cnt);
+
 /*
  * NO_HZ / high resolution timer shared code
  */

[tip:timers/core] clockevents: Implement unbind functionality

[tip-bot for Thomas Gleixner]

Commit-ID:  03e13cf5ee60584fe0c831682c67212effb7fca4
Gitweb:     http://git.kernel.org/tip/03e13cf5ee60584fe0c831682c67212effb7fca4
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 25 Apr 2013 20:31:50 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 16 May 2013 11:09:18 +0200

clockevents: Implement unbind functionality

Provide a sysfs interface to allow unbinding of clockevent
devices. The device is unbound if it is unused or if there is a
replacement device available. Unbinding of broadcast devices is not
supported as we don't want to foster that nonsense. If no replacement
device is available the unbind returns -EBUSY. Unbind is available
from the kernel and through sysfs, which is necessary to drop the
module refcount.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Link: http://lkml.kernel.org/r/20130425143436.499216659@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/clockchips.h  |   1 +
 kernel/time/clockevents.c   | 125 ++++++++++++++++++++++++++++++++++++++++++++
 kernel/time/clocksource.c   |   9 ++--
 kernel/time/tick-common.c   |  24 +++++++++
 kernel/time/tick-internal.h |   7 +++
 5 files changed, 162 insertions(+), 4 deletions(-)

diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h
index ae1193b..0857922 100644
--- a/include/linux/clockchips.h
+++ b/include/linux/clockchips.h
@@ -141,6 +141,7 @@ static inline unsigned long div_sc(unsigned long ticks, unsigned long nsec,
 extern u64 clockevent_delta2ns(unsigned long latch,
 			       struct clock_event_device *evt);
 extern void clockevents_register_device(struct clock_event_device *dev);
+extern int clockevents_unbind_device(struct clock_event_device *ced, int cpu);
 
 extern void clockevents_config(struct clock_event_device *dev, u32 freq);
 extern void clockevents_config_and_register(struct clock_event_device *dev,
diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index 0a23f4f..38959c8 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -25,6 +25,13 @@ static LIST_HEAD(clockevent_devices);
 static LIST_HEAD(clockevents_released);
 /* Protection for the above */
 static DEFINE_RAW_SPINLOCK(clockevents_lock);
+/* Protection for unbind operations */
+static DEFINE_MUTEX(clockevents_mutex);
+
+struct ce_unbind {
+	struct clock_event_device *ce;
+	int res;
+};
 
 /**
  * clockevents_delta2ns - Convert a latch value (device ticks) to nanoseconds
@@ -245,6 +252,90 @@ static void clockevents_notify_released(void)
 	}
 }
 
+/*
+ * Try to install a replacement clock event device
+ */
+static int clockevents_replace(struct clock_event_device *ced)
+{
+	struct clock_event_device *dev, *newdev = NULL;
+
+	list_for_each_entry(dev, &clockevent_devices, list) {
+		if (dev == ced || dev->mode != CLOCK_EVT_MODE_UNUSED)
+			continue;
+
+		if (!tick_check_replacement(newdev, dev))
+			continue;
+
+		if (!try_module_get(dev->owner))
+			continue;
+
+		if (newdev)
+			module_put(newdev->owner);
+		newdev = dev;
+	}
+	if (newdev) {
+		tick_install_replacement(newdev);
+		list_del_init(&ced->list);
+	}
+	return newdev ? 0 : -EBUSY;
+}
+
+/*
+ * Called with clockevents_mutex and clockevents_lock held
+ */
+static int __clockevents_try_unbind(struct clock_event_device *ced, int cpu)
+{
+	/* Fast track. Device is unused */
+	if (ced->mode == CLOCK_EVT_MODE_UNUSED) {
+		list_del_init(&ced->list);
+		return 0;
+	}
+
+	return ced == per_cpu(tick_cpu_device, cpu).evtdev ? -EAGAIN : -EBUSY;
+}
+
+/*
+ * SMP function call to unbind a device
+ */
+static void __clockevents_unbind(void *arg)
+{
+	struct ce_unbind *cu = arg;
+	int res;
+
+	raw_spin_lock(&clockevents_lock);
+	res = __clockevents_try_unbind(cu->ce, smp_processor_id());
+	if (res == -EAGAIN)
+		res = clockevents_replace(cu->ce);
+	cu->res = res;
+	raw_spin_unlock(&clockevents_lock);
+}
+
+/*
+ * Issues smp function call to unbind a per cpu device. Called with
+ * clockevents_mutex held.
+ */
+static int clockevents_unbind(struct clock_event_device *ced, int cpu)
+{
+	struct ce_unbind cu = { .ce = ced, .res = -ENODEV };
+
+	smp_call_function_single(cpu, __clockevents_unbind, &cu, 1);
+	return cu.res;
+}
+
+/*
+ * Unbind a clockevents device.
+ */
+int clockevents_unbind_device(struct clock_event_device *ced, int cpu)
+{
+	int ret;
+
+	mutex_lock(&clockevents_mutex);
+	ret = clockevents_unbind(ced, cpu);
+	mutex_unlock(&clockevents_mutex);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(clockevents_unbind);
+
 /**
  * clockevents_register_device - register a clock event device
  * @dev:	device to register
@@ -487,6 +578,38 @@ static ssize_t sysfs_show_current_tick_dev(struct device *dev,
 }
 static DEVICE_ATTR(current_device, 0444, sysfs_show_current_tick_dev, NULL);
 
+/* We don't support the abomination of removable broadcast devices */
+static ssize_t sysfs_unbind_tick_dev(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	char name[CS_NAME_LEN];
+	size_t ret = sysfs_get_uname(buf, name, count);
+	struct clock_event_device *ce;
+
+	if (ret < 0)
+		return ret;
+
+	ret = -ENODEV;
+	mutex_lock(&clockevents_mutex);
+	raw_spin_lock_irq(&clockevents_lock);
+	list_for_each_entry(ce, &clockevent_devices, list) {
+		if (!strcmp(ce->name, name)) {
+			ret = __clockevents_try_unbind(ce, dev->id);
+			break;
+		}
+	}
+	raw_spin_unlock_irq(&clockevents_lock);
+	/*
+	 * We hold clockevents_mutex, so ce can't go away
+	 */
+	if (ret == -EAGAIN)
+		ret = clockevents_unbind(ce, dev->id);
+	mutex_unlock(&clockevents_mutex);
+	return ret ? ret : count;
+}
+static DEVICE_ATTR(unbind_device, 0200, NULL, sysfs_unbind_tick_dev);
+
 #ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 static struct device tick_bc_dev = {
 	.init_name	= "broadcast",
@@ -529,6 +652,8 @@ static int __init tick_init_sysfs(void)
 		err = device_register(dev);
 		if (!err)
 			err = device_create_file(dev, &dev_attr_current_device);
+		if (!err)
+			err = device_create_file(dev, &dev_attr_unbind_device);
 		if (err)
 			return err;
 	}
diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index 31b9033..6d05b00 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -31,6 +31,8 @@
 #include <linux/tick.h>
 #include <linux/kthread.h>
 
+#include "tick-internal.h"
+
 void timecounter_init(struct timecounter *tc,
 		      const struct cyclecounter *cc,
 		      u64 start_tstamp)
@@ -174,7 +176,6 @@ clocks_calc_mult_shift(u32 *mult, u32 *shift, u32 from, u32 to, u32 maxsec)
 static struct clocksource *curr_clocksource;
 static LIST_HEAD(clocksource_list);
 static DEFINE_MUTEX(clocksource_mutex);
-#define CS_NAME_LEN		32
 static char override_name[CS_NAME_LEN];
 static int finished_booting;
 
@@ -864,7 +865,7 @@ sysfs_show_current_clocksources(struct device *dev,
 	return count;
 }
 
-static size_t clocksource_get_uname(const char *buf, char *dst, size_t cnt)
+size_t sysfs_get_uname(const char *buf, char *dst, size_t cnt)
 {
 	size_t ret = cnt;
 
@@ -899,7 +900,7 @@ static ssize_t sysfs_override_clocksource(struct device *dev,
 
 	mutex_lock(&clocksource_mutex);
 
-	ret = clocksource_get_uname(buf, override_name, count);
+	ret = sysfs_get_uname(buf, override_name, count);
 	if (ret >= 0)
 		clocksource_select();
 
@@ -925,7 +926,7 @@ static ssize_t sysfs_unbind_clocksource(struct device *dev,
 	char name[CS_NAME_LEN];
 	size_t ret;
 
-	ret = clocksource_get_uname(buf, name, count);
+	ret = sysfs_get_uname(buf, name, count);
 	if (ret < 0)
 		return ret;
 
diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index c34021650..5edfb48 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -205,6 +205,17 @@ static void tick_setup_device(struct tick_device *td,
 		tick_setup_oneshot(newdev, handler, next_event);
 }
 
+void tick_install_replacement(struct clock_event_device *newdev)
+{
+	struct tick_device *td = &__get_cpu_var(tick_cpu_device);
+	int cpu = smp_processor_id();
+
+	clockevents_exchange_device(td->evtdev, newdev);
+	tick_setup_device(td, newdev, cpu, cpumask_of(cpu));
+	if (newdev->features & CLOCK_EVT_FEAT_ONESHOT)
+		tick_oneshot_notify();
+}
+
 static bool tick_check_percpu(struct clock_event_device *curdev,
 			      struct clock_event_device *newdev, int cpu)
 {
@@ -237,6 +248,19 @@ static bool tick_check_preferred(struct clock_event_device *curdev,
 }
 
 /*
+ * Check whether the new device is a better fit than curdev. curdev
+ * can be NULL !
+ */
+bool tick_check_replacement(struct clock_event_device *curdev,
+			    struct clock_event_device *newdev)
+{
+	if (tick_check_percpu(curdev, newdev, smp_processor_id()))
+		return false;
+
+	return tick_check_preferred(curdev, newdev);
+}
+
+/*
  * Check, if the new registered device should be used. Called with
  * clockevents_lock held and interrupts disabled.
  */
diff --git a/kernel/time/tick-internal.h b/kernel/time/tick-internal.h
index 06bfc88..be1690e 100644
--- a/kernel/time/tick-internal.h
+++ b/kernel/time/tick-internal.h
@@ -11,6 +11,8 @@ extern seqlock_t jiffies_lock;
 #define TICK_DO_TIMER_NONE	-1
 #define TICK_DO_TIMER_BOOT	-2
 
+#define CS_NAME_LEN	32
+
 DECLARE_PER_CPU(struct tick_device, tick_cpu_device);
 extern ktime_t tick_next_period;
 extern ktime_t tick_period;
@@ -23,9 +25,14 @@ extern void tick_handover_do_timer(int *cpup);
 extern void tick_shutdown(unsigned int *cpup);
 extern void tick_suspend(void);
 extern void tick_resume(void);
+extern bool tick_check_replacement(struct clock_event_device *curdev,
+				   struct clock_event_device *newdev);
+extern void tick_install_replacement(struct clock_event_device *dev);
 
 extern void clockevents_shutdown(struct clock_event_device *dev);
 
+extern size_t sysfs_get_uname(const char *buf, char *dst, size_t cnt);
+
 /*
  * NO_HZ / high resolution timer shared code
  */

[patch 14/15] clockevents: Split out selection logic

[Thomas Gleixner]

[-- Attachment #1: clockevents-split-out-selection-logic.patch --]
[-- Type: text/plain, Size: 4315 bytes --]

Split out the clockevent device selection logic. Preparatory patch to
allow unbinding active clockevent devices.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/tick-broadcast.c |   25 ++++++++++++---
 kernel/time/tick-common.c    |   69 ++++++++++++++++++++++---------------------
 2 files changed, 56 insertions(+), 38 deletions(-)

Index: tip/kernel/time/tick-broadcast.c
===================================================================
--- tip.orig/kernel/time/tick-broadcast.c
+++ tip/kernel/time/tick-broadcast.c
@@ -65,19 +65,34 @@ static void tick_broadcast_start_periodi
 /*
  * Check, if the device can be utilized as broadcast device:
  */
+static bool tick_check_broadcast_device(struct clock_event_device *curdev,
+					struct clock_event_device *newdev)
+{
+	if ((newdev->features & CLOCK_EVT_FEAT_DUMMY) ||
+	    (newdev->features & CLOCK_EVT_FEAT_C3STOP))
+		return false;
+
+	if (tick_broadcast_device.mode == TICKDEV_MODE_ONESHOT &&
+	    !(newdev->features & CLOCK_EVT_FEAT_ONESHOT))
+		return false;
+
+	return !curdev || newdev->rating > curdev->rating;
+}
+
+/*
+ * Conditionally install/replace broadcast device
+ */
 void tick_install_broadcast_device(struct clock_event_device *dev)
 {
 	struct clock_event_device *cur = tick_broadcast_device.evtdev;
 
-	if ((dev->features & CLOCK_EVT_FEAT_DUMMY) ||
-	    (tick_broadcast_device.evtdev &&
-	     tick_broadcast_device.evtdev->rating >= dev->rating) ||
-	     (dev->features & CLOCK_EVT_FEAT_C3STOP))
+	if (!tick_check_broadcast_device(cur, dev))
 		return;
+
 	if (!try_module_get(dev->owner))
 		return;
 
-	clockevents_exchange_device(tick_broadcast_device.evtdev, dev);
+	clockevents_exchange_device(cur, dev);
 	if (cur)
 		cur->event_handler = clockevents_handle_noop;
 	tick_broadcast_device.evtdev = dev;
Index: tip/kernel/time/tick-common.c
===================================================================
--- tip.orig/kernel/time/tick-common.c
+++ tip/kernel/time/tick-common.c
@@ -202,6 +202,37 @@ static void tick_setup_device(struct tic
 		tick_setup_oneshot(newdev, handler, next_event);
 }
 
+static bool tick_check_percpu(struct clock_event_device *curdev,
+			      struct clock_event_device *newdev, int cpu)
+{
+	if (!cpumask_test_cpu(cpu, newdev->cpumask))
+		return false;
+	if (cpumask_equal(newdev->cpumask, cpumask_of(cpu)))
+		return true;
+	/* Check if irq affinity can be set */
+	if (newdev->irq >= 0 && !irq_can_set_affinity(newdev->irq))
+		return false;
+	/* Prefer an existing cpu local device */
+	if (curdev && cpumask_equal(curdev->cpumask, cpumask_of(cpu)))
+		return false;
+	return true;
+}
+
+static bool tick_check_preferred(struct clock_event_device *curdev,
+				 struct clock_event_device *newdev)
+{
+	/* Prefer oneshot capable device */
+	if (!(newdev->features & CLOCK_EVT_FEAT_ONESHOT)) {
+		if (curdev && (curdev->features & CLOCK_EVT_FEAT_ONESHOT))
+			return false;
+		if (tick_oneshot_mode_active())
+			return false;
+	}
+
+	/* Use the higher rated one */
+	return !curdev || newdev->rating > curdev->rating;
+}
+
 /*
  * Check, if the new registered device should be used. Called with
  * clockevents_lock held and interrupts disabled.
@@ -220,40 +251,12 @@ void tick_check_new_device(struct clock_
 	curdev = td->evtdev;
 
 	/* cpu local device ? */
-	if (!cpumask_equal(newdev->cpumask, cpumask_of(cpu))) {
-
-		/*
-		 * If the cpu affinity of the device interrupt can not
-		 * be set, ignore it.
-		 */
-		if (!irq_can_set_affinity(newdev->irq))
-			goto out_bc;
-
-		/*
-		 * If we have a cpu local device already, do not replace it
-		 * by a non cpu local device
-		 */
-		if (curdev && cpumask_equal(curdev->cpumask, cpumask_of(cpu)))
-			goto out_bc;
-	}
+	if (!tick_check_percpu(curdev, newdev, cpu))
+		goto out_bc;
 
-	/*
-	 * If we have an active device, then check the rating and the oneshot
-	 * feature.
-	 */
-	if (curdev) {
-		/*
-		 * Prefer one shot capable devices !
-		 */
-		if ((curdev->features & CLOCK_EVT_FEAT_ONESHOT) &&
-		    !(newdev->features & CLOCK_EVT_FEAT_ONESHOT))
-			goto out_bc;
-		/*
-		 * Check the rating
-		 */
-		if (curdev->rating >= newdev->rating)
-			goto out_bc;
-	}
+	/* Preference decision */
+	if (!tick_check_preferred(curdev, newdev))
+		goto out_bc;
 
 	if (!try_module_get(newdev->owner))
 		return;

[tip:timers/core] clockevents: Split out selection logic

[tip-bot for Thomas Gleixner]

Commit-ID:  45cb8e01b2ecef1c2afb18333e95793fa1a90281
Gitweb:     http://git.kernel.org/tip/45cb8e01b2ecef1c2afb18333e95793fa1a90281
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 25 Apr 2013 20:31:50 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 16 May 2013 11:09:17 +0200

clockevents: Split out selection logic

Split out the clockevent device selection logic. Preparatory patch to
allow unbinding active clockevent devices.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Link: http://lkml.kernel.org/r/20130425143436.431796247@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/tick-broadcast.c | 25 ++++++++++++----
 kernel/time/tick-common.c    | 69 +++++++++++++++++++++++---------------------
 2 files changed, 56 insertions(+), 38 deletions(-)

diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index 0e374cd..d067c01 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -65,19 +65,34 @@ static void tick_broadcast_start_periodic(struct clock_event_device *bc)
 /*
  * Check, if the device can be utilized as broadcast device:
  */
+static bool tick_check_broadcast_device(struct clock_event_device *curdev,
+					struct clock_event_device *newdev)
+{
+	if ((newdev->features & CLOCK_EVT_FEAT_DUMMY) ||
+	    (newdev->features & CLOCK_EVT_FEAT_C3STOP))
+		return false;
+
+	if (tick_broadcast_device.mode == TICKDEV_MODE_ONESHOT &&
+	    !(newdev->features & CLOCK_EVT_FEAT_ONESHOT))
+		return false;
+
+	return !curdev || newdev->rating > curdev->rating;
+}
+
+/*
+ * Conditionally install/replace broadcast device
+ */
 void tick_install_broadcast_device(struct clock_event_device *dev)
 {
 	struct clock_event_device *cur = tick_broadcast_device.evtdev;
 
-	if ((dev->features & CLOCK_EVT_FEAT_DUMMY) ||
-	    (tick_broadcast_device.evtdev &&
-	     tick_broadcast_device.evtdev->rating >= dev->rating) ||
-	     (dev->features & CLOCK_EVT_FEAT_C3STOP))
+	if (!tick_check_broadcast_device(cur, dev))
 		return;
+
 	if (!try_module_get(dev->owner))
 		return;
 
-	clockevents_exchange_device(tick_broadcast_device.evtdev, dev);
+	clockevents_exchange_device(cur, dev);
 	if (cur)
 		cur->event_handler = clockevents_handle_noop;
 	tick_broadcast_device.evtdev = dev;
diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
index 433a1e1..c34021650 100644
--- a/kernel/time/tick-common.c
+++ b/kernel/time/tick-common.c
@@ -205,6 +205,37 @@ static void tick_setup_device(struct tick_device *td,
 		tick_setup_oneshot(newdev, handler, next_event);
 }
 
+static bool tick_check_percpu(struct clock_event_device *curdev,
+			      struct clock_event_device *newdev, int cpu)
+{
+	if (!cpumask_test_cpu(cpu, newdev->cpumask))
+		return false;
+	if (cpumask_equal(newdev->cpumask, cpumask_of(cpu)))
+		return true;
+	/* Check if irq affinity can be set */
+	if (newdev->irq >= 0 && !irq_can_set_affinity(newdev->irq))
+		return false;
+	/* Prefer an existing cpu local device */
+	if (curdev && cpumask_equal(curdev->cpumask, cpumask_of(cpu)))
+		return false;
+	return true;
+}
+
+static bool tick_check_preferred(struct clock_event_device *curdev,
+				 struct clock_event_device *newdev)
+{
+	/* Prefer oneshot capable device */
+	if (!(newdev->features & CLOCK_EVT_FEAT_ONESHOT)) {
+		if (curdev && (curdev->features & CLOCK_EVT_FEAT_ONESHOT))
+			return false;
+		if (tick_oneshot_mode_active())
+			return false;
+	}
+
+	/* Use the higher rated one */
+	return !curdev || newdev->rating > curdev->rating;
+}
+
 /*
  * Check, if the new registered device should be used. Called with
  * clockevents_lock held and interrupts disabled.
@@ -223,40 +254,12 @@ void tick_check_new_device(struct clock_event_device *newdev)
 	curdev = td->evtdev;
 
 	/* cpu local device ? */
-	if (!cpumask_equal(newdev->cpumask, cpumask_of(cpu))) {
-
-		/*
-		 * If the cpu affinity of the device interrupt can not
-		 * be set, ignore it.
-		 */
-		if (!irq_can_set_affinity(newdev->irq))
-			goto out_bc;
-
-		/*
-		 * If we have a cpu local device already, do not replace it
-		 * by a non cpu local device
-		 */
-		if (curdev && cpumask_equal(curdev->cpumask, cpumask_of(cpu)))
-			goto out_bc;
-	}
+	if (!tick_check_percpu(curdev, newdev, cpu))
+		goto out_bc;
 
-	/*
-	 * If we have an active device, then check the rating and the oneshot
-	 * feature.
-	 */
-	if (curdev) {
-		/*
-		 * Prefer one shot capable devices !
-		 */
-		if ((curdev->features & CLOCK_EVT_FEAT_ONESHOT) &&
-		    !(newdev->features & CLOCK_EVT_FEAT_ONESHOT))
-			goto out_bc;
-		/*
-		 * Check the rating
-		 */
-		if (curdev->rating >= newdev->rating)
-			goto out_bc;
-	}
+	/* Preference decision */
+	if (!tick_check_preferred(curdev, newdev))
+		goto out_bc;
 
 	if (!try_module_get(newdev->owner))
 		return;