Uday Shankar <ushankar@xxxxxxxxxxxxxxx> writes:
> On Mon, Aug 21, 2023 at 08:16:05PM -0700, Jörn Engel wrote:
>> On Tue, Aug 22, 2023 at 09:56:38AM +0800, Huang, Ying wrote:
>> >
>> > ERST is mainly used to log the hardware error. While, hardware error
>> > may be reported via NMI (e.g., ACPI APEI GHES NMI), so we need to call
>> > ERST functions in NMI handlers. Where normal spinlock cannot be used
>> > because they will be converted to sleepable rt_mutex in RT kernel.
>>
>> Non-sleeping spinlocks cannot be used in NMI context either.
>> raw_spin_lock_irqsave() will prevent regular interrupts, but not NMI.
>> So taking a spinlock inside an NMI can trigger a deadlock.
>>
>> Am I missing something here?
>>
>> Jörn
>>
>> --
>> All art is but imitation of nature.
>> -- Lucius Annaeus Seneca
>
> Also want to point out that both before and after this commit, we only
> use trylock from erst_write, which looks like the only function touched
> by this patch which can be called from NMI context. Trylock should be
> safe in NMI context regardless of the type of synchronization used
> (raw_spinlock, spinlock, or otherwise).
Thanks for reminding! That's a good point.
Checked the implementation of rt_mutex version of spin_trylock(). One
possible code path is,
spin_trylock()
rt_spin_trylock()
__rt_spin_trylock()
rt_mutex_slowtrylock()
raw_spin_lock_irqsave()
IIUC, the deadlock is still possible for rt_mutex. While it seems that
the deadlock isn't possible for raw_spinlock. If so, it's still better
to use raw_spinlock.
--
Best Regards,
Huang, Ying
