Ian Campbell
2011-Oct-03 15:10 UTC
[Xen-devel] xen: IPI interrupts not resumed early enough on suspend/resume
Hi Thomas, Recently I''ve been chasing an issue where a Xen guest will fail to resume about 1 time in 100. I eventually managed to bisect this back to 676dc3cf5bc3 "xen: Use IRQF_FORCE_RESUME". The Xen suspend procedure (drivers/xen/manage.c:do_suspend()) is roughly (I''ve omitted some uninteresting parts) as follows: dpm_suspend_start() dpm_suspend_noirq() stop_machine() -> xen_suspend() syscore_suspend() HYPERVISOR_suspend() /* Hypercall, returns on resume */ xen_irq_resume() /* Re-establishes evtchn<->irq bindings */ syscore_resume() dpm_resume_noirq() dpm_resume_end() The resume process appears to be coming to a halt at the end of the stop_machine invocation of xen_suspend(), i.e. after syscore_resume() but before dpm_resume_noirq(). Looking at the stack traces of all VCPUs when this happens it appears that they are all idle, which suggests we are missing an event to cause a reschedule out of the stop_machine thread back into the suspending thread. One of the effects of 676dc3cf5bc3 was to move the unmasking of the timer and IPI interrupts from xen_irq_resume() (i.e. within the stop_machine region) to dpm_resume_noirq() (i.e. outside the stop_machine region). Since the IPI interrupts includes the reschedule IPI I rather suspect that is the reason for the problem. I added a hack to unmask the reched* IPIs at xen_irq_resume() time and so far it seems to fix things, which backs up my gut feeling. I can see a few options for how I might go about solving this in a non-hacky way, which approach do you think would be preferable: * Add "IRQF_RESUME_EARLY", driven from syscore_resume, and use it for these interrupts. * register syscore ops for the Xen event channel subsystem to unmask the IPIs earlier (would probably look a lot like the code removed by 676dc3cf5bc3). * add syscore_ops to Xen smp subsystem to unmask the specific IPIs (which it binds at start of day) earlier. * push dpm_(suspend|resume)_noirq down into stop machine region * use something other than stop_machine to quiesce system and move to cpu0 for suspend (doesn''t seem sensible to reproduce that functionality). Routing IPIs through the regular IRQ path seems a little bit unusual but it looks like powerpc does something similar in smp_request_message_ipi and mpic_request_ipis and that code uses the syscore approach. Does applying that here too seem sane? Any preference / advice? Thanks, Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Thomas Gleixner
2011-Oct-03 18:42 UTC
[Xen-devel] Re: xen: IPI interrupts not resumed early enough on suspend/resume
On Mon, 3 Oct 2011, Ian Campbell wrote:> I can see a few options for how I might go about solving this in a > non-hacky way, which approach do you think would be preferable:The question is whether you need to disable the IPI interrupt at all. If not, we have a flag for that.> * Add "IRQF_RESUME_EARLY", driven from syscore_resume, and use it > for these interrupts.That''s the preferable solution, as we could use that for PPC as well, unless we can move stuff around, so we disable stuff later.> * register syscore ops for the Xen event channel subsystem to > unmask the IPIs earlier (would probably look a lot like the code > removed by 676dc3cf5bc3).I''d like to avoid that.> * add syscore_ops to Xen smp subsystem to unmask the specific IPIs > (which it binds at start of day) earlier. > * push dpm_(suspend|resume)_noirq down into stop machine regionWhere is stomp machine used?> * use something other than stop_machine to quiesce system and move > to cpu0 for suspend (doesn''t seem sensible to reproduce that > functionality).We already shut down the nonboot cpus on suspend. We could do that _before_ we disable devices and the interrupts. Raphael ? Thanks, tglx _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Oct-03 19:08 UTC
[Xen-devel] Re: xen: IPI interrupts not resumed early enough on suspend/resume
On Mon, 2011-10-03 at 19:42 +0100, Thomas Gleixner wrote:> On Mon, 3 Oct 2011, Ian Campbell wrote: > > I can see a few options for how I might go about solving this in a > > non-hacky way, which approach do you think would be preferable: > > The question is whether you need to disable the IPI interrupt at > all. If not, we have a flag for that.We already that flag for these (I think that was why it was added even). The issue is that in the resuming domain on the other side event channels all start off masked and something needs to unmask them.> > * Add "IRQF_RESUME_EARLY", driven from syscore_resume, and use it > > for these interrupts. > > That''s the preferable solution, as we could use that for PPC as well, > unless we can move stuff around, so we disable stuff later.OK> > * register syscore ops for the Xen event channel subsystem to > > unmask the IPIs earlier (would probably look a lot like the code > > removed by 676dc3cf5bc3). > > I''d like to avoid that.Sure.> > * add syscore_ops to Xen smp subsystem to unmask the specific IPIs > > (which it binds at start of day) earlier. > > * push dpm_(suspend|resume)_noirq down into stop machine region > > Where is stomp machine used?It is used by the xen PV suspend handler which runs in that context in order to quiesce non-boot CPUs (which Xen does not unplug like native does).> > * use something other than stop_machine to quiesce system and move > > to cpu0 for suspend (doesn''t seem sensible to reproduce that > > functionality). > > We already shut down the nonboot cpus on suspend. We could do that > _before_ we disable devices and the interrupts.Xen PV suspend uses many of the PM/suspend core code paths but it does not have the bit which shuts down non-boot CPUs. It was a while ago but IIRC Xen used to unplug the secondary processors and it was found to lead to larger latencies in the migration and checkpointing cases (which at their core are a suspend/resume). The disaster recovery folks in particular care about this latency since they want to do rolling checkpoints many times a second. Ian.> > Raphael ? > > Thanks, > > tglx_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Rafael J. Wysocki
2011-Oct-03 20:02 UTC
[Xen-devel] Re: xen: IPI interrupts not resumed early enough on suspend/resume
On Monday, October 03, 2011, Thomas Gleixner wrote:> On Mon, 3 Oct 2011, Ian Campbell wrote: > > I can see a few options for how I might go about solving this in a > > non-hacky way, which approach do you think would be preferable: > > The question is whether you need to disable the IPI interrupt at > all. If not, we have a flag for that. > > > * Add "IRQF_RESUME_EARLY", driven from syscore_resume, and use it > > for these interrupts. > > That''s the preferable solution, as we could use that for PPC as well, > unless we can move stuff around, so we disable stuff later. > > > * register syscore ops for the Xen event channel subsystem to > > unmask the IPIs earlier (would probably look a lot like the code > > removed by 676dc3cf5bc3). > > I''d like to avoid that. > > > * add syscore_ops to Xen smp subsystem to unmask the specific IPIs > > (which it binds at start of day) earlier. > > * push dpm_(suspend|resume)_noirq down into stop machine region > > Where is stomp machine used? > > > * use something other than stop_machine to quiesce system and move > > to cpu0 for suspend (doesn''t seem sensible to reproduce that > > functionality). > > We already shut down the nonboot cpus on suspend. We could do that > _before_ we disable devices and the interrupts. > > Raphael ?I''m afraid that wouldn''t work. At least right now our suspend sequence is reasonably in line with what ACPI says. Besides, we suspend devices in parallel now, so that would be going backwards a bit. Thanks, Rafael _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Thomas Gleixner
2011-Oct-03 20:28 UTC
[Xen-devel] Re: xen: IPI interrupts not resumed early enough on suspend/resume
On Mon, 3 Oct 2011, Rafael J. Wysocki wrote:> On Monday, October 03, 2011, Thomas Gleixner wrote: > > We already shut down the nonboot cpus on suspend. We could do that > > _before_ we disable devices and the interrupts. > > > > Raphael ? > > I''m afraid that wouldn''t work. At least right now our suspend sequence is > reasonably in line with what ACPI says. > > Besides, we suspend devices in parallel now, so that would be going backwards > a bit.Fair enough. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Thomas Gleixner
2011-Oct-03 20:35 UTC
[Xen-devel] Re: xen: IPI interrupts not resumed early enough on suspend/resume
On Mon, 3 Oct 2011, Ian Campbell wrote:> On Mon, 2011-10-03 at 19:42 +0100, Thomas Gleixner wrote: > > On Mon, 3 Oct 2011, Ian Campbell wrote: > > > I can see a few options for how I might go about solving this in a > > > non-hacky way, which approach do you think would be preferable: > > > > The question is whether you need to disable the IPI interrupt at > > all. If not, we have a flag for that. > > We already that flag for these (I think that was why it was added even). > The issue is that in the resuming domain on the other side event > channels all start off masked and something needs to unmask them.Bah.> > > * Add "IRQF_RESUME_EARLY", driven from syscore_resume, and use it > > > for these interrupts. > > > > That''s the preferable solution, as we could use that for PPC as well, > > unless we can move stuff around, so we disable stuff later. > > OKI guess we should go down that road then. Thanks, tglx _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Oct-07 15:32 UTC
[Xen-devel] Re: xen: IPI interrupts not resumed early enough on suspend/resume
On Mon, 2011-10-03 at 22:35 +0200, Thomas Gleixner wrote:> > > > * Add "IRQF_RESUME_EARLY", driven from syscore_resume, and > use it > > > > for these interrupts. > > > > > > That''s the preferable solution, as we could use that for PPC as > well, > > > unless we can move stuff around, so we disable stuff later. > > > > OK > > I guess we should go down that road then.How does this look? I''ve done 100 successful migrations with it and am running another batch as we speak. BTW original changeset which exposed this has made it into at least the longterm 2.6.32 release (http://bugs.debian.org/644604) so I''m cc''ing stable@ too. Ian. 8<-------------------------->From bb24b9b45e071e5290a71a6445f4c156b8341699 Mon Sep 17 00:00:00 2001From: Ian Campbell <ian.campbell@citrix.com> Date: Mon, 3 Oct 2011 15:37:00 +0100 Subject: [PATCH] irq: add IRQF_RESUME_EARLY and resume such IRQs earlier in the process This adds a mechanism to resume selected IRQs during syscore_resume instead of dpm_resume_noirq. Under Xen we need to resume IRQs associated with IPIs earlier enough that the resched ipi is unmasked and we can therefore schedule ourselves out of the stop_machine where the suspend/resume takes place. This issue was introduced by 676dc3cf5bc3 "xen: Use IRQF_FORCE_RESUME". Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Rafael J. Wysocki <rjw@sisk.pl> Cc: Jeremy Fitzhardinge <jeremy@goop.org> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Cc: xen-devel <xen-devel@lists.xensource.com> Cc: stable@kernel.org (at least to 2.6.32.y) --- drivers/xen/events.c | 2 +- include/linux/interrupt.h | 2 ++ kernel/irq/pm.c | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletions(-) diff --git a/drivers/xen/events.c b/drivers/xen/events.c index 7523719..44490de 100644 --- a/drivers/xen/events.c +++ b/drivers/xen/events.c @@ -1021,7 +1021,7 @@ int bind_ipi_to_irqhandler(enum ipi_vector ipi, if (irq < 0) return irq; - irqflags |= IRQF_NO_SUSPEND | IRQF_FORCE_RESUME; + irqflags |= IRQF_NO_SUSPEND | IRQF_FORCE_RESUME | IRQF_EARLY_RESUME; retval = request_irq(irq, handler, irqflags, devname, dev_id); if (retval != 0) { unbind_from_irq(irq); diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h index a103732..a3b8baa 100644 --- a/include/linux/interrupt.h +++ b/include/linux/interrupt.h @@ -59,6 +59,7 @@ * IRQF_NO_SUSPEND - Do not disable this IRQ during suspend * IRQF_FORCE_RESUME - Force enable it on resume even if IRQF_NO_SUSPEND is set * IRQF_NO_THREAD - Interrupt cannot be threaded + * IRQF_EARLY_RESUME - Resume IRQ early during syscore instead of at device resume time */ #define IRQF_DISABLED 0x00000020 #define IRQF_SAMPLE_RANDOM 0x00000040 @@ -72,6 +73,7 @@ #define IRQF_NO_SUSPEND 0x00004000 #define IRQF_FORCE_RESUME 0x00008000 #define IRQF_NO_THREAD 0x00010000 +#define IRQF_EARLY_RESUME 0x00020000 #define IRQF_TIMER (__IRQF_TIMER | IRQF_NO_SUSPEND | IRQF_NO_THREAD) diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c index f76fc00..4fbfb36 100644 --- a/kernel/irq/pm.c +++ b/kernel/irq/pm.c @@ -9,6 +9,7 @@ #include <linux/irq.h> #include <linux/module.h> #include <linux/interrupt.h> +#include <linux/syscore_ops.h> #include "internals.h" @@ -40,6 +41,40 @@ void suspend_device_irqs(void) EXPORT_SYMBOL_GPL(suspend_device_irqs); /** + * irq_pm_syscore_ops - enable interrupt lines early + * + * Enable all interrupt lines with %IRQF_EARLY_RESUME set + */ +static void irq_pm_syscore_resume(void) +{ + struct irq_desc *desc; + int irq; + + for_each_irq_desc(irq, desc) { + unsigned long flags; + + if (!desc->action || !(desc->action->flags & IRQF_EARLY_RESUME)) + continue; + + raw_spin_lock_irqsave(&desc->lock, flags); + __enable_irq(desc, irq, true); + raw_spin_unlock_irqrestore(&desc->lock, flags); + } +} + +static struct syscore_ops irq_pm_syscore_ops = { + .resume = irq_pm_syscore_resume, +}; + +static int __init irq_pm_init_ops(void) +{ + register_syscore_ops(&irq_pm_syscore_ops); + return 0; +} + +device_initcall(irq_pm_init_ops); + +/** * resume_device_irqs - enable interrupt lines disabled by suspend_device_irqs() * * Enable all interrupt lines previously disabled by suspend_device_irqs() that @@ -53,6 +88,9 @@ void resume_device_irqs(void) for_each_irq_desc(irq, desc) { unsigned long flags; + if (desc->action && desc->action->flags & IRQF_EARLY_RESUME) + continue; + raw_spin_lock_irqsave(&desc->lock, flags); __enable_irq(desc, irq, true); raw_spin_unlock_irqrestore(&desc->lock, flags); -- 1.7.2.5 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Thomas Gleixner
2011-Oct-07 16:29 UTC
[Xen-devel] Re: xen: IPI interrupts not resumed early enough on suspend/resume
On Fri, 7 Oct 2011, Ian Campbell wrote:> On Mon, 2011-10-03 at 22:35 +0200, Thomas Gleixner wrote: > /** > + * irq_pm_syscore_ops - enable interrupt lines early > + * > + * Enable all interrupt lines with %IRQF_EARLY_RESUME set > + */ > +static void irq_pm_syscore_resume(void) > +{ > + struct irq_desc *desc; > + int irq; > + > + for_each_irq_desc(irq, desc) { > + unsigned long flags; > + > + if (!desc->action || !(desc->action->flags & IRQF_EARLY_RESUME)) > + continue; > + > + raw_spin_lock_irqsave(&desc->lock, flags); > + __enable_irq(desc, irq, true); > + raw_spin_unlock_irqrestore(&desc->lock, flags); > + } > +}Come on, this is a full copy of resume_device_irqs(). What about having a common function with an (bool early) argument and call it from both syscore and resume_device? Thanks, tglx _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Oct-10 13:06 UTC
[Xen-devel] Re: xen: IPI interrupts not resumed early enough on suspend/resume
On Fri, 2011-10-07 at 17:29 +0100, Thomas Gleixner wrote:> On Fri, 7 Oct 2011, Ian Campbell wrote: > > On Mon, 2011-10-03 at 22:35 +0200, Thomas Gleixner wrote: > > /** > > + * irq_pm_syscore_ops - enable interrupt lines early > > + * > > + * Enable all interrupt lines with %IRQF_EARLY_RESUME set > > + */ > > +static void irq_pm_syscore_resume(void) > > +{ > > + struct irq_desc *desc; > > + int irq; > > + > > + for_each_irq_desc(irq, desc) { > > + unsigned long flags; > > + > > + if (!desc->action || !(desc->action->flags & IRQF_EARLY_RESUME)) > > + continue; > > + > > + raw_spin_lock_irqsave(&desc->lock, flags); > > + __enable_irq(desc, irq, true); > > + raw_spin_unlock_irqrestore(&desc->lock, flags); > > + } > > +} > > Come on, this is a full copy of resume_device_irqs(). What about > having a common function with an (bool early) argument and call it > from both syscore and resume_device?Yes, that''s a good idea.>From dc8a8329f3e97f30e1290c898c816250aa7e8957 Mon Sep 17 00:00:00 2001From: Ian Campbell <ian.campbell@citrix.com> Date: Mon, 3 Oct 2011 15:37:00 +0100 Subject: [PATCH] irq: add IRQF_RESUME_EARLY and resume such IRQs earlier in the process This adds a mechanism to resume selected IRQs during syscore_resume instead of dpm_resume_noirq. Under Xen we need to resume IRQs associated with IPIs early enough that the resched IPI is unmasked and we can therefore schedule ourselves out of the stop_machine where the suspend/resume takes place. This issue was introduced by 676dc3cf5bc3 "xen: Use IRQF_FORCE_RESUME". Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Rafael J. Wysocki <rjw@sisk.pl> Cc: Jeremy Fitzhardinge <jeremy@goop.org> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Cc: xen-devel <xen-devel@lists.xensource.com> Cc: stable@kernel.org (at least to 2.6.32.y) --- drivers/xen/events.c | 2 +- include/linux/interrupt.h | 3 ++ kernel/irq/pm.c | 48 ++++++++++++++++++++++++++++++++++++++------ 3 files changed, 45 insertions(+), 8 deletions(-) diff --git a/drivers/xen/events.c b/drivers/xen/events.c index 7523719..44490de 100644 --- a/drivers/xen/events.c +++ b/drivers/xen/events.c @@ -1021,7 +1021,7 @@ int bind_ipi_to_irqhandler(enum ipi_vector ipi, if (irq < 0) return irq; - irqflags |= IRQF_NO_SUSPEND | IRQF_FORCE_RESUME; + irqflags |= IRQF_NO_SUSPEND | IRQF_FORCE_RESUME | IRQF_EARLY_RESUME; retval = request_irq(irq, handler, irqflags, devname, dev_id); if (retval != 0) { unbind_from_irq(irq); diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h index a103732..f51a81b 100644 --- a/include/linux/interrupt.h +++ b/include/linux/interrupt.h @@ -59,6 +59,8 @@ * IRQF_NO_SUSPEND - Do not disable this IRQ during suspend * IRQF_FORCE_RESUME - Force enable it on resume even if IRQF_NO_SUSPEND is set * IRQF_NO_THREAD - Interrupt cannot be threaded + * IRQF_EARLY_RESUME - Resume IRQ early during syscore instead of at device + * resume time. */ #define IRQF_DISABLED 0x00000020 #define IRQF_SAMPLE_RANDOM 0x00000040 @@ -72,6 +74,7 @@ #define IRQF_NO_SUSPEND 0x00004000 #define IRQF_FORCE_RESUME 0x00008000 #define IRQF_NO_THREAD 0x00010000 +#define IRQF_EARLY_RESUME 0x00020000 #define IRQF_TIMER (__IRQF_TIMER | IRQF_NO_SUSPEND | IRQF_NO_THREAD) diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c index f76fc00..86bcd79 100644 --- a/kernel/irq/pm.c +++ b/kernel/irq/pm.c @@ -9,6 +9,7 @@ #include <linux/irq.h> #include <linux/module.h> #include <linux/interrupt.h> +#include <linux/syscore_ops.h> #include "internals.h" @@ -39,25 +40,58 @@ void suspend_device_irqs(void) } EXPORT_SYMBOL_GPL(suspend_device_irqs); -/** - * resume_device_irqs - enable interrupt lines disabled by suspend_device_irqs() - * - * Enable all interrupt lines previously disabled by suspend_device_irqs() that - * have the IRQS_SUSPENDED flag set. - */ -void resume_device_irqs(void) +static resume_irqs(bool want_early) { struct irq_desc *desc; int irq; for_each_irq_desc(irq, desc) { unsigned long flags; + bool is_early = desc->action && + desc->action->flags & IRQF_EARLY_RESUME; + + if (is_early != want_early) + continue; raw_spin_lock_irqsave(&desc->lock, flags); __enable_irq(desc, irq, true); raw_spin_unlock_irqrestore(&desc->lock, flags); } } + +/** + * irq_pm_syscore_ops - enable interrupt lines early + * + * Enable all interrupt lines with %IRQF_EARLY_RESUME set. + */ +static void irq_pm_syscore_resume(void) +{ + resume_irqs(true); +} + +static struct syscore_ops irq_pm_syscore_ops = { + .resume = irq_pm_syscore_resume, +}; + +static int __init irq_pm_init_ops(void) +{ + register_syscore_ops(&irq_pm_syscore_ops); + return 0; +} + +device_initcall(irq_pm_init_ops); + +/** + * resume_device_irqs - enable interrupt lines disabled by suspend_device_irqs() + * + * Enable all non-%IRQF_EARLY_RESUME interrupt lines previously + * disabled by suspend_device_irqs() that have the IRQS_SUSPENDED flag + * set as well as those with %IRQF_FORCE_RESUME. + */ +void resume_device_irqs(void) +{ + resume_irqs(false); +} EXPORT_SYMBOL_GPL(resume_device_irqs); /** -- 1.7.2.5 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Oct-14 13:23 UTC
[Xen-devel] Re: xen: IPI interrupts not resumed early enough on suspend/resume
Ping? The original issue has made it into longterm-2.6.32.y and then onto Debian (bug #644604) so people keep asking me about it... Ian. On Mon, 2011-10-10 at 14:06 +0100, Ian Campbell wrote:> From dc8a8329f3e97f30e1290c898c816250aa7e8957 Mon Sep 17 00:00:00 2001 > From: Ian Campbell <ian.campbell@citrix.com> > Date: Mon, 3 Oct 2011 15:37:00 +0100 > Subject: [PATCH] irq: add IRQF_RESUME_EARLY and resume such IRQs earlier in the process > > This adds a mechanism to resume selected IRQs during syscore_resume instead of > dpm_resume_noirq. > > Under Xen we need to resume IRQs associated with IPIs early enough that the > resched IPI is unmasked and we can therefore schedule ourselves out of the > stop_machine where the suspend/resume takes place. > > This issue was introduced by 676dc3cf5bc3 "xen: Use IRQF_FORCE_RESUME". > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Rafael J. Wysocki <rjw@sisk.pl> > Cc: Jeremy Fitzhardinge <jeremy@goop.org> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > Cc: xen-devel <xen-devel@lists.xensource.com> > Cc: stable@kernel.org (at least to 2.6.32.y) > --- > drivers/xen/events.c | 2 +- > include/linux/interrupt.h | 3 ++ > kernel/irq/pm.c | 48 ++++++++++++++++++++++++++++++++++++++------ > 3 files changed, 45 insertions(+), 8 deletions(-) > > diff --git a/drivers/xen/events.c b/drivers/xen/events.c > index 7523719..44490de 100644 > --- a/drivers/xen/events.c > +++ b/drivers/xen/events.c > @@ -1021,7 +1021,7 @@ int bind_ipi_to_irqhandler(enum ipi_vector ipi, > if (irq < 0) > return irq; > > - irqflags |= IRQF_NO_SUSPEND | IRQF_FORCE_RESUME; > + irqflags |= IRQF_NO_SUSPEND | IRQF_FORCE_RESUME | IRQF_EARLY_RESUME; > retval = request_irq(irq, handler, irqflags, devname, dev_id); > if (retval != 0) { > unbind_from_irq(irq); > diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h > index a103732..f51a81b 100644 > --- a/include/linux/interrupt.h > +++ b/include/linux/interrupt.h > @@ -59,6 +59,8 @@ > * IRQF_NO_SUSPEND - Do not disable this IRQ during suspend > * IRQF_FORCE_RESUME - Force enable it on resume even if IRQF_NO_SUSPEND is set > * IRQF_NO_THREAD - Interrupt cannot be threaded > + * IRQF_EARLY_RESUME - Resume IRQ early during syscore instead of at device > + * resume time. > */ > #define IRQF_DISABLED 0x00000020 > #define IRQF_SAMPLE_RANDOM 0x00000040 > @@ -72,6 +74,7 @@ > #define IRQF_NO_SUSPEND 0x00004000 > #define IRQF_FORCE_RESUME 0x00008000 > #define IRQF_NO_THREAD 0x00010000 > +#define IRQF_EARLY_RESUME 0x00020000 > > #define IRQF_TIMER (__IRQF_TIMER | IRQF_NO_SUSPEND | IRQF_NO_THREAD) > > diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c > index f76fc00..86bcd79 100644 > --- a/kernel/irq/pm.c > +++ b/kernel/irq/pm.c > @@ -9,6 +9,7 @@ > #include <linux/irq.h> > #include <linux/module.h> > #include <linux/interrupt.h> > +#include <linux/syscore_ops.h> > > #include "internals.h" > > @@ -39,25 +40,58 @@ void suspend_device_irqs(void) > } > EXPORT_SYMBOL_GPL(suspend_device_irqs); > > -/** > - * resume_device_irqs - enable interrupt lines disabled by suspend_device_irqs() > - * > - * Enable all interrupt lines previously disabled by suspend_device_irqs() that > - * have the IRQS_SUSPENDED flag set. > - */ > -void resume_device_irqs(void) > +static resume_irqs(bool want_early) > { > struct irq_desc *desc; > int irq; > > for_each_irq_desc(irq, desc) { > unsigned long flags; > + bool is_early = desc->action && > + desc->action->flags & IRQF_EARLY_RESUME; > + > + if (is_early != want_early) > + continue; > > raw_spin_lock_irqsave(&desc->lock, flags); > __enable_irq(desc, irq, true); > raw_spin_unlock_irqrestore(&desc->lock, flags); > } > } > + > +/** > + * irq_pm_syscore_ops - enable interrupt lines early > + * > + * Enable all interrupt lines with %IRQF_EARLY_RESUME set. > + */ > +static void irq_pm_syscore_resume(void) > +{ > + resume_irqs(true); > +} > + > +static struct syscore_ops irq_pm_syscore_ops = { > + .resume = irq_pm_syscore_resume, > +}; > + > +static int __init irq_pm_init_ops(void) > +{ > + register_syscore_ops(&irq_pm_syscore_ops); > + return 0; > +} > + > +device_initcall(irq_pm_init_ops); > + > +/** > + * resume_device_irqs - enable interrupt lines disabled by suspend_device_irqs() > + * > + * Enable all non-%IRQF_EARLY_RESUME interrupt lines previously > + * disabled by suspend_device_irqs() that have the IRQS_SUSPENDED flag > + * set as well as those with %IRQF_FORCE_RESUME. > + */ > +void resume_device_irqs(void) > +{ > + resume_irqs(false); > +} > EXPORT_SYMBOL_GPL(resume_device_irqs); > > /**_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Oct-15 21:14 UTC
Re: [Xen-devel] Re: xen: IPI interrupts not resumed early enough on suspend/resume
On Fri, 2011-10-14 at 14:23 +0100, Ian Campbell wrote:> Ping? The original issue has made it into longterm-2.6.32.y and then > onto Debian (bug #644604) so people keep asking me about it...Sorry, the previous version had a warning in it (was missing the return type for resume_irqs), fixed up version follows. I noticed that the backport to longterm 2.6.32 won''t work since syscore isn''t available back then. I think one approach for the backport would be to call irq_pm_syscore_resume() directly from sysdev_resume(), does that sound sensible? Ian. 8<------------------------------>From cac68962d009cd916cc66b4d870630adc49c1e1e Mon Sep 17 00:00:00 2001From: Ian Campbell <ian.campbell@citrix.com> Date: Mon, 3 Oct 2011 15:37:00 +0100 Subject: [PATCH] irq: add IRQF_RESUME_EARLY and resume such IRQs earlier in the process MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds a mechanism to resume selected IRQs during syscore_resume instead of dpm_resume_noirq. Under Xen we need to resume IRQs associated with IPIs early enough that the resched IPI is unmasked and we can therefore schedule ourselves out of the stop_machine where the suspend/resume takes place. This issue was introduced by 676dc3cf5bc3 "xen: Use IRQF_FORCE_RESUME". Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Rafael J. Wysocki <rjw@sisk.pl> Cc: Jeremy Fitzhardinge <jeremy@goop.org> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Cc: xen-devel <xen-devel@lists.xensource.com> Cc: stable@kernel.org (at least to 2.6.32.y) [v2: refactor common code in irq_pm_syscore_resume and resume_device_irqs] [v3: resume_irqs returns void, "fixing warning: return type defaults to ‘int’"] --- drivers/xen/events.c | 2 +- include/linux/interrupt.h | 3 ++ kernel/irq/pm.c | 48 ++++++++++++++++++++++++++++++++++++++------ 3 files changed, 45 insertions(+), 8 deletions(-) diff --git a/drivers/xen/events.c b/drivers/xen/events.c index 7523719..44490de 100644 --- a/drivers/xen/events.c +++ b/drivers/xen/events.c @@ -1021,7 +1021,7 @@ int bind_ipi_to_irqhandler(enum ipi_vector ipi, if (irq < 0) return irq; - irqflags |= IRQF_NO_SUSPEND | IRQF_FORCE_RESUME; + irqflags |= IRQF_NO_SUSPEND | IRQF_FORCE_RESUME | IRQF_EARLY_RESUME; retval = request_irq(irq, handler, irqflags, devname, dev_id); if (retval != 0) { unbind_from_irq(irq); diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h index a103732..f51a81b 100644 --- a/include/linux/interrupt.h +++ b/include/linux/interrupt.h @@ -59,6 +59,8 @@ * IRQF_NO_SUSPEND - Do not disable this IRQ during suspend * IRQF_FORCE_RESUME - Force enable it on resume even if IRQF_NO_SUSPEND is set * IRQF_NO_THREAD - Interrupt cannot be threaded + * IRQF_EARLY_RESUME - Resume IRQ early during syscore instead of at device + * resume time. */ #define IRQF_DISABLED 0x00000020 #define IRQF_SAMPLE_RANDOM 0x00000040 @@ -72,6 +74,7 @@ #define IRQF_NO_SUSPEND 0x00004000 #define IRQF_FORCE_RESUME 0x00008000 #define IRQF_NO_THREAD 0x00010000 +#define IRQF_EARLY_RESUME 0x00020000 #define IRQF_TIMER (__IRQF_TIMER | IRQF_NO_SUSPEND | IRQF_NO_THREAD) diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c index f76fc00..15e53b1 100644 --- a/kernel/irq/pm.c +++ b/kernel/irq/pm.c @@ -9,6 +9,7 @@ #include <linux/irq.h> #include <linux/module.h> #include <linux/interrupt.h> +#include <linux/syscore_ops.h> #include "internals.h" @@ -39,25 +40,58 @@ void suspend_device_irqs(void) } EXPORT_SYMBOL_GPL(suspend_device_irqs); -/** - * resume_device_irqs - enable interrupt lines disabled by suspend_device_irqs() - * - * Enable all interrupt lines previously disabled by suspend_device_irqs() that - * have the IRQS_SUSPENDED flag set. - */ -void resume_device_irqs(void) +static void resume_irqs(bool want_early) { struct irq_desc *desc; int irq; for_each_irq_desc(irq, desc) { unsigned long flags; + bool is_early = desc->action && + desc->action->flags & IRQF_EARLY_RESUME; + + if (is_early != want_early) + continue; raw_spin_lock_irqsave(&desc->lock, flags); __enable_irq(desc, irq, true); raw_spin_unlock_irqrestore(&desc->lock, flags); } } + +/** + * irq_pm_syscore_ops - enable interrupt lines early + * + * Enable all interrupt lines with %IRQF_EARLY_RESUME set. + */ +static void irq_pm_syscore_resume(void) +{ + resume_irqs(true); +} + +static struct syscore_ops irq_pm_syscore_ops = { + .resume = irq_pm_syscore_resume, +}; + +static int __init irq_pm_init_ops(void) +{ + register_syscore_ops(&irq_pm_syscore_ops); + return 0; +} + +device_initcall(irq_pm_init_ops); + +/** + * resume_device_irqs - enable interrupt lines disabled by suspend_device_irqs() + * + * Enable all non-%IRQF_EARLY_RESUME interrupt lines previously + * disabled by suspend_device_irqs() that have the IRQS_SUSPENDED flag + * set as well as those with %IRQF_FORCE_RESUME. + */ +void resume_device_irqs(void) +{ + resume_irqs(false); +} EXPORT_SYMBOL_GPL(resume_device_irqs); /** -- 1.7.2.5 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Thomas Gleixner
2011-Oct-17 08:51 UTC
Re: [Xen-devel] Re: xen: IPI interrupts not resumed early enough on suspend/resume
On Sat, 15 Oct 2011, Ian Campbell wrote:> On Fri, 2011-10-14 at 14:23 +0100, Ian Campbell wrote: > > Ping? The original issue has made it into longterm-2.6.32.y and then > > onto Debian (bug #644604) so people keep asking me about it... > > Sorry, the previous version had a warning in it (was missing the return > type for resume_irqs), fixed up version follows. > > I noticed that the backport to longterm 2.6.32 won''t work since syscore > isn''t available back then. I think one approach for the backport would > be to call irq_pm_syscore_resume() directly from sysdev_resume(), does > that sound sensible?I think so. Can you please add a description (or the five lines patch) for the stable folks to the changelog, so they don''t have to dig around ? Thanks, tglx _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Oct-17 09:25 UTC
Re: [Xen-devel] Re: xen: IPI interrupts not resumed early enough on suspend/resume
On Mon, 2011-10-17 at 09:51 +0100, Thomas Gleixner wrote:> On Sat, 15 Oct 2011, Ian Campbell wrote: > > > On Fri, 2011-10-14 at 14:23 +0100, Ian Campbell wrote: > > > Ping? The original issue has made it into longterm-2.6.32.y and then > > > onto Debian (bug #644604) so people keep asking me about it... > > > > Sorry, the previous version had a warning in it (was missing the return > > type for resume_irqs), fixed up version follows. > > > > I noticed that the backport to longterm 2.6.32 won''t work since syscore > > isn''t available back then. I think one approach for the backport would > > be to call irq_pm_syscore_resume() directly from sysdev_resume(), does > > that sound sensible? > > I think so. Can you please add a description (or the five lines patch) > for the stable folks to the changelog, so they don''t have to dig > around ?If you are happy with the mainline version I''ll send out a backported version to stable@. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Thomas Gleixner
2011-Oct-17 09:39 UTC
Re: [Xen-devel] Re: xen: IPI interrupts not resumed early enough on suspend/resume
On Mon, 17 Oct 2011, Ian Campbell wrote:> On Mon, 2011-10-17 at 09:51 +0100, Thomas Gleixner wrote: > > On Sat, 15 Oct 2011, Ian Campbell wrote: > > > > > On Fri, 2011-10-14 at 14:23 +0100, Ian Campbell wrote: > > > > Ping? The original issue has made it into longterm-2.6.32.y and then > > > > onto Debian (bug #644604) so people keep asking me about it... > > > > > > Sorry, the previous version had a warning in it (was missing the return > > > type for resume_irqs), fixed up version follows. > > > > > > I noticed that the backport to longterm 2.6.32 won''t work since syscore > > > isn''t available back then. I think one approach for the backport would > > > be to call irq_pm_syscore_resume() directly from sysdev_resume(), does > > > that sound sensible? > > > > I think so. Can you please add a description (or the five lines patch) > > for the stable folks to the changelog, so they don''t have to dig > > around ? > > If you are happy with the mainline version I''ll send out a backported > version to stable@.Fine. I queue the other one for 3.2 Thanks, tglx _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Oct-17 13:55 UTC
Re: [Xen-devel] Re: xen: IPI interrupts not resumed early enough on suspend/resume
Hi Greg, On Mon, 2011-10-17 at 09:51 +0100, Thomas Gleixner wrote:> On Sat, 15 Oct 2011, Ian Campbell wrote: > > I noticed that the backport to longterm 2.6.32 won''t work since syscore > > isn''t available back then. I think one approach for the backport would > > be to call irq_pm_syscore_resume() directly from sysdev_resume(), does > > that sound sensible? > > I think so. [...]The following is a backport of 9bab0b7fbaceec47d32db51cd9e59c82fb071f5a (currently in tip.git#irq/core tree and queued for 3.2) to 2.6.32 as described above. 8<------------------------------------------------->From 0e3acef83438e502ca5a1985cc5231c325ca73e2 Mon Sep 17 00:00:00 2001From: Ian Campbell <ian.campbell@citrix.com> Date: Mon, 3 Oct 2011 15:37:00 +0100 Subject: [PATCH] genirq: Add IRQF_RESUME_EARLY and resume such IRQs earlier commit 9bab0b7fbaceec47d32db51cd9e59c82fb071f5a upstream This adds a mechanism to resume selected IRQs during syscore_resume instead of dpm_resume_noirq. Under Xen we need to resume IRQs associated with IPIs early enough that the resched IPI is unmasked and we can therefore schedule ourselves out of the stop_machine where the suspend/resume takes place. This issue was introduced by 676dc3cf5bc3 "xen: Use IRQF_FORCE_RESUME". Back ported to 2.6.32 (which lacks syscore support) by calling the relavant resume function directly from sysdev_resume). Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Cc: Rafael J. Wysocki <rjw@sisk.pl> Cc: Jeremy Fitzhardinge <Jeremy.Fitzhardinge@citrix.com> Cc: xen-devel <xen-devel@lists.xensource.com> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Link: http://lkml.kernel.org/r/1318713254.11016.52.camel@dagon.hellion.org.uk Cc: stable@kernel.org (at least to 2.6.32.y) Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- drivers/base/sys.c | 6 ++++++ drivers/xen/events.c | 2 +- include/linux/interrupt.h | 5 +++++ kernel/irq/pm.c | 35 ++++++++++++++++++++++++++++------- 4 files changed, 40 insertions(+), 8 deletions(-) diff --git a/drivers/base/sys.c b/drivers/base/sys.c index 0d90390..3f202f7 100644 --- a/drivers/base/sys.c +++ b/drivers/base/sys.c @@ -471,6 +471,12 @@ int sysdev_resume(void) { struct sysdev_class *cls; + /* + * Called from syscore in mainline but called directly here + * since syscore does not exist in this tree. + */ + irq_pm_syscore_resume(); + WARN_ONCE(!irqs_disabled(), "Interrupts enabled while resuming system devices\n"); diff --git a/drivers/xen/events.c b/drivers/xen/events.c index 009ca4e..15ed43e 100644 --- a/drivers/xen/events.c +++ b/drivers/xen/events.c @@ -536,7 +536,7 @@ int bind_ipi_to_irqhandler(enum ipi_vector ipi, if (irq < 0) return irq; - irqflags |= IRQF_NO_SUSPEND | IRQF_FORCE_RESUME; + irqflags |= IRQF_NO_SUSPEND | IRQF_FORCE_RESUME | IRQF_EARLY_RESUME; retval = request_irq(irq, handler, irqflags, devname, dev_id); if (retval != 0) { unbind_from_irq(irq); diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h index 4528f29..c7e1aa5 100644 --- a/include/linux/interrupt.h +++ b/include/linux/interrupt.h @@ -54,6 +54,8 @@ * irq line disabled until the threaded handler has been run. * IRQF_NO_SUSPEND - Do not disable this IRQ during suspend * IRQF_FORCE_RESUME - Force enable it on resume even if IRQF_NO_SUSPEND is set + * IRQF_EARLY_RESUME - Resume IRQ early during syscore instead of at device + * resume time. */ #define IRQF_DISABLED 0x00000020 #define IRQF_SAMPLE_RANDOM 0x00000040 @@ -66,6 +68,7 @@ #define IRQF_ONESHOT 0x00002000 #define IRQF_NO_SUSPEND 0x00004000 #define IRQF_FORCE_RESUME 0x00008000 +#define IRQF_EARLY_RESUME 0x00020000 #define IRQF_TIMER (__IRQF_TIMER | IRQF_NO_SUSPEND) @@ -196,6 +199,7 @@ extern void enable_irq(unsigned int irq); #ifdef CONFIG_GENERIC_HARDIRQS extern void suspend_device_irqs(void); extern void resume_device_irqs(void); +extern void irq_pm_syscore_resume(void); #ifdef CONFIG_PM_SLEEP extern int check_wakeup_irqs(void); #else @@ -204,6 +208,7 @@ static inline int check_wakeup_irqs(void) { return 0; } #else static inline void suspend_device_irqs(void) { }; static inline void resume_device_irqs(void) { }; +static inline void irq_pm_syscore_resume(void) { }; static inline int check_wakeup_irqs(void) { return 0; } #endif diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c index 0067abb..b1fc3dd 100644 --- a/kernel/irq/pm.c +++ b/kernel/irq/pm.c @@ -39,25 +39,46 @@ void suspend_device_irqs(void) } EXPORT_SYMBOL_GPL(suspend_device_irqs); -/** - * resume_device_irqs - enable interrupt lines disabled by suspend_device_irqs() - * - * Enable all interrupt lines previously disabled by suspend_device_irqs() that - * have the IRQ_SUSPENDED flag set. - */ -void resume_device_irqs(void) +static void resume_irqs(bool want_early) { struct irq_desc *desc; int irq; for_each_irq_desc(irq, desc) { unsigned long flags; + bool is_early = desc->action && + desc->action->flags & IRQF_EARLY_RESUME; + + if (is_early != want_early) + continue; spin_lock_irqsave(&desc->lock, flags); __enable_irq(desc, irq, true); spin_unlock_irqrestore(&desc->lock, flags); } } + +/** + * irq_pm_syscore_ops - enable interrupt lines early + * + * Enable all interrupt lines with %IRQF_EARLY_RESUME set. + */ +void irq_pm_syscore_resume(void) +{ + resume_irqs(true); +} + +/** + * resume_device_irqs - enable interrupt lines disabled by suspend_device_irqs() + * + * Enable all non-%IRQF_EARLY_RESUME interrupt lines previously + * disabled by suspend_device_irqs() that have the IRQS_SUSPENDED flag + * set as well as those with %IRQF_FORCE_RESUME. + */ +void resume_device_irqs(void) +{ + resume_irqs(false); +} EXPORT_SYMBOL_GPL(resume_device_irqs); /** -- 1.7.2.5 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel