xen: IPI interrupts not resumed early enough on suspend/resume

xen: IPI interrupts not resumed early enough on suspend/resume

[Ian Campbell]

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.

Re: xen: IPI interrupts not resumed early enough on suspend/resume

[Thomas Gleixner]

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 ?

Thanks,

	tglx

Re: xen: IPI interrupts not resumed early enough on suspend/resume

[Thomas Gleixner]

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 ?

Thanks,

	tglx

Re: xen: IPI interrupts not resumed early enough on suspend/resume

[Ian Campbell]

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

Re: xen: IPI interrupts not resumed early enough on suspend/resume

[Thomas Gleixner]

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.
> 
> OK

I guess we should go down that road then.

Thanks,

	tglx

Re: xen: IPI interrupts not resumed early enough on suspend/resume

[Thomas Gleixner]

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.
> 
> OK

I guess we should go down that road then.

Thanks,

	tglx

Re: xen: IPI interrupts not resumed early enough on suspend/resume

[Ian Campbell]

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 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 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

Re: xen: IPI interrupts not resumed early enough on suspend/resume

[Ian Campbell]

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 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 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

Re: xen: IPI interrupts not resumed early enough on suspend/resume

[Thomas Gleixner]

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

Re: xen: IPI interrupts not resumed early enough on suspend/resume

[Ian Campbell]

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 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);
 
 /**
-- 
1.7.2.5

Re: xen: IPI interrupts not resumed early enough on suspend/resume

[Ian Campbell]

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);
>  
>  /**

Re: [Xen-devel] Re: xen: IPI interrupts not resumed early enough on suspend/resume

[Ian Campbell]

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 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
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

Re: [Xen-devel] Re: xen: IPI interrupts not resumed early enough on suspend/resume

[Ian Campbell]

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 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
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

Re: [Xen-devel] Re: xen: IPI interrupts not resumed early enough on suspend/resume

[Thomas Gleixner]

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

Re: [Xen-devel] Re: xen: IPI interrupts not resumed early enough on suspend/resume

[Ian Campbell]

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.

Re: [Xen-devel] Re: xen: IPI interrupts not resumed early enough on suspend/resume

[Thomas Gleixner]

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

Re: [Xen-devel] Re: xen: IPI interrupts not resumed early enough on suspend/resume

[Ian Campbell]

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 2001
From: 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

Re: Re: xen: IPI interrupts not resumed early enough on suspend/resume

[Ian Campbell]

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 2001
From: 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

Re: xen: IPI interrupts not resumed early enough on suspend/resume

[Rafael J. Wysocki]

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

Re: xen: IPI interrupts not resumed early enough on suspend/resume

[Thomas Gleixner]

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.