Re: [PATCH 08/09] tick/xen: Provide and use tick_suspend_local() and tick_resume_local()
From: Rafael J. Wysocki
Date: Wed Mar 25 2015 - 17:12:39 EST
On Wednesday, March 25, 2015 04:44:45 PM Konrad Rzeszutek Wilk wrote:
> On Wed, Mar 25, 2015 at 09:57:18PM +0100, Rafael J. Wysocki wrote:
> > On Wednesday, March 25, 2015 04:13:09 PM Konrad Rzeszutek Wilk wrote:
> > > On Wed, Mar 25, 2015 at 01:11:04PM +0100, Rafael J. Wysocki wrote:
> > > > From: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> > > >
> > > > 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.
> > > >
> > > > Provide a tick_resume_local() function and use it in XEN.
> > > >
> > > > Also provide a complementary tick_suspend_local() and modify
> > > > tick_unfreeze() and tick_freeze(), respectively, to use the
> > > > new local tick resume/suspend functions.
> > > >
> > > > Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> > > > Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> > > > Cc: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> > > > Cc: David Vrabel <david.vrabel@xxxxxxxxxx>
> > > > [ rjw: Combined two patches, rebased, modified subject/changelog ]
> > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > > > ---
> > > > arch/x86/xen/suspend.c | 2 -
> > > > include/linux/tick.h | 6 ++--
> > > > kernel/time/tick-broadcast.c | 24 ++++++++++++------
> > > > kernel/time/tick-common.c | 57 +++++++++++++++++++++++++++++--------------
> > > > kernel/time/tick-internal.h | 8 ++++--
> > > > 5 files changed, 66 insertions(+), 31 deletions(-)
> > > >
> > > > Index: linux-pm/arch/x86/xen/suspend.c
> > > > ===================================================================
> > > > --- linux-pm.orig/arch/x86/xen/suspend.c
> > > > +++ linux-pm/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();
> > >
> > > I am looking at Linus's tree and I see:
> > >
> > > 82 static void xen_vcpu_notify_restore(void *data)
> > > 83 {
> > > 84 unsigned long reason = (unsigned long)data;
> > > 85
> > > 86 /* Boot processor notified via generic timekeeping_resume() */
> > > 87 if ( smp_processor_id() == 0)
> > > 88 return;
> > > 89
> > > 90 clockevents_notify(reason, NULL);
> > > 91 }
> > >
> > > So there must be another patch that altered the clockevents_notify?
> >
> > Yes, there is. It is the [06/09] in this series
> > (https://patchwork.kernel.org/patch/6090631/).
> >
> > I should have CCed it to you too, sorry about that.
>
> That is alright.
> >
> > > Is there an git tree with all of these patches?
> >
> > Not yet, but they all are in the linux-pm patchwork at
> >
> > https://patchwork.kernel.org/project/linux-pm/list/
> >
> > I can create a branch in the linux-pm.git tree for them if that helps,
> > please let me know.
>
> If it is not too much trouble..
Done:
git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git tick-pm
Also turns out that the $subject patch was actually on top of some debug stuff
I have in tick_freeze()/tick_unfreeze(), so here goes a cleaned-up updated.
---
From: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
Subject: tick/xen: Provide and use tick_suspend_local() and tick_resume_local()
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.
Provide a tick_resume_local() function and use it in XEN.
Also provide a complementary tick_suspend_local() and modify
tick_unfreeze() and tick_freeze(), respectively, to use the
new local tick resume/suspend functions.
Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Cc: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
Cc: David Vrabel <david.vrabel@xxxxxxxxxx>
[ rjw: Combined two patches, rebased, modified subject/changelog ]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
---
arch/x86/xen/suspend.c | 2 -
include/linux/tick.h | 6 ++--
kernel/time/tick-broadcast.c | 24 +++++++++++-----
kernel/time/tick-common.c | 62 ++++++++++++++++++++++++++++---------------
kernel/time/tick-internal.h | 8 ++++-
5 files changed, 68 insertions(+), 34 deletions(-)
Index: linux-pm/arch/x86/xen/suspend.c
===================================================================
--- linux-pm.orig/arch/x86/xen/suspend.c
+++ linux-pm/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-pm/include/linux/tick.h
===================================================================
--- linux-pm.orig/include/linux/tick.h
+++ linux-pm/include/linux/tick.h
@@ -29,13 +29,13 @@ extern struct tick_device *tick_get_devi
extern void __init tick_init(void);
extern void tick_freeze(void);
extern void tick_unfreeze(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_freeze(void) { }
static inline void tick_unfreeze(void) { }
-static inline void tick_resume(void) { }
+static inline void tick_resume_local(void) { }
#endif /* !CONFIG_GENERIC_CLOCKEVENTS */
#ifdef CONFIG_TICK_ONESHOT
Index: linux-pm/kernel/time/tick-broadcast.c
===================================================================
--- linux-pm.orig/kernel/time/tick-broadcast.c
+++ linux-pm/kernel/time/tick-broadcast.c
@@ -455,11 +455,26 @@ 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, tick_unfreeze() and the magic XEN
+ * resume hackery.
+ *
+ * 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 +487,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 +495,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-pm/kernel/time/tick-common.c
===================================================================
--- linux-pm.orig/kernel/time/tick-common.c
+++ linux-pm/kernel/time/tick-common.c
@@ -373,40 +373,32 @@ void tick_shutdown(unsigned int *cpup)
}
/**
- * tick_suspend - Suspend the tick and the broadcast device
+ * tick_suspend_local - Suspend the local tick device
*
- * Called from syscore_suspend() via timekeeping_suspend with only one
- * CPU online and interrupts disabled or from tick_unfreeze() under
- * tick_freeze_lock.
+ * Called from the local cpu for freeze with interrupts disabled.
*
* No locks required. Nothing can change the per cpu device.
*/
-void tick_suspend(void)
+static void tick_suspend_local(void)
{
struct tick_device *td = this_cpu_ptr(&tick_cpu_device);
clockevents_shutdown(td->evtdev);
- tick_suspend_broadcast();
}
/**
- * 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 or from tick_unfreeze() under
- * tick_freeze_lock.
+ * 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);
@@ -415,6 +407,35 @@ void tick_resume(void)
}
}
+/**
+ * tick_suspend - Suspend the tick and the broadcast device
+ *
+ * Called from syscore_suspend() via timekeeping_suspend with only one
+ * CPU online and interrupts disabled or from tick_unfreeze() under
+ * tick_freeze_lock.
+ *
+ * No locks required. Nothing can change the per cpu device.
+ */
+void tick_suspend(void)
+{
+ tick_suspend_local();
+ tick_suspend_broadcast();
+}
+
+/**
+ * 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();
+}
+
static DEFINE_RAW_SPINLOCK(tick_freeze_lock);
static unsigned int tick_freeze_depth;
@@ -432,11 +453,10 @@ void tick_freeze(void)
raw_spin_lock(&tick_freeze_lock);
tick_freeze_depth++;
- if (tick_freeze_depth == num_online_cpus()) {
+ if (tick_freeze_depth == num_online_cpus())
timekeeping_suspend();
- } else {
- tick_suspend();
- }
+ else
+ tick_suspend_local();
raw_spin_unlock(&tick_freeze_lock);
}
@@ -457,7 +477,7 @@ void tick_unfreeze(void)
if (tick_freeze_depth == num_online_cpus())
timekeeping_resume();
else
- tick_resume();
+ tick_resume_local();
tick_freeze_depth--;
Index: linux-pm/kernel/time/tick-internal.h
===================================================================
--- linux-pm.orig/kernel/time/tick-internal.h
+++ linux-pm/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; }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/