Re: [PATCH v19 03/14] acpi/apei/ghes: Use raw_spinlock_t for CXL CPER work locks

"Luck, Tony" <[email protected]> Wed, 5 Aug 2026 11:41:25 -0700
Newsgroups org.kernel.vger.linux-acpi,org.kernel.vger.linux-cxl,org.kernel.vger.linux-doc,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci
Message-ID <anOD1casyoxFOgG7@agluck-desk3>
On Mon, Aug 03, 2026 at 05:17:59PM -0500, Terry Bowman wrote:
> The CXL CPER work registration and unregistration helpers acquire
> cxl_cper_work_lock and cxl_cper_prot_err_work_lock with a spinlock
> guard(), which leaves local interrupts enabled. The corresponding post
> paths (cxl_cper_post_event(), cxl_cper_post_prot_err()) execute in hard
> IRQ context (they are called from the GHES error notification path) and
> acquire the same locks with an irqsave guard().
> 
> If a CPU is holding one of these locks via a spinlock guard() when a GHES
> interrupt arrives on the same CPU, the IRQ handler spins on the held lock
> waiting for it to release, while the lock holder is preempted by the IRQ.
> The result is a deadlock.
> 
> Convert both locks from spinlock_t to raw_spinlock_t and use guard() at
> all call sites. On PREEMPT_RT kernels spinlock_t is backed by rt_mutex and
> sleeping from hard IRQ context is not permitted; raw_spinlock_t is safe in
> both contexts.
> 
> Add WARN_ONCE to both register functions to surface double-registration
> bugs at runtime.
> 
> Restructure both unregister functions to clear the global work pointer
> under the lock before calling cancel_work_sync(), closing the window
> where a CPER interrupt could schedule work on a pointer about to be
> freed. Add kfifo_reset() after cancel_work_sync() so stale entries
> are not replayed on next module load.
> 
> Both kfifos are single-consumer: only one work_struct is registered at
> a time, enforced by the WARN_ONCE guard in the register functions.
> kfifo_reset() is safe outside the lock because cancel_work_sync() has
> already quiesced the consumer, and no new consumer can register until
> the current module exit completes and a fresh module init runs.
> 
> Remove the redundant cancel_work_sync() call from cxl_ras_exit() and
> cxl_pci_driver_exit(). The CPER unregister functions now quiesce
> the work internally.
> 
> Reported-by: Sashiko <[email protected]>
> Signed-off-by: Terry Bowman <[email protected]>
> Fixes: 5e4a264bf8b5 ("acpi/ghes: Process CXL Component Events")
> Fixes: 36f257e3b0ba ("acpi/ghes, cxl/pci: Process CXL CPER Protocol Errors")
> Cc: [email protected]
> Reviewed-by: Dave Jiang <[email protected]>
> Reviewed-by: Jonathan Cameron <[email protected]>

Reviewed-by: Tony Luck <[email protected]>

[But could this be split ... the commit message feels like a list
of three changes. No strong feelings about this]

-Tony