public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Jassi Brar <jassisinghbrar@gmail.com>,
	Sasha Levin <sashal@kernel.org>,
	clrkwllms@kernel.org, rostedt@goodmis.org,
	linux-kernel@vger.kernel.org, linux-rt-devel@lists.linux.dev
Subject: [PATCH AUTOSEL 6.19-5.10] mailbox: bcm-ferxrm-mailbox: Use default primary handler
Date: Sun, 15 Feb 2026 12:41:09 -0500	[thread overview]
Message-ID: <20260215174120.2390402-1-sashal@kernel.org> (raw)

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

[ Upstream commit fa84883d44422208b45869a67c0265234fdce1f0 ]

request_threaded_irq() is invoked with a primary and a secondary handler
and no flags are passed. The primary handler is the same as
irq_default_primary_handler() so there is no need to have an identical
copy.
The lack of the IRQF_ONESHOT can be dangerous because the interrupt
source is not masked while the threaded handler is active. This means,
especially on LEVEL typed interrupt lines, the interrupt can fire again
before the threaded handler had a chance to run.

Use the default primary interrupt handler by specifying NULL and set
IRQF_ONESHOT so the interrupt source is masked until the secondary
handler is done.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Jassi Brar <jassisinghbrar@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Both commits are the same patch. The one being evaluated (fa84883d44422,
signed by Jassi Brar) is the one from the mailbox tree.

## Summary of Analysis

### What the commit fixes

The commit fixes a missing `IRQF_ONESHOT` flag in
`request_threaded_irq()` for the BCM FlexRM mailbox driver. The existing
code had a custom primary handler (`flexrm_irq_event`) that was
functionally identical to the kernel's `irq_default_primary_handler()` —
it simply returned `IRQ_WAKE_THREAD`. However, because the code didn't
set `IRQF_ONESHOT`, the interrupt line is not masked while the threaded
handler runs. On level-triggered interrupt lines, this can cause an
interrupt storm (the interrupt fires repeatedly before the threaded
handler gets to run).

### Why it matters

1. **Correctness**: Without `IRQF_ONESHOT`, if the interrupt is level-
   triggered (or chained through a level-triggered parent), the
   interrupt will continuously fire between the primary handler
   returning and the threaded handler completing. This is a potential
   interrupt storm / soft lockup.

2. **Author credibility**: Sebastian Siewior is the PREEMPT_RT
   maintainer and a leading expert on interrupt handling in Linux.
   Thomas Gleixner, the IRQ subsystem maintainer, also signed off on
   this patch via the tip tree.

3. **Pattern**: This is part of a systematic series fixing the same
   pattern across multiple drivers (efct, btintel_pcie, fsl-mc, etc.),
   all from the same author.

### Stable kernel criteria

- **Obviously correct**: Yes - removes an exact duplicate of
  `irq_default_primary_handler()` and adds the missing `IRQF_ONESHOT`
  flag. This is the canonical pattern for threaded IRQs.
- **Fixes a real bug**: Yes - missing `IRQF_ONESHOT` can cause interrupt
  storms on level-triggered lines.
- **Small and contained**: Yes - 1 file changed, 2 insertions, 12
  deletions. The change is strictly self-contained.
- **No new features**: Correct - this is purely a bug fix.
- **Risk**: Very low. The behavior is identical except that the
  interrupt line is now properly masked during threaded handler
  execution, which prevents the storm.

### Nuance: MSI vs Level-Triggered

The driver uses platform MSI interrupts for completions (the comment
says "We only have MSI for completions"). MSI interrupts are typically
edge-triggered and may not suffer from the re-assertion problem.
However:
1. Platform MSIs may be chained through parent controllers that ARE
   level-triggered.
2. The underlying IRQ chip may not have `IRQCHIP_ONESHOT_SAFE` set (only
   PCI MSI explicitly sets this).
3. Even if the risk is lower for MSI, adding `IRQF_ONESHOT` is the
   correct defensive practice and has zero downside.
4. The kernel enforcement check (`__setup_irq`) would actually reject
   the new code (handler=NULL without IRQF_ONESHOT) if the chip lacks
   `IRQCHIP_ONESHOT_SAFE`, so `IRQF_ONESHOT` is needed for the code to
   even work.

### Verification

- **Verified** `flexrm_irq_event()` returns only `IRQ_WAKE_THREAD` at
  line 1176-1182 of bcm-flexrm-mailbox.c, identical to
  `irq_default_primary_handler()` at kernel/irq/manage.c:976-979.
- **Verified** `request_threaded_irq()` with handler=NULL uses
  `irq_default_primary_handler` (kernel/irq/manage.c:2128-2132).
- **Verified** `__setup_irq()` requires IRQF_ONESHOT when
  handler==irq_default_primary_handler and chip lacks
  IRQCHIP_ONESHOT_SAFE (kernel/irq/manage.c:1664-1685).
- **Verified** platform MSI irq chips do NOT automatically set
  `IRQCHIP_ONESHOT_SAFE` - only PCI MSI does
  (drivers/pci/msi/irqdomain.c:133,187). No irqchip drivers set it
  either.
- **Verified** the driver has existed since v4.12 (commit
  dbc049eee7300), present in all active stable trees.
- **Verified** the fix is self-contained: only touches 1 file, no
  dependencies on other commits.
- **Verified** author Sebastian Siewior is the PREEMPT_RT maintainer
  with deep IRQ expertise, and this is part of a systematic series.
- **Verified** the commit also went through Thomas Gleixner's tip tree
  (commit 03843d95a4a4e) with his Signed-off-by, confirming IRQ
  subsystem maintainer review.

This is a small, safe, well-understood bug fix from a domain expert that
prevents a potential interrupt storm. It meets all stable kernel
criteria.

**YES**

 drivers/mailbox/bcm-flexrm-mailbox.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/mailbox/bcm-flexrm-mailbox.c b/drivers/mailbox/bcm-flexrm-mailbox.c
index 41f79e51d9e5a..4255fefc3a5a0 100644
--- a/drivers/mailbox/bcm-flexrm-mailbox.c
+++ b/drivers/mailbox/bcm-flexrm-mailbox.c
@@ -1173,14 +1173,6 @@ static int flexrm_debugfs_stats_show(struct seq_file *file, void *offset)
 
 /* ====== FlexRM interrupt handler ===== */
 
-static irqreturn_t flexrm_irq_event(int irq, void *dev_id)
-{
-	/* We only have MSI for completions so just wakeup IRQ thread */
-	/* Ring related errors will be informed via completion descriptors */
-
-	return IRQ_WAKE_THREAD;
-}
-
 static irqreturn_t flexrm_irq_thread(int irq, void *dev_id)
 {
 	flexrm_process_completions(dev_id);
@@ -1271,10 +1263,8 @@ static int flexrm_startup(struct mbox_chan *chan)
 		ret = -ENODEV;
 		goto fail_free_cmpl_memory;
 	}
-	ret = request_threaded_irq(ring->irq,
-				   flexrm_irq_event,
-				   flexrm_irq_thread,
-				   0, dev_name(ring->mbox->dev), ring);
+	ret = request_threaded_irq(ring->irq, NULL, flexrm_irq_thread,
+				   IRQF_ONESHOT, dev_name(ring->mbox->dev), ring);
 	if (ret) {
 		dev_err(ring->mbox->dev,
 			"failed to request ring%d IRQ\n", ring->num);
-- 
2.51.0


             reply	other threads:[~2026-02-15 17:41 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-15 17:41 Sasha Levin [this message]
2026-02-15 17:41 ` [PATCH AUTOSEL 6.19-6.1] mailbox: pcc: Remove spurious IRQF_ONESHOT usage Sasha Levin
2026-02-15 17:41 ` [PATCH AUTOSEL 6.19-6.1] remoteproc: imx_dsp_rproc: Skip RP_MBOX_SUSPEND_SYSTEM when mailbox TX channel is uninitialized Sasha Levin
2026-02-15 17:41 ` [PATCH AUTOSEL 6.19-6.1] mailbox: imx: Skip the suspend flag for i.MX7ULP Sasha Levin
2026-02-15 17:41 ` [PATCH AUTOSEL 6.19-6.18] mailbox: mchp-ipc-sbi: fix uninitialized symbol and other smatch warnings Sasha Levin
2026-02-15 17:41 ` [PATCH AUTOSEL 6.19-5.15] mailbox: sprd: mask interrupts that are not handled Sasha Levin
2026-02-15 17:41 ` [PATCH AUTOSEL 6.19-6.18] mailbox: mchp-ipc-sbi: fix out-of-bounds access in mchp_ipc_get_cluster_aggr_irq() Sasha Levin
2026-02-15 17:41 ` [PATCH AUTOSEL 6.19-5.10] mailbox: sprd: clear delivery flag before handling TX done Sasha Levin
2026-02-15 17:41 ` [PATCH AUTOSEL 6.19-6.18] f2fs: fix to do sanity check on node footer in __write_node_folio() Sasha Levin
2026-02-15 17:41 ` [PATCH AUTOSEL 6.19-5.15] remoteproc: mediatek: Break lock dependency to `prepare_lock` Sasha Levin
  -- strict thread matches above, loose matches on Subject: below --
2026-02-12  1:09 [PATCH AUTOSEL 6.19-5.10] clocksource/drivers/sh_tmu: Always leave device running after probe Sasha Levin
2026-02-12  1:09 ` [PATCH AUTOSEL 6.19-5.10] mailbox: bcm-ferxrm-mailbox: Use default primary handler Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260215174120.2390402-1-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=bigeasy@linutronix.de \
    --cc=clrkwllms@kernel.org \
    --cc=jassisinghbrar@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-devel@lists.linux.dev \
    --cc=patches@lists.linux.dev \
    --cc=rostedt@goodmis.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox