From: Thomas Gleixner <[email protected]>
Xen calls on every cpu into tick_resume() which is just
wrong. tick_resume() is for the syscore global suspend/resume
invocation. What XEN really wants is a per cpu local resume function,
but yes, its simpler to just use something which works by chance and
let those who want to change that code deal with the fallout.
Provide a tick_resume_local() function and use it in XEN.
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Konrad Rzeszutek Wilk <[email protected]>
Cc: Boris Ostrovsky <[email protected]>
Cc: David Vrabel <[email protected]>
---
arch/x86/xen/suspend.c | 2 +-
include/linux/tick.h | 6 +++---
kernel/time/tick-broadcast.c | 27 ++++++++++++++++++++-------
kernel/time/tick-common.c | 28 +++++++++++++++++++---------
kernel/time/tick-internal.h | 8 ++++++--
5 files changed, 49 insertions(+), 22 deletions(-)
Index: linux/arch/x86/xen/suspend.c
===================================================================
--- linux.orig/arch/x86/xen/suspend.c
+++ linux/arch/x86/xen/suspend.c
@@ -85,7 +85,7 @@ static void xen_vcpu_notify_restore(void
if (smp_processor_id() == 0)
return;
- tick_resume();
+ tick_resume_local();
}
void xen_arch_resume(void)
Index: linux/include/linux/tick.h
===================================================================
--- linux.orig/include/linux/tick.h
+++ linux/include/linux/tick.h
@@ -27,11 +27,11 @@ extern struct tick_device *tick_get_devi
#ifdef CONFIG_GENERIC_CLOCKEVENTS
extern void __init tick_init(void);
-/* Should be core only, but XEN resume magic abuses this interface */
-extern void tick_resume(void);
+/* Should be core only, but XEN resume magic requires this */
+extern void tick_resume_local(void);
#else /* CONFIG_GENERIC_CLOCKEVENTS */
static inline void tick_init(void) { }
-static inline void tick_resume(void) { }
+static inline void tick_resume_local(void) { }
#endif /* !CONFIG_GENERIC_CLOCKEVENTS */
#ifdef CONFIG_TICK_ONESHOT
Index: linux/kernel/time/tick-broadcast.c
===================================================================
--- linux.orig/kernel/time/tick-broadcast.c
+++ linux/kernel/time/tick-broadcast.c
@@ -455,11 +455,29 @@ void tick_suspend_broadcast(void)
raw_spin_unlock_irqrestore(&tick_broadcast_lock, flags);
}
-int tick_resume_broadcast(void)
+/*
+ * This is called from tick_resume_local() on a resuming cpu. That's *
+ * called from the core resume function and the magic XEN resume hackery.
+ *
+ * It's intended to be called from tick_unfreeze_local() if the
+ * freezer folks figure out to guarantee that this is called before
+ * they reenable interrupts.
+ *
+ * In none of these cases the broadcast device mode can change and the
+ * bit of the resuming cpu in the broadcast mask is safe as well.
+ */
+bool tick_resume_check_broadcast(void)
+{
+ if (tick_broadcast_device.mode == TICKDEV_MODE_ONESHOT)
+ return false;
+ else
+ return cpumask_test_cpu(smp_processor_id(), tick_broadcast_mask);
+}
+
+void tick_resume_broadcast(void)
{
struct clock_event_device *bc;
unsigned long flags;
- int broadcast = 0;
raw_spin_lock_irqsave(&tick_broadcast_lock, flags);
@@ -472,8 +490,6 @@ int tick_resume_broadcast(void)
case TICKDEV_MODE_PERIODIC:
if (!cpumask_empty(tick_broadcast_mask))
tick_broadcast_start_periodic(bc);
- broadcast = cpumask_test_cpu(smp_processor_id(),
- tick_broadcast_mask);
break;
case TICKDEV_MODE_ONESHOT:
if (!cpumask_empty(tick_broadcast_mask))
@@ -482,11 +498,8 @@ int tick_resume_broadcast(void)
}
}
raw_spin_unlock_irqrestore(&tick_broadcast_lock, flags);
-
- return broadcast;
}
-
#ifdef CONFIG_TICK_ONESHOT
static cpumask_var_t tick_broadcast_oneshot_mask;
Index: linux/kernel/time/tick-common.c
===================================================================
--- linux.orig/kernel/time/tick-common.c
+++ linux/kernel/time/tick-common.c
@@ -389,22 +389,18 @@ void tick_suspend(void)
}
/**
- * tick_resume - Resume the tick and the broadcast device
+ * tick_resume_local - Resume the local tick device
*
- * Called from syscore_resume() via timekeeping_resume with only one
- * CPU online and interrupts disabled.
+ * Called from the local cpu for unfreeze or XEN resume magic
*
* No locks required. Nothing can change the per cpu device.
*/
-void tick_resume(void)
+void tick_resume_local(void)
{
- struct tick_device *td;
- int broadcast;
+ struct tick_device *td = this_cpu_ptr(&tick_cpu_device);
+ bool broadcast = tick_resume_check_broadcast();
- broadcast = tick_resume_broadcast();
- td = this_cpu_ptr(&tick_cpu_device);
clockevents_set_mode(td->evtdev, CLOCK_EVT_MODE_RESUME);
-
if (!broadcast) {
if (td->mode == TICKDEV_MODE_PERIODIC)
tick_setup_periodic(td->evtdev, 0);
@@ -414,6 +410,20 @@ void tick_resume(void)
}
/**
+ * tick_resume - Resume the tick and the broadcast device
+ *
+ * Called from syscore_resume() via timekeeping_resume with only one
+ * CPU online and interrupts disabled.
+ *
+ * No locks required. Nothing can change the per cpu device.
+ */
+void tick_resume(void)
+{
+ tick_resume_broadcast();
+ tick_resume_local();
+}
+
+/**
* tick_init - initialize the tick control
*/
void __init tick_init(void)
Index: linux/kernel/time/tick-internal.h
===================================================================
--- linux.orig/kernel/time/tick-internal.h
+++ linux/kernel/time/tick-internal.h
@@ -23,6 +23,7 @@ extern void tick_check_new_device(struct
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);
@@ -42,6 +43,7 @@ extern int __clockevents_update_freq(str
extern ssize_t sysfs_get_uname(const char *buf, char *dst, size_t cnt);
#else
static inline void tick_suspend(void) { }
+static inline void tick_resume(void) { }
#endif /* GENERIC_CLOCKEVENTS */
/* Oneshot related functions */
@@ -80,7 +82,8 @@ extern int tick_is_broadcast_device(stru
extern void tick_broadcast_on_off(unsigned long reason, int *oncpu);
extern void tick_shutdown_broadcast(unsigned int *cpup);
extern void tick_suspend_broadcast(void);
-extern int tick_resume_broadcast(void);
+extern void tick_resume_broadcast(void);
+extern bool tick_resume_check_broadcast(void);
extern void tick_broadcast_init(void);
extern void tick_set_periodic_handler(struct clock_event_device *dev, int broadcast);
extern int tick_broadcast_update_freq(struct clock_event_device *dev, u32 freq);
@@ -94,7 +97,8 @@ static inline void tick_do_periodic_broa
static inline void tick_broadcast_on_off(unsigned long reason, int *oncpu) { }
static inline void tick_shutdown_broadcast(unsigned int *cpup) { }
static inline void tick_suspend_broadcast(void) { }
-static inline int tick_resume_broadcast(void) { return 0; }
+static inline void tick_resume_broadcast(void) { }
+static inline bool tick_resume_check_broadcast(void) { return false; }
static inline void tick_broadcast_init(void) { }
static inline int tick_broadcast_update_freq(struct clock_event_device *dev, u32 freq) { return -ENODEV; }
On 16/02/15 12:14, Peter Zijlstra wrote:
>
> From: Thomas Gleixner <[email protected]>
>
> Xen calls on every cpu into tick_resume() which is just
> wrong. tick_resume() is for the syscore global suspend/resume
> invocation. What XEN really wants is a per cpu local resume function,
> but yes, its simpler to just use something which works by chance and
> let those who want to change that code deal with the fallout.
>
> Provide a tick_resume_local() function and use it in XEN.
Acked-by: David Vrabel <[email protected]>
David