linux-kernel - Re: [PATCH v3 1/2] PCI/aer_inject: Convert inject_lock to raw_spinlock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251027081125.n7far5BO@linutronix.de>
Date: Mon, 27 Oct 2025 09:11:25 +0100
From: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To: Guangbo Cui <jckeep.cuiguangbo@...il.com>
Cc: Clark Williams <clrkwllms@...nel.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>,
	Boqun Feng <boqun.feng@...il.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Jonathan Cameron <jonathan.cameron@...wei.com>,
	Waiman Long <longman@...hat.com>, linux-rt-devel@...ts.linux.dev,
	linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org
Subject: Re: [PATCH v3 1/2] PCI/aer_inject: Convert inject_lock to
 raw_spinlock_t

On 2025-10-26 04:43:34 [+0000], Guangbo Cui wrote:
> The AER injection path may run in forced-threaded interrupt context
> under PREEMPT_RT while holding the lock with IRQs disabled.

not IRQs but interrupts. 

> On RT kernels, spinlocks are converted into rt_spinlocks, which may

rt_spinlocks is not a thing. An established term is sleeping spinlock.

> sleep. Sleeping is not permitted in IRQ-off sections, so this may
> trigger lockdep warnings such as "Invalid wait context" in [1].

Why that reference. You should be able to describe your change _here_.

>   raw_spin_lock_irqsave(&pci_lock);
>       ↓
>   rt_spin_lock(&inject_lock);  <-- not allowed

It is still a spin_lock() that happens. The problem is that a
raw_spinlock_t must not nest into a spinlock_t. See
	Documentation/locking/locktypes.rst

very bottom of that file.

> Switching inject_lock to raw_spinlock_t preserves non-sleeping locking
> semantics and avoids the warning when running with PREEMPT_RT enabled.
> 
> The list protected by this lock is bounded and only used for debugging
> purposes, so using a raw spinlock will not cause unbounded latencies.

This is I like.

> [1] https://lore.kernel.org/all/20251009150651.93618-1-jckeep.cuiguangbo@gmail.com/
> 
> Acked-by: Waiman Long <longman@...hat.com>
> Signed-off-by: Guangbo Cui <jckeep.cuiguangbo@...il.com>
> ---
> @@ -445,7 +445,7 @@ static int aer_inject(struct aer_error_inj *einj)
>  		rperr->source_id &= 0x0000ffff;
>  		rperr->source_id |= PCI_DEVID(einj->bus, devfn) << 16;
>  	}
> -	spin_unlock_irqrestore(&inject_lock, flags);
> +	raw_spin_unlock_irqrestore(&inject_lock, flags);
>  
>  	if (aer_mask_override) {
>  		pci_write_config_dword(dev, pos_cap_err + PCI_ERR_COR_MASK,

This is the last hunk. You miss the module exit part. This won't
compile on its own, as such it can't be applied. It might be chosen for
a backport. A change must always be self-contained.

Sebastian