* [PATCH] PCI: iproc-msi: Bound events handled per IRQ
@ 2026-10-16 12:18 Eugen Hristev
2026-10-16 12:31 ` sashiko-bot
2026-10-16 18:21 ` Bjorn Helgaas
0 siblings, 2 replies; 4+ messages in thread
From: Eugen Hristev @ 2026-10-16 12:18 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Ray Jui,
Scott Branden, Broadcom internal kernel review list
Cc: linux-pci, linux-arm-kernel, linux-kernel, Eugen Hristev
Limit each iProc MSI parent IRQ to 16 event-queue entries.
A continuously fed queue could otherwise keep the chained handler running
and starve other work. The budget still drains pending events and lets the
next parent interrupt continue processing, improving fairness and bounding
interrupt-handler latency.
Without this starvation avoidance, one CPU can stall indefinitely
in the loop and not serve other interrupts, timers, etc.
While testing it with an Ethernet NIC, when too many MSI interrupts
come in, CPU might get starved:
Connecting to host 192.168.30.1, port 5201
[ 5] local 192.168.30.2 port 52586 connected to 192.168.30.1 port 5201
[ ID] Interval Transfer Bitrate Retr Cwnd
[ 5] 0.00-1.00 sec 240 MBytes 2.01 Gbits/sec 2 334 KBytes
[ 5] 1.00-2.00 sec 63.2 MBytes 531 Mbits/sec 1 1.41 KBytes
[ 5] 2.00-3.00 sec 0.00 Bytes 0.00 bits/sec 0 1.41 KBytes
[ 5] 3.00-4.00 sec 0.00 Bytes 0.00 bits/sec 0 1.41 KBytes
[ 5] 4.00-5.00 sec 0.00 Bytes 0.00 bits/sec 0 1.41 KBytes
[ 5] 5.00-6.00 sec 0.00 Bytes 0.00 bits/sec 0 1.41 KBytes
[ 5] 6.00-7.00 sec 0.00 Bytes 0.00 bits/sec 0 1.41 KBytes
[ 5] 7.00-8.00 sec 0.00 Bytes 0.00 bits/sec 0 1.41 KBytes
[ 5] 8.00-9.00 sec 0.00 Bytes 0.00 bits/sec 0 1.41 KBytes
[ 177.568497] rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
[ 177.568913] rcu: 0-...!: (1 GPs behind) idle=28ac/1/0x4000000000000002 softirq=28922/28930 fqs=1
[ 177.569457] rcu: (detected by 1, t=5256 jiffies, g=21629, q=819 ncpus=4)
[ 177.569870] Sending NMI from CPU 1 to CPUs 0:
With this bounded approach, IRQ handler will complete and continue
to serve MSI events on the next handler invocation.
Assisted-by: Github Copilot
Signed-off-by: Eugen Hristev <ehristev@kernel.org>
---
drivers/pci/controller/pcie-iproc-msi.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/pcie-iproc-msi.c b/drivers/pci/controller/pcie-iproc-msi.c
index 9ba242ab9596..6649807822d2 100644
--- a/drivers/pci/controller/pcie-iproc-msi.c
+++ b/drivers/pci/controller/pcie-iproc-msi.c
@@ -29,6 +29,9 @@
/* Number of entries in each event queue */
#define EQ_LEN 64
+/* Bound parent interrupt handler such that a queue cannot starve CPU. */
+#define IPROC_MSI_MAX_EVENTS_PER_IRQ 16
+
/* Size of each event queue memory region */
#define EQ_MEM_REGION_SIZE SZ_4K
@@ -326,6 +329,7 @@ static void iproc_msi_handler(struct irq_desc *desc)
struct iproc_msi_grp *grp;
struct iproc_msi *msi;
u32 eq, head, tail, nr_events;
+ unsigned int event_budget = IPROC_MSI_MAX_EVENTS_PER_IRQ;
unsigned long hwirq;
chained_irq_enter(chip, desc);
@@ -360,9 +364,11 @@ static void iproc_msi_handler(struct irq_desc *desc)
break;
/* process all outstanding events */
- while (nr_events--) {
+ while (nr_events && event_budget) {
hwirq = decode_msi_hwirq(msi, eq, head);
generic_handle_domain_irq(msi->inner_domain, hwirq);
+ nr_events--;
+ event_budget--;
head++;
head %= EQ_LEN;
@@ -373,6 +379,8 @@ static void iproc_msi_handler(struct irq_desc *desc)
* head pointer.
*/
iproc_msi_write_reg(msi, IPROC_MSI_EQ_HEAD, eq, head);
+ if (!event_budget)
+ break;
/*
* Now go read the tail pointer again to see if there are new
---
base-commit: b810335a0699d1810683796b0e3ffe8827ddfe6c
change-id: 20261016-iproc-irq-9c58cfe39158
Best regards,
--
Eugen Hristev <ehristev@kernel.org>
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] PCI: iproc-msi: Bound events handled per IRQ
2026-10-16 12:18 [PATCH] PCI: iproc-msi: Bound events handled per IRQ Eugen Hristev
@ 2026-10-16 12:31 ` sashiko-bot
2026-10-16 18:21 ` Bjorn Helgaas
1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-10-16 12:31 UTC (permalink / raw)
To: Eugen Hristev; +Cc: linux-pci
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pci: iproc-msi: missing dma_rmb() after TAIL register read
--
commit 8736e22d751e9208bad7bb267a7f08a76f8d4f77
Author: Eugen Hristev <ehristev@kernel.org>
PCI: iproc-msi: Bound events handled per IRQ
Limit each iProc MSI parent IRQ to 16 event-queue entries.
A continuously fed queue could otherwise keep the chained handler running
and starve other work. The budget still drains pending events and lets the
next parent interrupt continue processing, improving fairness and bounding
interrupt-handler latency.
> diff --git a/drivers/pci/controller/pcie-iproc-msi.c b/drivers/pci/controller/pcie-iproc-msi.c
> index 9ba242ab95967..6649807822d27 100644
> --- a/drivers/pci/controller/pcie-iproc-msi.c
> +++ b/drivers/pci/controller/pcie-iproc-msi.c
[ ... ]
> @@ -360,9 +364,11 @@ static void iproc_msi_handler(struct irq_desc *desc)
> break;
>
> /* process all outstanding events */
> - while (nr_events--) {
> + while (nr_events && event_budget) {
> hwirq = decode_msi_hwirq(msi, eq, head);
[Severity: High]
Does this sequence lack a necessary dma_rmb() before reading the event queue
in decode_msi_hwirq()?
Because iproc_msi_read_reg() uses readl_relaxed() to read the TAIL register,
weakly-ordered architectures could speculatively execute the DMA memory read
from msi->eq_cpu (inside decode_msi_hwirq()) before the MMIO read of the TAIL
register completes.
Could this result in reading stale hardware IRQ numbers from the event queue
on subsequent iterations of the polling loop, leading to missed or spurious
interrupts?
> generic_handle_domain_irq(msi->inner_domain, hwirq);
> + nr_events--;
> + event_budget--;
>
> head++;
> head %= EQ_LEN;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-iproc-irq-v1-1-ebaaf027f29e@kernel.org?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] PCI: iproc-msi: Bound events handled per IRQ
2026-10-16 12:18 [PATCH] PCI: iproc-msi: Bound events handled per IRQ Eugen Hristev
2026-10-16 12:31 ` sashiko-bot
@ 2026-10-16 18:21 ` Bjorn Helgaas
2026-10-16 20:10 ` Eugen Hristev
1 sibling, 1 reply; 4+ messages in thread
From: Bjorn Helgaas @ 2026-10-16 18:21 UTC (permalink / raw)
To: Eugen Hristev
Cc: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Ray Jui,
Scott Branden, Broadcom internal kernel review list, linux-pci,
linux-arm-kernel, linux-kernel
On Fri, Oct 16, 2026 at 03:18:53PM +0300, Eugen Hristev wrote:
> Limit each iProc MSI parent IRQ to 16 event-queue entries.
>
> A continuously fed queue could otherwise keep the chained handler running
> and starve other work. The budget still drains pending events and lets the
> next parent interrupt continue processing, improving fairness and bounding
> interrupt-handler latency.
>
> Without this starvation avoidance, one CPU can stall indefinitely
> in the loop and not serve other interrupts, timers, etc.
>
> While testing it with an Ethernet NIC, when too many MSI interrupts
> come in, CPU might get starved:
>
> Connecting to host 192.168.30.1, port 5201
> [ 5] local 192.168.30.2 port 52586 connected to 192.168.30.1 port 5201
> [ ID] Interval Transfer Bitrate Retr Cwnd
> [ 5] 0.00-1.00 sec 240 MBytes 2.01 Gbits/sec 2 334 KBytes
> [ 5] 1.00-2.00 sec 63.2 MBytes 531 Mbits/sec 1 1.41 KBytes
> [ 5] 2.00-3.00 sec 0.00 Bytes 0.00 bits/sec 0 1.41 KBytes
> [ 5] 3.00-4.00 sec 0.00 Bytes 0.00 bits/sec 0 1.41 KBytes
> [ 5] 4.00-5.00 sec 0.00 Bytes 0.00 bits/sec 0 1.41 KBytes
> [ 5] 5.00-6.00 sec 0.00 Bytes 0.00 bits/sec 0 1.41 KBytes
> [ 5] 6.00-7.00 sec 0.00 Bytes 0.00 bits/sec 0 1.41 KBytes
> [ 5] 7.00-8.00 sec 0.00 Bytes 0.00 bits/sec 0 1.41 KBytes
> [ 5] 8.00-9.00 sec 0.00 Bytes 0.00 bits/sec 0 1.41 KBytes
> [ 177.568497] rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
> [ 177.568913] rcu: 0-...!: (1 GPs behind) idle=28ac/1/0x4000000000000002 softirq=28922/28930 fqs=1
> [ 177.569457] rcu: (detected by 1, t=5256 jiffies, g=21629, q=819 ncpus=4)
> [ 177.569870] Sending NMI from CPU 1 to CPUs 0:
>
> With this bounded approach, IRQ handler will complete and continue
> to serve MSI events on the next handler invocation.
Is there something unique about iproc that makes this an issue, or do
we need to worry about this in every MSI interrupt handler?
> Assisted-by: Github Copilot
> Signed-off-by: Eugen Hristev <ehristev@kernel.org>
> ---
> drivers/pci/controller/pcie-iproc-msi.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/pcie-iproc-msi.c b/drivers/pci/controller/pcie-iproc-msi.c
> index 9ba242ab9596..6649807822d2 100644
> --- a/drivers/pci/controller/pcie-iproc-msi.c
> +++ b/drivers/pci/controller/pcie-iproc-msi.c
> @@ -29,6 +29,9 @@
> /* Number of entries in each event queue */
> #define EQ_LEN 64
>
> +/* Bound parent interrupt handler such that a queue cannot starve CPU. */
> +#define IPROC_MSI_MAX_EVENTS_PER_IRQ 16
> +
> /* Size of each event queue memory region */
> #define EQ_MEM_REGION_SIZE SZ_4K
>
> @@ -326,6 +329,7 @@ static void iproc_msi_handler(struct irq_desc *desc)
> struct iproc_msi_grp *grp;
> struct iproc_msi *msi;
> u32 eq, head, tail, nr_events;
> + unsigned int event_budget = IPROC_MSI_MAX_EVENTS_PER_IRQ;
> unsigned long hwirq;
>
> chained_irq_enter(chip, desc);
> @@ -360,9 +364,11 @@ static void iproc_msi_handler(struct irq_desc *desc)
> break;
>
> /* process all outstanding events */
> - while (nr_events--) {
> + while (nr_events && event_budget) {
> hwirq = decode_msi_hwirq(msi, eq, head);
> generic_handle_domain_irq(msi->inner_domain, hwirq);
> + nr_events--;
> + event_budget--;
>
> head++;
> head %= EQ_LEN;
> @@ -373,6 +379,8 @@ static void iproc_msi_handler(struct irq_desc *desc)
> * head pointer.
> */
> iproc_msi_write_reg(msi, IPROC_MSI_EQ_HEAD, eq, head);
> + if (!event_budget)
> + break;
>
> /*
> * Now go read the tail pointer again to see if there are new
>
> ---
> base-commit: b810335a0699d1810683796b0e3ffe8827ddfe6c
> change-id: 20261016-iproc-irq-9c58cfe39158
>
> Best regards,
> --
> Eugen Hristev <ehristev@kernel.org>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] PCI: iproc-msi: Bound events handled per IRQ
2026-10-16 18:21 ` Bjorn Helgaas
@ 2026-10-16 20:10 ` Eugen Hristev
0 siblings, 0 replies; 4+ messages in thread
From: Eugen Hristev @ 2026-10-16 20:10 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Ray Jui,
Scott Branden, Broadcom internal kernel review list, linux-pci,
linux-arm-kernel, linux-kernel
On 10/16/26 21:21, Bjorn Helgaas wrote:
> On Fri, Oct 16, 2026 at 03:18:53PM +0300, Eugen Hristev wrote:
>> Limit each iProc MSI parent IRQ to 16 event-queue entries.
>>
>> A continuously fed queue could otherwise keep the chained handler running
>> and starve other work. The budget still drains pending events and lets the
>> next parent interrupt continue processing, improving fairness and bounding
>> interrupt-handler latency.
>>
>> Without this starvation avoidance, one CPU can stall indefinitely
>> in the loop and not serve other interrupts, timers, etc.
>>
>> While testing it with an Ethernet NIC, when too many MSI interrupts
>> come in, CPU might get starved:
>>
>> Connecting to host 192.168.30.1, port 5201
>> [ 5] local 192.168.30.2 port 52586 connected to 192.168.30.1 port 5201
>> [ ID] Interval Transfer Bitrate Retr Cwnd
>> [ 5] 0.00-1.00 sec 240 MBytes 2.01 Gbits/sec 2 334 KBytes
>> [ 5] 1.00-2.00 sec 63.2 MBytes 531 Mbits/sec 1 1.41 KBytes
>> [ 5] 2.00-3.00 sec 0.00 Bytes 0.00 bits/sec 0 1.41 KBytes
>> [ 5] 3.00-4.00 sec 0.00 Bytes 0.00 bits/sec 0 1.41 KBytes
>> [ 5] 4.00-5.00 sec 0.00 Bytes 0.00 bits/sec 0 1.41 KBytes
>> [ 5] 5.00-6.00 sec 0.00 Bytes 0.00 bits/sec 0 1.41 KBytes
>> [ 5] 6.00-7.00 sec 0.00 Bytes 0.00 bits/sec 0 1.41 KBytes
>> [ 5] 7.00-8.00 sec 0.00 Bytes 0.00 bits/sec 0 1.41 KBytes
>> [ 5] 8.00-9.00 sec 0.00 Bytes 0.00 bits/sec 0 1.41 KBytes
>> [ 177.568497] rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
>> [ 177.568913] rcu: 0-...!: (1 GPs behind) idle=28ac/1/0x4000000000000002 softirq=28922/28930 fqs=1
>> [ 177.569457] rcu: (detected by 1, t=5256 jiffies, g=21629, q=819 ncpus=4)
>> [ 177.569870] Sending NMI from CPU 1 to CPUs 0:
>>
>> With this bounded approach, IRQ handler will complete and continue
>> to serve MSI events on the next handler invocation.
>
> Is there something unique about iproc that makes this an issue, or do
> we need to worry about this in every MSI interrupt handler?
>
This is not specific to iPROC, so in theory it can happen to any other
MSI interrupt handler. If the handler has no way to break the loop, and
the condition that generates the MSI IRQ happens continuously, the same
situation will happen.
Some other drivers e.g. take the IRQ state into a local variable, do the
task, clear it up, and don't care to read it again, like the iPROC is
doing, unless another MSI fires.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-10-16 20:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-16 12:18 [PATCH] PCI: iproc-msi: Bound events handled per IRQ Eugen Hristev
2026-10-16 12:31 ` sashiko-bot
2026-10-16 18:21 ` Bjorn Helgaas
2026-10-16 20:10 ` Eugen Hristev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox