Skip to contentSkip to conversation

clockevents: Add helpers to verify state of a clockevent device

PATCH New Viresh Kumar 2 participants 5 messages May 21, 2015 – Jun 2, 2015

Subsystems (2)
High-resolution timers, timer wheel, clockevents Nohz, dynticks support
lore
Share & tools
lore

Viresh Kumar

author
#0 lore raw

v2 cover clockevents: Add helpers to verify state of a clockevent device

Hi Thomas,

Some clockevent drivers, once migrated to use per-state callbacks, would need to verify 'state' of the clockevent device in their callbacks or interrupt handler.

(Details of these drivers are posted by Thomas earlier:
https://lkml.org/lkml/2015/5/20/454)

In order to discourage drivers from using CLK_EVT_STATE_* symbols directly and to make it easy grep for abusers of these symbols, this series introduces few accessor functions. The second patch updates core code to use these functions.

V1->V2:
- Don't move 'enum clock_event_state' to core's internal file.
- Move accessor functions to clockchips.h and mark them inline.
- New patch, 2/2.
- s/\<evt\>/dev in accessor functions

Rebased-over: tip/timers/core (dependency on 8fff52fd5093 ("clockevents:
Introduce CLOCK_EVT_STATE_ONESHOT_STOPPED state"))

--
viresh

Viresh Kumar (2):
clockevents: Add helpers to verify state of a clockevent device
clockevents: Use helpers to verify state of a clockevent device

 include/linux/clockchips.h   | 26 ++++++++++++++++++++++++++
 kernel/time/clockevents.c    | 24 ++++++++++++------------
 kernel/time/tick-broadcast.c |  6 +++---
 kernel/time/tick-common.c    |  2 +-
 kernel/time/tick-oneshot.c   |  2 +-
 5 files changed, 43 insertions(+), 17 deletions(-)
Signature · 1 line
2.4.0
author
#3 lore raw

PATCH v2 2/2 clockevents: Use helpers to verify state of a clockevent device

Use accessor functions to verify state of clockevent devices in core code.

Signed-off-by Viresh Kumar <[email protected]>
---
 kernel/time/clockevents.c    | 24 ++++++++++++------------
 kernel/time/tick-broadcast.c |  6 +++---
 kernel/time/tick-common.c    |  2 +-
 kernel/time/tick-oneshot.c   |  2 +-
 4 files changed, 17 insertions(+), 17 deletions(-)
kernel/time/clockevents.c +12 -12
index 4922f1b805ea..f533d3b13b92 100644
@@ -136,7 +136,7 @@ static int __clockevents_set_state(struct clock_event_device *dev,
 
 	case CLOCK_EVT_STATE_ONESHOT_STOPPED:
 		/* Core internal bug */
-		if (WARN_ONCE(dev->state != CLOCK_EVT_STATE_ONESHOT,
+		if (WARN_ONCE(!clockevent_state_oneshot(dev),
 			      "Current state: %d\n", dev->state))
 			return -EINVAL;
 

@@ -170,7 +170,7 @@ void clockevents_set_state(struct clock_event_device *dev,
 		 * A nsec2cyc multiplicator of 0 is invalid and we'd crash
 		 * on it, so fix it up and emit a warning:
 		 */
-		if (state == CLOCK_EVT_STATE_ONESHOT) {
+		if (clockevent_state_oneshot(dev)) {
 			if (unlikely(!dev->mult)) {
 				dev->mult = 1;
 				WARN_ON(1);

@@ -259,7 +259,7 @@ static int clockevents_program_min_delta(struct clock_event_device *dev)
 		delta = dev->min_delta_ns;
 		dev->next_event = ktime_add_ns(ktime_get(), delta);
 
-		if (dev->state == CLOCK_EVT_STATE_SHUTDOWN)
+		if (clockevent_state_shutdown(dev))
 			return 0;
 
 		dev->retries++;

@@ -296,7 +296,7 @@ static int clockevents_program_min_delta(struct clock_event_device *dev)
 	delta = dev->min_delta_ns;
 	dev->next_event = ktime_add_ns(ktime_get(), delta);
 
-	if (dev->state == CLOCK_EVT_STATE_SHUTDOWN)
+	if (clockevent_state_shutdown(dev))
 		return 0;
 
 	dev->retries++;

@@ -328,11 +328,11 @@ int clockevents_program_event(struct clock_event_device *dev, ktime_t expires,
 
 	dev->next_event = expires;
 
-	if (dev->state == CLOCK_EVT_STATE_SHUTDOWN)
+	if (clockevent_state_shutdown(dev))
 		return 0;
 
 	/* We must be in ONESHOT state here */
-	WARN_ONCE(dev->state != CLOCK_EVT_STATE_ONESHOT, "Current state: %d\n",
+	WARN_ONCE(!clockevent_state_oneshot(dev), "Current state: %d\n",
 		  dev->state);
 
 	/* Shortcut for clockevent devices that can deal with ktime. */

@@ -377,7 +377,7 @@ 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->state != CLOCK_EVT_STATE_DETACHED)
+		if (dev == ced || !clockevent_state_detached(dev))
 			continue;
 
 		if (!tick_check_replacement(newdev, dev))

@@ -403,7 +403,7 @@ static int clockevents_replace(struct clock_event_device *ced)
 static int __clockevents_try_unbind(struct clock_event_device *ced, int cpu)
 {
 	/* Fast track. Device is unused */
-	if (ced->state == CLOCK_EVT_STATE_DETACHED) {
+	if (clockevent_state_detached(ced)) {
 		list_del_init(&ced->list);
 		return 0;
 	}

@@ -561,10 +561,10 @@ int __clockevents_update_freq(struct clock_event_device *dev, u32 freq)
 {
 	clockevents_config(dev, freq);
 
-	if (dev->state == CLOCK_EVT_STATE_ONESHOT)
+	if (clockevent_state_oneshot(dev))
 		return clockevents_program_event(dev, dev->next_event, false);
 
-	if (dev->state == CLOCK_EVT_STATE_PERIODIC)
+	if (clockevent_state_periodic(dev))
 		return __clockevents_set_state(dev, CLOCK_EVT_STATE_PERIODIC);
 
 	return 0;

@@ -625,7 +625,7 @@ void clockevents_exchange_device(struct clock_event_device *old,
 	}
 
 	if (new) {
-		BUG_ON(new->state != CLOCK_EVT_STATE_DETACHED);
+		BUG_ON(!clockevent_state_detached(new));
 		clockevents_shutdown(new);
 	}
 }

@@ -681,7 +681,7 @@ void tick_cleanup_dead_cpu(int cpu)
 		if (cpumask_test_cpu(cpu, dev->cpumask) &&
 		    cpumask_weight(dev->cpumask) == 1 &&
 		    !tick_is_broadcast_device(dev)) {
-			BUG_ON(dev->state != CLOCK_EVT_STATE_DETACHED);
+			BUG_ON(!clockevent_state_detached(dev));
 			list_del(&dev->list);
 		}
 	}
kernel/time/tick-broadcast.c +3 -3
index 12fcc55d607a..132f819fdcdf 100644
@@ -303,7 +303,7 @@ static void tick_handle_periodic_broadcast(struct clock_event_device *dev)
 	raw_spin_lock(&tick_broadcast_lock);
 	bc_local = tick_do_periodic_broadcast();
 
-	if (dev->state == CLOCK_EVT_STATE_ONESHOT) {
+	if (clockevent_state_oneshot(dev)) {
 		ktime_t next = ktime_add(dev->next_event, tick_period);
 
 		clockevents_program_event(dev, next, true);

@@ -528,7 +528,7 @@ static void tick_broadcast_set_affinity(struct clock_event_device *bc,
 static void tick_broadcast_set_event(struct clock_event_device *bc, int cpu,
 				     ktime_t expires)
 {
-	if (bc->state != CLOCK_EVT_STATE_ONESHOT)
+	if (!clockevent_state_oneshot(bc))
 		clockevents_set_state(bc, CLOCK_EVT_STATE_ONESHOT);
 
 	clockevents_program_event(bc, expires, 1);

@@ -831,7 +831,7 @@ void tick_broadcast_setup_oneshot(struct clock_event_device *bc)
 
 	/* Set it up only once ! */
 	if (bc->event_handler != tick_handle_oneshot_broadcast) {
-		int was_periodic = bc->state == CLOCK_EVT_STATE_PERIODIC;
+		int was_periodic = clockevent_state_periodic(bc);
 
 		bc->event_handler = tick_handle_oneshot_broadcast;
kernel/time/tick-common.c +1 -1
index ea5f9eae8f74..cf881c62c3c5 100644
@@ -112,7 +112,7 @@ void tick_handle_periodic(struct clock_event_device *dev)
 		return;
 #endif
 
-	if (dev->state != CLOCK_EVT_STATE_ONESHOT)
+	if (!clockevent_state_oneshot(dev))
 		return;
 	for (;;) {
 		/*
kernel/time/tick-oneshot.c +1 -1
index f8de75715c2f..3f9715bec291 100644
@@ -36,7 +36,7 @@ int tick_program_event(ktime_t expires, int force)
 		return 0;
 	}
 
-	if (unlikely(dev->state == CLOCK_EVT_STATE_ONESHOT_STOPPED)) {
+	if (unlikely(clockevent_state_oneshot_stopped(dev))) {
 		/*
 		 * We need the clock event again, configure it in ONESHOT mode
 		 * before using it.
Signature · 1 line
2.4.0
author
#1 lore raw

PATCH v2 1/2 clockevents: Add helpers to verify state of a clockevent device

In reply to Viresh Kumar

Some clockevent drivers, once migrated to use per-state callbacks, would need to verify 'state' of the clockevent device in their callbacks or interrupt handler.

Add accessor functions clockevent_state_*() to get this information.

Signed-off-by Viresh Kumar <[email protected]>
---
 include/linux/clockchips.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)
include/linux/clockchips.h +26 -0
index 271fa4c8eb29..64214ad85af9 100644
@@ -149,6 +149,32 @@ struct clock_event_device {
 	struct module		*owner;
 } ____cacheline_aligned;
 
+/* Helpers to verify state of a clockevent device */
+static inline bool clockevent_state_detached(struct clock_event_device *dev)
+{
+	return dev->state == CLOCK_EVT_STATE_DETACHED;
+}
+
+static inline bool clockevent_state_shutdown(struct clock_event_device *dev)
+{
+	return dev->state == CLOCK_EVT_STATE_SHUTDOWN;
+}
+
+static inline bool clockevent_state_periodic(struct clock_event_device *dev)
+{
+	return dev->state == CLOCK_EVT_STATE_PERIODIC;
+}
+
+static inline bool clockevent_state_oneshot(struct clock_event_device *dev)
+{
+	return dev->state == CLOCK_EVT_STATE_ONESHOT;
+}
+
+static inline bool clockevent_state_oneshot_stopped(struct clock_event_device *dev)
+{
+	return dev->state == CLOCK_EVT_STATE_ONESHOT_STOPPED;
+}
+
 /*
  * Calculate a multiplication factor for scaled math, which is used to convert
  * nanoseconds based values to clock ticks:
Signature · 1 line
2.4.0
merge bot
#2 lore raw

[tip:timers/core] clockevents: Add helpers to check the state of a clockevent device

clockevents: Add helpers to check the state of a clockevent device
Commit-ID:  3434d23b694e5cb6e44e966914563406c31c4053
Gitweb:     http://git.kernel.org/tip/3434d23b694e5cb6e44e966914563406c31c4053
Author:     Viresh Kumar <[email protected]>
AuthorDate: Thu, 21 May 2015 13:33:45 +0530
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Tue, 2 Jun 2015 14:40:47 +0200

clockevents: Add helpers to check the state of a clockevent device

Some clockevent drivers, once migrated to use per-state callbacks, need to check the state of the clockevent device in their callbacks or interrupt handler.

Add accessor functions clockevent_state_*() to get this information.

Signed-off-by Viresh Kumar <[email protected]>
Cc [email protected]
Cc Peter Zijlstra <[email protected]>
Link http://lkml.kernel.org/r/04a717d490335c688dd7af899fbcede97e1bb8ee.1432192527.git.viresh.kumar@linaro.org open here
Signed-off-by Thomas Gleixner <[email protected]>
---
 include/linux/clockchips.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)
include/linux/clockchips.h +26 -0
index 271fa4c..64214ad 100644
@@ -149,6 +149,32 @@ struct clock_event_device {
 	struct module		*owner;
 } ____cacheline_aligned;
 
+/* Helpers to verify state of a clockevent device */
+static inline bool clockevent_state_detached(struct clock_event_device *dev)
+{
+	return dev->state == CLOCK_EVT_STATE_DETACHED;
+}
+
+static inline bool clockevent_state_shutdown(struct clock_event_device *dev)
+{
+	return dev->state == CLOCK_EVT_STATE_SHUTDOWN;
+}
+
+static inline bool clockevent_state_periodic(struct clock_event_device *dev)
+{
+	return dev->state == CLOCK_EVT_STATE_PERIODIC;
+}
+
+static inline bool clockevent_state_oneshot(struct clock_event_device *dev)
+{
+	return dev->state == CLOCK_EVT_STATE_ONESHOT;
+}
+
+static inline bool clockevent_state_oneshot_stopped(struct clock_event_device *dev)
+{
+	return dev->state == CLOCK_EVT_STATE_ONESHOT_STOPPED;
+}
+
 /*
  * Calculate a multiplication factor for scaled math, which is used to convert
  * nanoseconds based values to clock ticks:
merge bot
#4 lore raw

[tip:timers/core] clockevents: Use helpers to check the state of a clockevent device

In reply to Viresh Kumar
clockevents: Use helpers to check the state of a clockevent device
Commit-ID:  472c4a9437d3c6a0b1e59df7c5aa14075946aa70
Gitweb:     http://git.kernel.org/tip/472c4a9437d3c6a0b1e59df7c5aa14075946aa70
Author:     Viresh Kumar <[email protected]>
AuthorDate: Thu, 21 May 2015 13:33:46 +0530
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Tue, 2 Jun 2015 14:40:47 +0200

clockevents: Use helpers to check the state of a clockevent device

Use accessor functions to check the state of clockevent devices in core code.

Signed-off-by Viresh Kumar <[email protected]>
Cc [email protected]
Cc Peter Zijlstra <[email protected]>
Link http://lkml.kernel.org/r/fa2b9869fd17f210eaa156ec2b594efd0230b6c7.1432192527.git.viresh.kumar@linaro.org open here
Signed-off-by Thomas Gleixner <[email protected]>
---
 kernel/time/clockevents.c    | 24 ++++++++++++------------
 kernel/time/tick-broadcast.c |  6 +++---
 kernel/time/tick-common.c    |  2 +-
 kernel/time/tick-oneshot.c   |  2 +-
 4 files changed, 17 insertions(+), 17 deletions(-)
kernel/time/clockevents.c +12 -12
index 2a5c369..e568ec8 100644
@@ -136,7 +136,7 @@ static int __clockevents_set_state(struct clock_event_device *dev,
 
 	case CLOCK_EVT_STATE_ONESHOT_STOPPED:
 		/* Core internal bug */
-		if (WARN_ONCE(dev->state != CLOCK_EVT_STATE_ONESHOT,
+		if (WARN_ONCE(!clockevent_state_oneshot(dev),
 			      "Current state: %d\n", dev->state))
 			return -EINVAL;
 

@@ -170,7 +170,7 @@ void clockevents_set_state(struct clock_event_device *dev,
 		 * A nsec2cyc multiplicator of 0 is invalid and we'd crash
 		 * on it, so fix it up and emit a warning:
 		 */
-		if (state == CLOCK_EVT_STATE_ONESHOT) {
+		if (clockevent_state_oneshot(dev)) {
 			if (unlikely(!dev->mult)) {
 				dev->mult = 1;
 				WARN_ON(1);

@@ -259,7 +259,7 @@ static int clockevents_program_min_delta(struct clock_event_device *dev)
 		delta = dev->min_delta_ns;
 		dev->next_event = ktime_add_ns(ktime_get(), delta);
 
-		if (dev->state == CLOCK_EVT_STATE_SHUTDOWN)
+		if (clockevent_state_shutdown(dev))
 			return 0;
 
 		dev->retries++;

@@ -296,7 +296,7 @@ static int clockevents_program_min_delta(struct clock_event_device *dev)
 	delta = dev->min_delta_ns;
 	dev->next_event = ktime_add_ns(ktime_get(), delta);
 
-	if (dev->state == CLOCK_EVT_STATE_SHUTDOWN)
+	if (clockevent_state_shutdown(dev))
 		return 0;
 
 	dev->retries++;

@@ -328,11 +328,11 @@ int clockevents_program_event(struct clock_event_device *dev, ktime_t expires,
 
 	dev->next_event = expires;
 
-	if (dev->state == CLOCK_EVT_STATE_SHUTDOWN)
+	if (clockevent_state_shutdown(dev))
 		return 0;
 
 	/* We must be in ONESHOT state here */
-	WARN_ONCE(dev->state != CLOCK_EVT_STATE_ONESHOT, "Current state: %d\n",
+	WARN_ONCE(!clockevent_state_oneshot(dev), "Current state: %d\n",
 		  dev->state);
 
 	/* Shortcut for clockevent devices that can deal with ktime. */

@@ -377,7 +377,7 @@ 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->state != CLOCK_EVT_STATE_DETACHED)
+		if (dev == ced || !clockevent_state_detached(dev))
 			continue;
 
 		if (!tick_check_replacement(newdev, dev))

@@ -403,7 +403,7 @@ static int clockevents_replace(struct clock_event_device *ced)
 static int __clockevents_try_unbind(struct clock_event_device *ced, int cpu)
 {
 	/* Fast track. Device is unused */
-	if (ced->state == CLOCK_EVT_STATE_DETACHED) {
+	if (clockevent_state_detached(ced)) {
 		list_del_init(&ced->list);
 		return 0;
 	}

@@ -561,10 +561,10 @@ int __clockevents_update_freq(struct clock_event_device *dev, u32 freq)
 {
 	clockevents_config(dev, freq);
 
-	if (dev->state == CLOCK_EVT_STATE_ONESHOT)
+	if (clockevent_state_oneshot(dev))
 		return clockevents_program_event(dev, dev->next_event, false);
 
-	if (dev->state == CLOCK_EVT_STATE_PERIODIC)
+	if (clockevent_state_periodic(dev))
 		return __clockevents_set_state(dev, CLOCK_EVT_STATE_PERIODIC);
 
 	return 0;

@@ -625,7 +625,7 @@ void clockevents_exchange_device(struct clock_event_device *old,
 	}
 
 	if (new) {
-		BUG_ON(new->state != CLOCK_EVT_STATE_DETACHED);
+		BUG_ON(!clockevent_state_detached(new));
 		clockevents_shutdown(new);
 	}
 }

@@ -681,7 +681,7 @@ void tick_cleanup_dead_cpu(int cpu)
 		if (cpumask_test_cpu(cpu, dev->cpumask) &&
 		    cpumask_weight(dev->cpumask) == 1 &&
 		    !tick_is_broadcast_device(dev)) {
-			BUG_ON(dev->state != CLOCK_EVT_STATE_DETACHED);
+			BUG_ON(!clockevent_state_detached(dev));
 			list_del(&dev->list);
 		}
 	}
kernel/time/tick-broadcast.c +3 -3
index 12fcc55..132f819f 100644
@@ -303,7 +303,7 @@ static void tick_handle_periodic_broadcast(struct clock_event_device *dev)
 	raw_spin_lock(&tick_broadcast_lock);
 	bc_local = tick_do_periodic_broadcast();
 
-	if (dev->state == CLOCK_EVT_STATE_ONESHOT) {
+	if (clockevent_state_oneshot(dev)) {
 		ktime_t next = ktime_add(dev->next_event, tick_period);
 
 		clockevents_program_event(dev, next, true);

@@ -528,7 +528,7 @@ static void tick_broadcast_set_affinity(struct clock_event_device *bc,
 static void tick_broadcast_set_event(struct clock_event_device *bc, int cpu,
 				     ktime_t expires)
 {
-	if (bc->state != CLOCK_EVT_STATE_ONESHOT)
+	if (!clockevent_state_oneshot(bc))
 		clockevents_set_state(bc, CLOCK_EVT_STATE_ONESHOT);
 
 	clockevents_program_event(bc, expires, 1);

@@ -831,7 +831,7 @@ void tick_broadcast_setup_oneshot(struct clock_event_device *bc)
 
 	/* Set it up only once ! */
 	if (bc->event_handler != tick_handle_oneshot_broadcast) {
-		int was_periodic = bc->state == CLOCK_EVT_STATE_PERIODIC;
+		int was_periodic = clockevent_state_periodic(bc);
 
 		bc->event_handler = tick_handle_oneshot_broadcast;
kernel/time/tick-common.c +1 -1
index ea5f9ea..cf881c6 100644
@@ -112,7 +112,7 @@ void tick_handle_periodic(struct clock_event_device *dev)
 		return;
 #endif
 
-	if (dev->state != CLOCK_EVT_STATE_ONESHOT)
+	if (!clockevent_state_oneshot(dev))
 		return;
 	for (;;) {
 		/*
kernel/time/tick-oneshot.c +1 -1
index f8de757..3f9715be 100644
@@ -36,7 +36,7 @@ int tick_program_event(ktime_t expires, int force)
 		return 0;
 	}
 
-	if (unlikely(dev->state == CLOCK_EVT_STATE_ONESHOT_STOPPED)) {
+	if (unlikely(clockevent_state_oneshot_stopped(dev))) {
 		/*
 		 * We need the clock event again, configure it in ONESHOT mode
 		 * before using it.