[PATCH 2/3] add IRQF_NO_SUSPEND for 'bind_ipi_to_irqhandler'

[PATCH 2/3] add IRQF_NO_SUSPEND for 'bind_ipi_to_irqhandler'

[Guanqun Lu]

This function exports the ipi functionality to kernel,
and the corresponding irq should not be disabled during
host S3 suspend.

Signed-off-by: Guanqun Lu <guanqun.lu@intel.com>
---
 drivers/xen/events.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 97f4b39..58606d5 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -532,6 +532,7 @@ int bind_ipi_to_irqhandler(enum ipi_vector ipi,
 	if (irq < 0)
 		return irq;
 
+	irqflags |= IRQF_NO_SUSPEND;
 	retval = request_irq(irq, handler, irqflags, devname, dev_id);
 	if (retval != 0) {
 		unbind_from_irq(irq);
-- 
1.6.1.rc3

[PATCH 1/3] add flag IRQF_NO_SUSPEND in 'struct irqaction'

[Guanqun Lu]

We currently only bypass IRQF_TIMER in '__disable_irq',
but Xen specific IRQs should not be disabled either.
This commit adds a new flag to accompolish this goal
without being mixed up with IRQF_TIMER flag.

Signed-off-by: Guanqun Lu <guanqun.lu@intel.com>
---
 include/linux/interrupt.h |    1 +
 kernel/irq/manage.c       |    3 ++-
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 8a9613d..8ad2b6f 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -58,6 +58,7 @@
 #define IRQF_PERCPU		0x00000400
 #define IRQF_NOBALANCING	0x00000800
 #define IRQF_IRQPOLL		0x00001000
+#define IRQF_NO_SUSPEND		0x00002000
 
 typedef irqreturn_t (*irq_handler_t)(int, void *);
 
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 1516ab7..f814678 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -165,7 +165,8 @@ static inline int setup_affinity(unsigned int irq, struct irq_desc *desc)
 void __disable_irq(struct irq_desc *desc, unsigned int irq, bool suspend)
 {
 	if (suspend) {
-		if (!desc->action || (desc->action->flags & IRQF_TIMER))
+		if (!desc->action ||
+		    (desc->action->flags & (IRQF_TIMER | IRQF_NO_SUSPEND)))
 			return;
 		desc->status |= IRQ_SUSPENDED;
 	}
-- 
1.6.1.rc3

[PATCH 3/3] add IRQF_TIMER to Xen timer.

[Guanqun Lu]

This flag tells the kernel that this irq should not be disabled
during host S3 suspend.

Signed-off-by: Guanqun Lu <guanqun.lu@intel.com>
---
 arch/x86/xen/time.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index 14f2406..ae42832 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -429,7 +429,7 @@ void xen_setup_timer(int cpu)
 		name = "<timer kasprintf failed>";
 
 	irq = bind_virq_to_irqhandler(VIRQ_TIMER, cpu, xen_timer_interrupt,
-				      IRQF_DISABLED|IRQF_PERCPU|IRQF_NOBALANCING,
+				      IRQF_DISABLED|IRQF_PERCPU|IRQF_NOBALANCING|IRQF_TIMER,
 				      name, NULL);
 
 	evt = &per_cpu(xen_clock_events, cpu);
-- 
1.6.1.rc3

RE: [PATCH 1/3] add flag IRQF_NO_SUSPEND in 'struct irqaction'

[Yu, Ke]

Hi Jeremy,

Forget to mention, this series of patches are targeting for pv_ops dom0 host S3.

We have tried the host S3 branch (xen-tip/dom0/acpi) and find host S3 will hang at disable_nonboot_cpus. the root cause is that suspend_device_irqs () unfortunately will disable the irq of XEN_CALL_FUNCTION_SINGLE_VECTOR IPI and xen_timer_interrupt VIRQ, which however are required by disable_nonboot_cpus. To fix this issue, flag IRQF_NO_SUSPEND is added to Xen IPI and IRQF_TIMER is added to xen_timer_interrupt VIRQ, to notify suspend_device_irqs not to disable these irqs when suspend.

After applying this series of patches to xen-tip/dom0/acpi branch, and rebasing xen-tip/dom0/acpi to xen-tip/master, the pv_ops dom0 host S3 is working in our side.

Best Regards
Ke

>-----Original Message-----
>From: xen-devel-bounces@lists.xensource.com
>[mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Guanqun Lu
>Sent: Tuesday, July 14, 2009 5:44 PM
>To: xen-devel@lists.xensource.com
>Cc: Lu, Guanqun
>Subject: [Xen-devel] [PATCH 1/3] add flag IRQF_NO_SUSPEND in 'struct
>irqaction'
>
>We currently only bypass IRQF_TIMER in '__disable_irq',
>but Xen specific IRQs should not be disabled either.
>This commit adds a new flag to accompolish this goal
>without being mixed up with IRQF_TIMER flag.
>
>Signed-off-by: Guanqun Lu <guanqun.lu@intel.com>
>---
> include/linux/interrupt.h |    1 +
> kernel/irq/manage.c       |    3 ++-
> 2 files changed, 3 insertions(+), 1 deletions(-)
>
>diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
>index 8a9613d..8ad2b6f 100644
>--- a/include/linux/interrupt.h
>+++ b/include/linux/interrupt.h
>@@ -58,6 +58,7 @@
> #define IRQF_PERCPU		0x00000400
> #define IRQF_NOBALANCING	0x00000800
> #define IRQF_IRQPOLL		0x00001000
>+#define IRQF_NO_SUSPEND		0x00002000
>
> typedef irqreturn_t (*irq_handler_t)(int, void *);
>
>diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
>index 1516ab7..f814678 100644
>--- a/kernel/irq/manage.c
>+++ b/kernel/irq/manage.c
>@@ -165,7 +165,8 @@ static inline int setup_affinity(unsigned int irq, struct
>irq_desc *desc)
> void __disable_irq(struct irq_desc *desc, unsigned int irq, bool suspend)
> {
> 	if (suspend) {
>-		if (!desc->action || (desc->action->flags & IRQF_TIMER))
>+		if (!desc->action ||
>+		    (desc->action->flags & (IRQF_TIMER | IRQF_NO_SUSPEND)))
> 			return;
> 		desc->status |= IRQ_SUSPENDED;
> 	}
>--
>1.6.1.rc3
>
>
>_______________________________________________
>Xen-devel mailing list
>Xen-devel@lists.xensource.com
>http://lists.xensource.com/xen-devel

Re: [PATCH 1/3] add flag IRQF_NO_SUSPEND in 'struct irqaction'

[Jeremy Fitzhardinge]

On 07/13/09 01:46, Yu, Ke wrote:
> Hi Jeremy,
>
> Forget to mention, this series of patches are targeting for pv_ops dom0 host S3.
>
> We have tried the host S3 branch (xen-tip/dom0/acpi) and find host S3 will hang at disable_nonboot_cpus. the root cause is that suspend_device_irqs () unfortunately will disable the irq of XEN_CALL_FUNCTION_SINGLE_VECTOR IPI and xen_timer_interrupt VIRQ, which however are required by disable_nonboot_cpus. To fix this issue, flag IRQF_NO_SUSPEND is added to Xen IPI and IRQF_TIMER is added to xen_timer_interrupt VIRQ, to notify suspend_device_irqs not to disable these irqs when suspend.
>
> After applying this series of patches to xen-tip/dom0/acpi branch, and rebasing xen-tip/dom0/acpi to xen-tip/master, the pv_ops dom0 host S3 is working in our side.
>   

Thanks for debugging that.  Could you send me a patch?  Is the
IRQF_NO_SUSPEND flag something you've had to add?

Thanks,

    J

RE: [PATCH 1/3] add flag IRQF_NO_SUSPEND in 'struct irqaction'

[Yu, Ke]

>-----Original Message-----
>From: Jeremy Fitzhardinge [mailto:jeremy@goop.org]
>Sent: Wednesday, July 15, 2009 4:01 AM
>To: Yu, Ke
>Cc: xen-devel@lists.xensource.com; Lu, Guanqun
>Subject: Re: [Xen-devel] [PATCH 1/3] add flag IRQF_NO_SUSPEND in 'struct
>irqaction'
>
>On 07/13/09 01:46, Yu, Ke wrote:
>> Hi Jeremy,
>>
>> Forget to mention, this series of patches are targeting for pv_ops dom0
>host S3.
>>
>> We have tried the host S3 branch (xen-tip/dom0/acpi) and find host S3 will
>hang at disable_nonboot_cpus. the root cause is that suspend_device_irqs ()
>unfortunately will disable the irq of XEN_CALL_FUNCTION_SINGLE_VECTOR
>IPI and xen_timer_interrupt VIRQ, which however are required by
>disable_nonboot_cpus. To fix this issue, flag IRQF_NO_SUSPEND is added to
>Xen IPI and IRQF_TIMER is added to xen_timer_interrupt VIRQ, to notify
>suspend_device_irqs not to disable these irqs when suspend.
>>
>> After applying this series of patches to xen-tip/dom0/acpi branch, and
>rebasing xen-tip/dom0/acpi to xen-tip/master, the pv_ops dom0 host S3 is
>working in our side.
>>
>
>Thanks for debugging that.  Could you send me a patch?  Is the
>IRQF_NO_SUSPEND flag something you've had to add?
>
>Thanks,
>
>    J

The three patches Guanqun posted has all the fixes I mentioned. I just add some background here for better understanding :)

For the flag IRQF_NO_SUSPEND, indeed, we can use IRQF_TIMER to achieve the same functionality, but this does not match the semantic of IRQF_TIMER, since it is used for timer, while xen IPI obviously is not. There is similar discussion in LKML thread http://marc.info/?l=linux-kernel&m=123561730928222&w=2 where Ingo suggested to add new flag IRQF_NO_SUSPEND instead of using the existing IRQF_TIMER. Although the IRQF_NO_SUSPEND is not added finally due to that issue is fixed in another way. This thread implies IRQF_NO_SUSPEND is more clean in this case.

Best Regards
Ke

Re: [PATCH 1/3] add flag IRQF_NO_SUSPEND in 'struct irqaction'

[Rafael J. Wysocki]

On Friday 17 July 2009, Guanqun Lu wrote:
> We currently only bypass IRQF_TIMER in '__disable_irq',
> but Xen specific IRQs should not be disabled either.
> This commit adds a new flag to accompolish this goal
> without being mixed up with IRQF_TIMER flag.

For some obscure reasons [2/3] didn't reach my inbox.  The other 2 patches
look fine to me.

Best,
Rafael


> Signed-off-by: Guanqun Lu <guanqun.lu@intel.com>
> ---
>  include/linux/interrupt.h |    1 +
>  kernel/irq/manage.c       |    3 ++-
>  2 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index 2721f07..99264c3 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -58,6 +58,7 @@
>  #define IRQF_PERCPU		0x00000400
>  #define IRQF_NOBALANCING	0x00000800
>  #define IRQF_IRQPOLL		0x00001000
> +#define IRQF_NO_SUSPEND		0x00002000
>  
>  /*
>   * Bits used by threaded handlers:
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 50da676..3dc4e74 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -192,7 +192,8 @@ static inline int setup_affinity(unsigned int irq, struct irq_desc *desc)
>  void __disable_irq(struct irq_desc *desc, unsigned int irq, bool suspend)
>  {
>  	if (suspend) {
> -		if (!desc->action || (desc->action->flags & IRQF_TIMER))
> +		if (!desc->action ||
> +		    (desc->action->flags & (IRQF_TIMER | IRQF_NO_SUSPEND)))
>  			return;
>  		desc->status |= IRQ_SUSPENDED;
>  	}

RE: [PATCH 1/3] add flag IRQF_NO_SUSPEND in 'struct irqaction'

[Lu, Guanqun]

[-- Attachment #1: Type: text/plain, Size: 2012 bytes --]

>-----Original Message-----
>From: Rafael J. Wysocki [mailto:rjw@sisk.pl]
>Sent: Friday, July 17, 2009 9:52 AM
>To: Lu, Guanqun
>Cc: linux-kernel@vger.kernel.org; mingo@elte.hu; jeremy@goop.org
>Subject: Re: [PATCH 1/3] add flag IRQF_NO_SUSPEND in 'struct irqaction'
>
>On Friday 17 July 2009, Guanqun Lu wrote:
>> We currently only bypass IRQF_TIMER in '__disable_irq',
>> but Xen specific IRQs should not be disabled either.
>> This commit adds a new flag to accompolish this goal
>> without being mixed up with IRQF_TIMER flag.
>
>For some obscure reasons [2/3] didn't reach my inbox.  The other 2 patches
>look fine to me.

I'm sending patch [2/3] via attachment in case it's lost accidentally by git-send-email again.
Sorry for the inconvenience it may cause.

Guanqun

>
>Best,
>Rafael
>
>
>> Signed-off-by: Guanqun Lu <guanqun.lu@intel.com>
>> ---
>>  include/linux/interrupt.h |    1 +
>>  kernel/irq/manage.c       |    3 ++-
>>  2 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
>> index 2721f07..99264c3 100644
>> --- a/include/linux/interrupt.h
>> +++ b/include/linux/interrupt.h
>> @@ -58,6 +58,7 @@
>>  #define IRQF_PERCPU		0x00000400
>>  #define IRQF_NOBALANCING	0x00000800
>>  #define IRQF_IRQPOLL		0x00001000
>> +#define IRQF_NO_SUSPEND		0x00002000
>>
>>  /*
>>   * Bits used by threaded handlers:
>> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
>> index 50da676..3dc4e74 100644
>> --- a/kernel/irq/manage.c
>> +++ b/kernel/irq/manage.c
>> @@ -192,7 +192,8 @@ static inline int setup_affinity(unsigned int irq, struct irq_desc
>*desc)
>>  void __disable_irq(struct irq_desc *desc, unsigned int irq, bool suspend)
>>  {
>>  	if (suspend) {
>> -		if (!desc->action || (desc->action->flags & IRQF_TIMER))
>> +		if (!desc->action ||
>> +		    (desc->action->flags & (IRQF_TIMER | IRQF_NO_SUSPEND)))
>>  			return;
>>  		desc->status |= IRQ_SUSPENDED;
>>  	}

[-- Attachment #2: 0002-add-IRQF_NO_SUSPEND-for-bind_ipi_to_irqhandler.patch --]
[-- Type: application/octet-stream, Size: 882 bytes --]

From fcd8a32c16d509270ef90069b92dadc506ce49ea Mon Sep 17 00:00:00 2001
From: Guanqun Lu <guanqun.lu@intel.com>
Date: Tue, 14 Jul 2009 05:15:12 -0400
Subject: [PATCH 2/3] add IRQF_NO_SUSPEND for 'bind_ipi_to_irqhandler'

This function exports the ipi functionality to kernel,
and the corresponding irq should not be disabled during
host S3 suspend.

Signed-off-by: Guanqun Lu <guanqun.lu@intel.com>
---
 drivers/xen/events.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index abad71b..f0647bd 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -532,6 +532,7 @@ int bind_ipi_to_irqhandler(enum ipi_vector ipi,
 	if (irq < 0)
 		return irq;
 
+	irqflags |= IRQF_NO_SUSPEND;
 	retval = request_irq(irq, handler, irqflags, devname, dev_id);
 	if (retval != 0) {
 		unbind_from_irq(irq);
-- 
1.6.1.rc3

Re: [PATCH 1/3] add flag IRQF_NO_SUSPEND in 'struct irqaction'

[Rafael J. Wysocki]

On Friday 17 July 2009, Lu, Guanqun wrote:
> >-----Original Message-----
> >From: Rafael J. Wysocki [mailto:rjw@sisk.pl]
> >Sent: Friday, July 17, 2009 9:52 AM
> >To: Lu, Guanqun
> >Cc: linux-kernel@vger.kernel.org; mingo@elte.hu; jeremy@goop.org
> >Subject: Re: [PATCH 1/3] add flag IRQF_NO_SUSPEND in 'struct irqaction'
> >
> >On Friday 17 July 2009, Guanqun Lu wrote:
> >> We currently only bypass IRQF_TIMER in '__disable_irq',
> >> but Xen specific IRQs should not be disabled either.
> >> This commit adds a new flag to accompolish this goal
> >> without being mixed up with IRQF_TIMER flag.
> >
> >For some obscure reasons [2/3] didn't reach my inbox.  The other 2 patches
> >look fine to me.
> 
> I'm sending patch [2/3] via attachment in case it's lost accidentally by git-send-email again.
> Sorry for the inconvenience it may cause.

Thanks, patch looks OK to me.

Best,
Rafael


> >> Signed-off-by: Guanqun Lu <guanqun.lu@intel.com>
> >> ---
> >>  include/linux/interrupt.h |    1 +
> >>  kernel/irq/manage.c       |    3 ++-
> >>  2 files changed, 3 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> >> index 2721f07..99264c3 100644
> >> --- a/include/linux/interrupt.h
> >> +++ b/include/linux/interrupt.h
> >> @@ -58,6 +58,7 @@
> >>  #define IRQF_PERCPU		0x00000400
> >>  #define IRQF_NOBALANCING	0x00000800
> >>  #define IRQF_IRQPOLL		0x00001000
> >> +#define IRQF_NO_SUSPEND		0x00002000
> >>
> >>  /*
> >>   * Bits used by threaded handlers:
> >> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> >> index 50da676..3dc4e74 100644
> >> --- a/kernel/irq/manage.c
> >> +++ b/kernel/irq/manage.c
> >> @@ -192,7 +192,8 @@ static inline int setup_affinity(unsigned int irq, struct irq_desc
> >*desc)
> >>  void __disable_irq(struct irq_desc *desc, unsigned int irq, bool suspend)
> >>  {
> >>  	if (suspend) {
> >> -		if (!desc->action || (desc->action->flags & IRQF_TIMER))
> >> +		if (!desc->action ||
> >> +		    (desc->action->flags & (IRQF_TIMER | IRQF_NO_SUSPEND)))
> >>  			return;
> >>  		desc->status |= IRQ_SUSPENDED;
> >>  	}

Re: [PATCH 1/3] add flag IRQF_NO_SUSPEND in 'struct irqaction'

[Thomas Gleixner]

On Fri, 17 Jul 2009, Rafael J. Wysocki wrote:

> On Friday 17 July 2009, Guanqun Lu wrote:
> > We currently only bypass IRQF_TIMER in '__disable_irq',
> > but Xen specific IRQs should not be disabled either.
> > This commit adds a new flag to accompolish this goal
> > without being mixed up with IRQF_TIMER flag.
> 
> For some obscure reasons [2/3] didn't reach my inbox.  The other 2 patches
> look fine to me.

I do not receive the patches at all. Can you please resend and cc me ?

Thanks,

	tglx

RE: [PATCH 1/3] add flag IRQF_NO_SUSPEND in 'struct irqaction'

[Lu, Guanqun]

>-----Original Message-----
>From: Thomas Gleixner [mailto:tglx@linutronix.de]
>Sent: Friday, July 17, 2009 10:18 PM
>To: Rafael J. Wysocki
>Cc: Lu, Guanqun; linux-kernel@vger.kernel.org; mingo@elte.hu; jeremy@goop.org
>Subject: Re: [PATCH 1/3] add flag IRQF_NO_SUSPEND in 'struct irqaction'
>
>On Fri, 17 Jul 2009, Rafael J. Wysocki wrote:
>
>> On Friday 17 July 2009, Guanqun Lu wrote:
>> > We currently only bypass IRQF_TIMER in '__disable_irq',
>> > but Xen specific IRQs should not be disabled either.
>> > This commit adds a new flag to accompolish this goal
>> > without being mixed up with IRQF_TIMER flag.
>>
>> For some obscure reasons [2/3] didn't reach my inbox.  The other 2 patches
>> look fine to me.
>
>I do not receive the patches at all. Can you please resend and cc me ?

Resent now, let me know it doesn't get to your inbox again.

Guanqun

>
>Thanks,
>
>	tglx