From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id AC3ACE9E2FC for ; Wed, 11 Feb 2026 12:32:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:MIME-Version:References:In-Reply-To:Message-ID:Date:Subject:Cc: To:From:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=Hr58faM5DwIkAmlhLtcnHsJcQ4DMZ+02Xio2GfGAQmY=; b=CXz3ZcgU9MNOofj1RlapSmJZOp C4DHZlCcSMF3rkAsDwelWwRy6SUxjtNSRNETSF3lp4V5ECJV70butqtEMkWNp6n0+N6Tkvo9uah8D 5HdgDJws6+aH1Kl8FbJVTrnN4JwI0mYELRDoXLeCYBjDeq4yTeSAsyhseSMP3Bvbxz7BCLQKUKSR/ PlA/GXbohbFAs7e6JS4tMdN+VpBshgxCyI44yiQMbs1ElVotxb3hgKcHACHe8aKtq0KxI3tYDkEZ2 uFEA68aJbKDqrNEnI/Z9H5IoxFOZqKYsl0qwm9YVgWBXrpHpuLgOfVxcVEi55DeLNvG1wIDjBLF1q M87JAo9Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1vq9OH-00000000cgW-0HuU; Wed, 11 Feb 2026 12:32:21 +0000 Received: from sea.source.kernel.org ([2600:3c0a:e001:78e:0:1991:8:25]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1vq9OE-00000000cfW-2gg8 for linux-arm-kernel@lists.infradead.org; Wed, 11 Feb 2026 12:32:19 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sea.source.kernel.org (Postfix) with ESMTP id 9419542E5B; Wed, 11 Feb 2026 12:32:17 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2ECDEC4CEF7; Wed, 11 Feb 2026 12:32:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1770813137; bh=MqWL7t5U8J2aPpBZafPdls2RGoEtAeN85Xsz9fAz/Ng=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=A8eXsKd7BMZwVepQZb5bzYlVKc3wQPpws7vOlIDFfJYoUdEE7B08QhHeWZTB5L/Vx D4w3e15ioabQX19z4ureovNypkHsHHAwTWjOOqyglj7yn8ARsKXZ8ZlaSkyzhcT6pf 8ImNgL8KMrdkkXIsnlquMLmi5feJG5ip5M8OM4MjfL6EuSlXew980l7gqutTOAZu/y xDWvW/znIAmevnGIHqGbkjZav2Nx6vDrNLeLj0FaINXxCV8RE0Tq4az5bNmg4Po5c/ k3jwP55VCMGDQMpbCa9hHef0nbJb5IXjG5vr5see16M3OYj8tTBKGD8kL+GNe946kY Q6Xg9AiPooJ4Q== From: Sasha Levin To: patches@lists.linux.dev, stable@vger.kernel.org Cc: Sebastian Andrzej Siewior , Jonathan Cameron , Will Deacon , Sasha Levin , mark.rutland@arm.com, clrkwllms@kernel.org, rostedt@goodmis.org, linux-cxl@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-perf-users@vger.kernel.org, linux-rt-devel@lists.linux.dev Subject: [PATCH AUTOSEL 6.19-6.6] perf/cxlpmu: Replace IRQF_ONESHOT with IRQF_NO_THREAD Date: Wed, 11 Feb 2026 07:30:41 -0500 Message-ID: <20260211123112.1330287-31-sashal@kernel.org> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260211123112.1330287-1-sashal@kernel.org> References: <20260211123112.1330287-1-sashal@kernel.org> MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore X-stable-base: Linux 6.19 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20260211_043218_721212_87C3BAAF X-CRM114-Status: GOOD ( 24.31 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org From: Sebastian Andrzej Siewior [ Upstream commit ab26d9c85554c4ff1d95ca8341522880ed9219d6 ] Passing IRQF_ONESHOT ensures that the interrupt source is masked until the secondary (threaded) handler is done. If only a primary handler is used then the flag makes no sense because the interrupt can not fire (again) while its handler is running. The flag also disallows force-threading of the primary handler and the irq-core will warn about this. The intention here was probably not allowing forced-threading. Replace IRQF_ONESHOT with IRQF_NO_THREAD. Reviewed-by: Jonathan Cameron Signed-off-by: Sebastian Andrzej Siewior Signed-off-by: Will Deacon Signed-off-by: Sasha Levin --- LLM Generated explanations, may be completely bogus: Now I have gathered all the information I need for a thorough analysis. Let me compile my findings. --- ## Complete Analysis: `perf/cxlpmu: Replace IRQF_ONESHOT with IRQF_NO_THREAD` ### 1. COMMIT MESSAGE ANALYSIS The commit, authored by Sebastian Andrzej Siewior (bigeasy@linutronix.de) — one of the PREEMPT_RT core maintainers — replaces `IRQF_ONESHOT` with `IRQF_NO_THREAD` in the CXL PMU driver's interrupt registration. The commit message explains: - `IRQF_ONESHOT` ensures the interrupt source is masked until the threaded (secondary) handler finishes - This driver only has a **primary** handler (`cxl_pmu_irq`) and no threaded handler - Therefore `IRQF_ONESHOT` makes no sense here — the interrupt can't fire while its hardirq handler is running anyway - The flag also **disables force-threading** of the primary handler - The "irq-core will warn about this" (via lockdep assertions) - The **intended** semantics were to prevent forced-threading, so `IRQF_NO_THREAD` is the correct replacement ### 2. CODE CHANGE ANALYSIS The change is a **single flag swap** on one line: ```880:881:drivers/perf/cxl_pmu.c rc = devm_request_irq(dev, irq, cxl_pmu_irq, IRQF_SHARED | IRQF_ONESHOT, irq_name, info); ``` becomes: ```880:881:drivers/perf/cxl_pmu.c rc = devm_request_irq(dev, irq, cxl_pmu_irq, IRQF_SHARED | IRQF_NO_THREAD, irq_name, info); ``` #### What `IRQF_ONESHOT` does here (incorrectly): Looking at `irq_setup_forced_threading()` in `kernel/irq/manage.c`: ```1291:1296:kernel/irq/manage.c static int irq_setup_forced_threading(struct irqaction *new) { if (!force_irqthreads()) return 0; if (new->flags & (IRQF_NO_THREAD | IRQF_PERCPU | IRQF_ONESHOT)) return 0; ``` Both `IRQF_NO_THREAD` and `IRQF_ONESHOT` cause `irq_setup_forced_threading()` to bail out early, preventing the interrupt from being force-threaded. However, `IRQF_ONESHOT` has an **additional side effect**: it tells the IRQ core to mask the interrupt line until a threaded handler completes. Since there is no threaded handler here, this masking behavior is semantically wrong. #### Why `IRQF_NO_THREAD` is the correct flag: The `cxl_pmu_irq` handler is a PMU overflow interrupt handler that: 1. Reads the overflow register via `readq()` 2. Processes each overflowed counter via `__cxl_pmu_read()` 3. Clears the overflow status via `writeq()` This handler interacts with perf core internals. As the arm-ccn PMU fix (commit `0811ef7e2f54`) established, **PMU interrupt handlers must not be force-threaded** because the perf core relies on strict CPU affinity and interrupt disabling for mutual exclusion. Force-threading a PMU interrupt handler would break these synchronization guarantees. #### The actual bugs: **Bug 1 — PREEMPT_RT / `threadirqs` warning:** When the kernel is booted with `threadirqs` command-line parameter or PREEMPT_RT is enabled, the IRQ core's lockdep infrastructure marks handlers as `hardirq_threaded` if they can be force-threaded. In `kernel/irq/handle.c`: ```199:201:kernel/irq/handle.c if (irq_settings_can_thread(desc) && !(action->flags & (IRQF_NO_THREAD | IRQF_PERCPU | IRQF_ONESHOT))) lockdep_hardirq_threaded(); ``` Because `IRQF_ONESHOT` is already set, this particular path won't trigger the threaded annotation, but the **masking semantics** of ONESHOT are incorrect for a primary-only handler. In the `IRQF_ONESHOT` path, the IRQ core does: ```1726:1727:kernel/irq/manage.c if (new->flags & IRQF_ONESHOT) desc->istate |= IRQS_ONESHOT; ``` This causes the interrupt line to be masked during the handler and unmask logic depends on `desc->threads_oneshot` — but there's no thread to clear this mask, so it depends on the `cond_unmask_irq`/`cond_unmask_eoi_irq` fallback path. **Bug 2 — IRQF_SHARED conflict potential:** The interrupt uses `IRQF_SHARED`. When sharing interrupts, all handlers on the same line must agree on `IRQF_ONESHOT`. If another driver on the shared line doesn't use `IRQF_ONESHOT`, `request_threaded_irq()` will fail with `-EINVAL` at the mismatch check: ```1606:1607:kernel/irq/manage.c else if ((old->flags ^ new->flags) & IRQF_ONESHOT) goto mismatch; ``` This is a real failure mode for shared interrupts — CXL PMU's incorrect use of `IRQF_ONESHOT` could prevent other handlers from sharing the same IRQ line. ### 3. CLASSIFICATION This is a **bug fix** — it corrects incorrect IRQ flag usage that: 1. Applies semantically wrong masking behavior (ONESHOT without a thread) 2. Can trigger warnings/assertions under PREEMPT_RT or `threadirqs` 3. Could cause shared IRQ registration failures 4. Prevents force-threading in the wrong way (the intention is correct, but the mechanism is wrong) ### 4. SCOPE AND RISK ASSESSMENT - **Lines changed:** 1 line (single flag change) - **Files touched:** 1 (`drivers/perf/cxl_pmu.c`) - **Complexity:** Minimal — straightforward flag replacement - **Risk:** Extremely low. `IRQF_NO_THREAD` is the semantically correct flag for the intended purpose. The interrupt handler is a hardirq-only handler that performs only register I/O and atomic operations — it should never be threaded. - **Subsystem:** CXL PMU performance monitoring driver (PCI-based) ### 5. USER IMPACT - **PREEMPT_RT users:** On PREEMPT_RT kernels, `IRQF_ONESHOT` without a thread handler causes the interrupt line to be masked with incorrect oneshot semantics. This could lead to lockdep warnings or subtle interrupt masking issues. - **Shared IRQ users:** CXL devices sharing PCI MSI/MSI-X vectors with other devices could experience IRQ registration failures due to ONESHOT flag mismatch. - **General users:** The CXL PMU driver is used for performance monitoring on CXL 3.0 memory devices — an increasingly important hardware platform for servers and data centers. ### 6. STABILITY INDICATORS - **Author:** Sebastian Andrzej Siewior — PREEMPT_RT core maintainer, expert on IRQ threading - **Reviewer:** Jonathan Cameron — CXL maintainer - **Signed-off-by:** Will Deacon — ARM architecture and perf subsystem maintainer - **Pattern:** This is the same fix Sebastian has applied to multiple drivers (i2c-exynos5, i2c-hix5hd2) with proven track record - **Precedent:** The arm-ccn PMU used exactly this fix (`IRQF_NO_THREAD`) for the same class of issue ### 7. DEPENDENCY CHECK - No dependencies on other commits - The driver exists in stable kernels 6.6.y and later (introduced in v6.5-rc1) - The change is self-contained — just a flag change in `devm_request_irq()` ### 8. CONCLUSION This is a small, surgical, well-understood bug fix that corrects incorrect IRQ flag usage in the CXL PMU driver. The `IRQF_ONESHOT` flag was semantically wrong for a primary-only interrupt handler and could cause: - Incorrect interrupt line masking behavior - Shared IRQ registration failures - Issues on PREEMPT_RT kernels The replacement with `IRQF_NO_THREAD` correctly expresses the intended behavior (prevent force-threading of this PMU interrupt handler) without the incorrect side effects. The fix is trivial, low-risk, authored by a world-class IRQ threading expert, and reviewed by CXL and ARM maintainers. **YES** drivers/perf/cxl_pmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c index d094030220bf2..68a54d97d2a8a 100644 --- a/drivers/perf/cxl_pmu.c +++ b/drivers/perf/cxl_pmu.c @@ -877,7 +877,7 @@ static int cxl_pmu_probe(struct device *dev) if (!irq_name) return -ENOMEM; - rc = devm_request_irq(dev, irq, cxl_pmu_irq, IRQF_SHARED | IRQF_ONESHOT, + rc = devm_request_irq(dev, irq, cxl_pmu_irq, IRQF_SHARED | IRQF_NO_THREAD, irq_name, info); if (rc) return rc; -- 2.51.0