[PATCH] irq: Warn when shared interrupts do not match on NO_SUSPEND

[PATCH] irq: Warn when shared interrupts do not match on NO_SUSPEND

[Peter Zijlstra]

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

Subject: irq: Warn when shared interrupts do not match on NO_SUSPEND

When suspend_device_irqs() iterates all descriptors, its pointless if
one has NO_SUSPEND set while another has not.

Validate on request_irq() that NO_SUSPEND state maches for SHARED
interrupts.

Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
---
 kernel/irq/manage.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 88657d7bc9dd..27a1fe028afb 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1077,9 +1077,12 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
 		 * set the trigger type must match. Also all must
 		 * agree on ONESHOT.
 		 */
+
+#define IRQF_MISMATCH \
+	(IRQF_TRIGGER_MASK | IRQF_ONESHOT | IRQF_NO_SUSPEND)
+
 		if (!((old->flags & new->flags) & IRQF_SHARED) ||
-		    ((old->flags ^ new->flags) & IRQF_TRIGGER_MASK) ||
-		    ((old->flags ^ new->flags) & IRQF_ONESHOT))
+		    ((old->flags ^ new->flags) & IRQF_MISMATCH))
 			goto mismatch;
 
 		/* All handlers must agree on per-cpuness */

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] irq: Warn when shared interrupts do not match on NO_SUSPEND

[Thomas Gleixner]

On Thu, 24 Jul 2014, Peter Zijlstra wrote:

> Subject: irq: Warn when shared interrupts do not match on NO_SUSPEND
> 
> When suspend_device_irqs() iterates all descriptors, its pointless if
> one has NO_SUSPEND set while another has not.
> 
> Validate on request_irq() that NO_SUSPEND state maches for SHARED
> interrupts.

I'm happy to take that patch, but you are aware that it is going to
break a few things?

  25:  ....   PCI-MSI-edge      aerdrv, PCIe PME

Thanks,

	tglx
 
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Peter Zijlstra <peterz@infradead.org>
> ---
>  kernel/irq/manage.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 88657d7bc9dd..27a1fe028afb 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -1077,9 +1077,12 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
>  		 * set the trigger type must match. Also all must
>  		 * agree on ONESHOT.
>  		 */
> +
> +#define IRQF_MISMATCH \
> +	(IRQF_TRIGGER_MASK | IRQF_ONESHOT | IRQF_NO_SUSPEND)
> +
>  		if (!((old->flags & new->flags) & IRQF_SHARED) ||
> -		    ((old->flags ^ new->flags) & IRQF_TRIGGER_MASK) ||
> -		    ((old->flags ^ new->flags) & IRQF_ONESHOT))
> +		    ((old->flags ^ new->flags) & IRQF_MISMATCH))
>  			goto mismatch;
>  
>  		/* All handlers must agree on per-cpuness */
> 

Re: [PATCH] irq: Warn when shared interrupts do not match on NO_SUSPEND

[Peter Zijlstra]

On Thu, Jul 24, 2014 at 03:46:38PM +0200, Thomas Gleixner wrote:
> On Thu, 24 Jul 2014, Peter Zijlstra wrote:
> 
> > Subject: irq: Warn when shared interrupts do not match on NO_SUSPEND
> > 
> > When suspend_device_irqs() iterates all descriptors, its pointless if
> > one has NO_SUSPEND set while another has not.
> > 
> > Validate on request_irq() that NO_SUSPEND state maches for SHARED
> > interrupts.
> 
> I'm happy to take that patch, but you are aware that it is going to
> break a few things?
> 
>   25:  ....   PCI-MSI-edge      aerdrv, PCIe PME

Better an obvious splat than non obvious misbehaviour in my book.

Re: [PATCH] irq: Warn when shared interrupts do not match on NO_SUSPEND

[Rafael J. Wysocki]

On Thursday, July 24, 2014 04:15:25 PM Peter Zijlstra wrote:
> On Thu, Jul 24, 2014 at 03:46:38PM +0200, Thomas Gleixner wrote:
> > On Thu, 24 Jul 2014, Peter Zijlstra wrote:
> > 
> > > Subject: irq: Warn when shared interrupts do not match on NO_SUSPEND
> > > 
> > > When suspend_device_irqs() iterates all descriptors, its pointless if
> > > one has NO_SUSPEND set while another has not.
> > > 
> > > Validate on request_irq() that NO_SUSPEND state maches for SHARED
> > > interrupts.
> > 
> > I'm happy to take that patch, but you are aware that it is going to
> > break a few things?
> > 
> >   25:  ....   PCI-MSI-edge      aerdrv, PCIe PME
> 
> Better an obvious splat than non obvious misbehaviour in my book.

And it will only break things that are already broken.

ACK for the patch.

Rafael

[tip:irq/core] irq: Warn when shared interrupts do not match on NO_SUSPEND

[tip-bot for Peter Zijlstra]

Commit-ID:  4fae4e7624653ef498d0e2a38f00620b9701ab04
Gitweb:     http://git.kernel.org/tip/4fae4e7624653ef498d0e2a38f00620b9701ab04
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Thu, 24 Jul 2014 15:39:21 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 24 Jul 2014 17:32:56 +0200

irq: Warn when shared interrupts do not match on NO_SUSPEND

When suspend_device_irqs() iterates all descriptors, its pointless if
one has NO_SUSPEND set while another has not.

Validate on request_irq() that NO_SUSPEND state maches for SHARED
interrupts.

Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Acked-by: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Link: http://lkml.kernel.org/r/20140724133921.GY6758@twins.programming.kicks-ass.net
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/irq/manage.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 88657d7..27a1fe0 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1077,9 +1077,12 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
 		 * set the trigger type must match. Also all must
 		 * agree on ONESHOT.
 		 */
+
+#define IRQF_MISMATCH \
+	(IRQF_TRIGGER_MASK | IRQF_ONESHOT | IRQF_NO_SUSPEND)
+
 		if (!((old->flags & new->flags) & IRQF_SHARED) ||
-		    ((old->flags ^ new->flags) & IRQF_TRIGGER_MASK) ||
-		    ((old->flags ^ new->flags) & IRQF_ONESHOT))
+		    ((old->flags ^ new->flags) & IRQF_MISMATCH))
 			goto mismatch;
 
 		/* All handlers must agree on per-cpuness */

Re: [tip:irq/core] irq: Warn when shared interrupts do not match on NO_SUSPEND

[Rafael J. Wysocki]

Hi Thomas,

On Thursday, July 24, 2014 08:36:36 AM tip-bot for Peter Zijlstra wrote:
> Commit-ID:  4fae4e7624653ef498d0e2a38f00620b9701ab04
> Gitweb:     http://git.kernel.org/tip/4fae4e7624653ef498d0e2a38f00620b9701ab04
> Author:     Peter Zijlstra <peterz@infradead.org>
> AuthorDate: Thu, 24 Jul 2014 15:39:21 +0200
> Committer:  Thomas Gleixner <tglx@linutronix.de>
> CommitDate: Thu, 24 Jul 2014 17:32:56 +0200
> 
> irq: Warn when shared interrupts do not match on NO_SUSPEND

Please revert this one as it is likely to break existing stuff (I'm mostly
worried about the ACPI interrupt, but also about a number of drivers that have
been using NO_SUSPEND along with SHARED for quite some time).

Rafael