2024-04-24 09:12:44

by David Stevens

[permalink] [raw]
Subject: [PATCH v2] genirq: Skip suspended irqs when restoring affinity

In irq_restore_affinity_of_irq(), skip suspended interrupts and let
resume_device_irqs() deal with restoring them. This ensures that irqs
are not delivered to drivers during the noirq phase of resuming from S3,
after non-boot CPUs are brought back online.

Signed-off-by: David Stevens <[email protected]>
---
kernel/irq/cpuhotplug.c | 11 ++++++++---
kernel/irq/manage.c | 12 ++++++++----
2 files changed, 16 insertions(+), 7 deletions(-)

v1 -> v2:
- Completely defer irq_startup() until __enable_irq().
diff --git a/kernel/irq/cpuhotplug.c b/kernel/irq/cpuhotplug.c
index 1ed2b1739363..43340e0b6df0 100644
--- a/kernel/irq/cpuhotplug.c
+++ b/kernel/irq/cpuhotplug.c
@@ -195,10 +195,15 @@ static void irq_restore_affinity_of_irq(struct irq_desc *desc, unsigned int cpu)
!irq_data_get_irq_chip(data) || !cpumask_test_cpu(cpu, affinity))
return;

- if (irqd_is_managed_and_shutdown(data)) {
- irq_startup(desc, IRQ_RESEND, IRQ_START_COND);
+ /*
+ * Don't restore suspended interrupts here when a system comes back
+ * from S3. They are reenabled via resume_device_irqs().
+ */
+ if (desc->istate & IRQS_SUSPENDED)
return;
- }
+
+ if (irqd_is_managed_and_shutdown(data))
+ irq_startup(desc, IRQ_RESEND, IRQ_START_COND);

/*
* If the interrupt can only be directed to a single target
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 1782f90cd8c6..82124f5bbe03 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -796,10 +796,14 @@ void __enable_irq(struct irq_desc *desc)
irq_settings_set_noprobe(desc);
/*
* Call irq_startup() not irq_enable() here because the
- * interrupt might be marked NOAUTOEN. So irq_startup()
- * needs to be invoked when it gets enabled the first
- * time. If it was already started up, then irq_startup()
- * will invoke irq_enable() under the hood.
+ * interrupt might be marked NOAUTOEN so irq_startup()
+ * needs to be invoked when it gets enabled the first time.
+ * This is also required when __enable_irq() is invoked for
+ * a managed and shutdown interrupt from the S3 resume
+ * path.
+ *
+ * If it was already started up, then irq_startup() will
+ * invoke irq_enable() under the hood.
*/
irq_startup(desc, IRQ_RESEND, IRQ_START_FORCE);
break;

base-commit: e8f897f4afef0031fe618a8e94127a0934896aba
--
2.44.0.769.g3c40516874-goog



Subject: [tip: irq/core] genirq/cpuhotplug: Skip suspended interrupts when restoring affinity

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

Commit-ID: a60dd06af674d3bb76b40da5d722e4a0ecefe650
Gitweb: https://git.kernel.org/tip/a60dd06af674d3bb76b40da5d722e4a0ecefe650
Author: David Stevens <[email protected]>
AuthorDate: Wed, 24 Apr 2024 18:03:41 +09:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Wed, 24 Apr 2024 20:42:57 +02:00

genirq/cpuhotplug: Skip suspended interrupts when restoring affinity

irq_restore_affinity_of_irq() restarts managed interrupts unconditionally
when the first CPU in the affinity mask comes online. That's correct during
normal hotplug operations, but not when resuming from S3 because the
drivers are not resumed yet and interrupt delivery is not expected by them.

Skip the startup of suspended interrupts and let resume_device_irqs() deal
with restoring them. This ensures that irqs are not delivered to drivers
during the noirq phase of resuming from S3, after non-boot CPUs are brought
back online.

Signed-off-by: David Stevens <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
kernel/irq/cpuhotplug.c | 11 ++++++++---
kernel/irq/manage.c | 12 ++++++++----
2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/kernel/irq/cpuhotplug.c b/kernel/irq/cpuhotplug.c
index 1ed2b17..43340e0 100644
--- a/kernel/irq/cpuhotplug.c
+++ b/kernel/irq/cpuhotplug.c
@@ -195,10 +195,15 @@ static void irq_restore_affinity_of_irq(struct irq_desc *desc, unsigned int cpu)
!irq_data_get_irq_chip(data) || !cpumask_test_cpu(cpu, affinity))
return;

- if (irqd_is_managed_and_shutdown(data)) {
- irq_startup(desc, IRQ_RESEND, IRQ_START_COND);
+ /*
+ * Don't restore suspended interrupts here when a system comes back
+ * from S3. They are reenabled via resume_device_irqs().
+ */
+ if (desc->istate & IRQS_SUSPENDED)
return;
- }
+
+ if (irqd_is_managed_and_shutdown(data))
+ irq_startup(desc, IRQ_RESEND, IRQ_START_COND);

/*
* If the interrupt can only be directed to a single target
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index ad3eaf2..29c378d 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -800,10 +800,14 @@ void __enable_irq(struct irq_desc *desc)
irq_settings_set_noprobe(desc);
/*
* Call irq_startup() not irq_enable() here because the
- * interrupt might be marked NOAUTOEN. So irq_startup()
- * needs to be invoked when it gets enabled the first
- * time. If it was already started up, then irq_startup()
- * will invoke irq_enable() under the hood.
+ * interrupt might be marked NOAUTOEN so irq_startup()
+ * needs to be invoked when it gets enabled the first time.
+ * This is also required when __enable_irq() is invoked for
+ * a managed and shutdown interrupt from the S3 resume
+ * path.
+ *
+ * If it was already started up, then irq_startup() will
+ * invoke irq_enable() under the hood.
*/
irq_startup(desc, IRQ_RESEND, IRQ_START_FORCE);
break;