2020-08-11 18:01:12

by Guenter Roeck

Subject: [PATCH] genirq/PM: Always unlock IRQ descriptor in rearm_wake_irq

rearm_wake_irq() does not unlock the irq descriptor if the interrupt
is not suspended or if wakeup is not enabled on it. Fix it.

Fixes: 3a79bc63d9075 ("PCI: irq: Introduce rearm_wake_irq()")
Cc: Rafael J. Wysocki <[email protected]>
Signed-off-by: Guenter Roeck <[email protected]>
---
kernel/irq/pm.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c
index 8f557fa1f4fe..c6c7e187ae74 100644
--- a/kernel/irq/pm.c
+++ b/kernel/irq/pm.c
@@ -185,14 +185,18 @@ void rearm_wake_irq(unsigned int irq)
unsigned long flags;
struct irq_desc *desc = irq_get_desc_buslock(irq, &flags, IRQ_GET_DESC_CHECK_GLOBAL);

- if (!desc || !(desc->istate & IRQS_SUSPENDED) ||
- !irqd_is_wakeup_set(&desc->irq_data))
+ if (!desc)
return;

+ if (!(desc->istate & IRQS_SUSPENDED) ||
+ !irqd_is_wakeup_set(&desc->irq_data))
+ goto unlock;
+
desc->istate &= ~IRQS_SUSPENDED;
irqd_set(&desc->irq_data, IRQD_WAKEUP_ARMED);
__enable_irq(desc);

+unlock:
irq_put_desc_busunlock(desc, flags);
}

--
2.17.1

2020-08-11 19:25:24

by Wysocki, Rafael J

Subject: Re: [PATCH] genirq/PM: Always unlock IRQ descriptor in rearm_wake_irq

On 8/11/2020 8:00 PM, Guenter Roeck wrote:
> rearm_wake_irq() does not unlock the irq descriptor if the interrupt
> is not suspended or if wakeup is not enabled on it. Fix it.
>
> Fixes: 3a79bc63d9075 ("PCI: irq: Introduce rearm_wake_irq()")
> Cc: Rafael J. Wysocki <[email protected]>
> Signed-off-by: Guenter Roeck <[email protected]>

Acked-by: Rafael J. Wysocki <[email protected]>

And it needs to go to -stable (even though the bug is latent now,
because this function is called for suspended IRQs only AFAICS).

Or I can apply this as the mistake was in my commit.  Please let me know
what you prefer.


> ---
> kernel/irq/pm.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c
> index 8f557fa1f4fe..c6c7e187ae74 100644
> --- a/kernel/irq/pm.c
> +++ b/kernel/irq/pm.c
> @@ -185,14 +185,18 @@ void rearm_wake_irq(unsigned int irq)
> unsigned long flags;
> struct irq_desc *desc = irq_get_desc_buslock(irq, &flags, IRQ_GET_DESC_CHECK_GLOBAL);
>
> - if (!desc || !(desc->istate & IRQS_SUSPENDED) ||
> - !irqd_is_wakeup_set(&desc->irq_data))
> + if (!desc)
> return;
>
> + if (!(desc->istate & IRQS_SUSPENDED) ||
> + !irqd_is_wakeup_set(&desc->irq_data))
> + goto unlock;
> +
> desc->istate &= ~IRQS_SUSPENDED;
> irqd_set(&desc->irq_data, IRQD_WAKEUP_ARMED);
> __enable_irq(desc);
>
> +unlock:
> irq_put_desc_busunlock(desc, flags);
> }
>

2020-08-12 09:14:43

by tip-bot2 for Jacob Pan

Subject: [tip: irq/urgent] genirq/PM: Always unlock IRQ descriptor in rearm_wake_irq()

The following commit has been merged into the irq/urgent branch of tip:

Commit-ID: e27b1636e9337d1a1d174b191e53d0f86421a822
Gitweb: https://git.kernel.org/tip/e27b1636e9337d1a1d174b191e53d0f86421a822
Author: Guenter Roeck <[email protected]>
AuthorDate: Tue, 11 Aug 2020 11:00:01 -07:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Wed, 12 Aug 2020 11:04:05 +02:00

genirq/PM: Always unlock IRQ descriptor in rearm_wake_irq()

rearm_wake_irq() does not unlock the irq descriptor if the interrupt
is not suspended or if wakeup is not enabled on it.

Restucture the exit conditions so the unlock is always ensured.

Fixes: 3a79bc63d9075 ("PCI: irq: Introduce rearm_wake_irq()")
Signed-off-by: Guenter Roeck <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Acked-by: Rafael J. Wysocki <[email protected]>
Cc: [email protected]
Link: https://lore.kernel.org/r/[email protected]

---
kernel/irq/pm.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c
index 8f557fa..c6c7e18 100644
--- a/kernel/irq/pm.c
+++ b/kernel/irq/pm.c
@@ -185,14 +185,18 @@ void rearm_wake_irq(unsigned int irq)
unsigned long flags;
struct irq_desc *desc = irq_get_desc_buslock(irq, &flags, IRQ_GET_DESC_CHECK_GLOBAL);

- if (!desc || !(desc->istate & IRQS_SUSPENDED) ||
- !irqd_is_wakeup_set(&desc->irq_data))
+ if (!desc)
return;

+ if (!(desc->istate & IRQS_SUSPENDED) ||
+ !irqd_is_wakeup_set(&desc->irq_data))
+ goto unlock;
+
desc->istate &= ~IRQS_SUSPENDED;
irqd_set(&desc->irq_data, IRQD_WAKEUP_ARMED);
__enable_irq(desc);

+unlock:
irq_put_desc_busunlock(desc, flags);
}