linux-kernel - Re: [PATCH] sparc/pci: Make pci_poke_lock a 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: <b59961b6-2026-4f7a-8b72-9b94adfff310@roeck-us.net>
Date: Wed, 27 Nov 2024 17:17:32 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Waiman Long <llong@...hat.com>,
 Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc: Andreas Larsson <andreas@...sler.com>, sparclinux@...r.kernel.org,
 linux-kernel@...r.kernel.org, Boqun Feng <boqun.feng@...il.com>,
 Ingo Molnar <mingo@...hat.com>, Peter Zijlstra <peterz@...radead.org>,
 Thomas Gleixner <tglx@...utronix.de>, Will Deacon <will@...nel.org>,
 "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH] sparc/pci: Make pci_poke_lock a raw_spinlock_t.

On 11/27/24 16:31, Waiman Long wrote:
> 
> On 11/27/24 7:08 PM, Guenter Roeck wrote:
>> On 11/27/24 15:47, Waiman Long wrote:
>>> On 11/27/24 12:44 PM, Guenter Roeck wrote:
>>>> On 11/27/24 08:53, Sebastian Andrzej Siewior wrote:
>>>>> On 2024-11-27 08:02:50 [-0800], Guenter Roeck wrote:
>>>>>> On 11/27/24 07:39, Andreas Larsson wrote:
>>>>>>> Even though this is for sparc64, there is work being done looking into
>>>>>>> enabling RT for sparc32. If the amount of fixes needed to keep
>>>>>>> PROVE_RAW_LOCK_NESTING enabled is quite small at the moment I'd rather
>>>>>>> see it enabled for sparc rather than risking it becoming worse in the
>>>>>>> future.
>>>>>
>>>>> Okay. So you seem to be in favour of fixing the sparc64 splats Guenter
>>>>> reported?
>>>>>
>>>>>>> I don't know what the situation is for other architectures that does not
>>>>>>> support RT.
>>>>>>>
>>>>>>
>>>>>> For my part I still don't understand why PROVE_RAW_LOCK_NESTING is no longer
>>>>>> a configurable option, or in other words why it is mandated even for architectures
>>>>>> not supporting RT. To me this means that I'll either have to disable PROVE_LOCKING
>>>>>> for sparc or live with endless warning backtraces. The latter obscures real
>>>>>> problems, so it is a no-go.
>>>>>
>>>>> It is documented in Documentation/locking/locktypes.rst how the locks
>>>>> should nest. It is just nobody enabled it on sparc64 and tested. The
>>>>> option was meant temporary until the big read blocks are cleared.
>>>>>
>>>>
>>>> That doesn't explain why PROVE_RAW_LOCK_NESTING is now mandatory if
>>>> PROVE_LOCKING is enabled, even on architectures where is was not tested.
>>>> I am all for testing, but that doesn't include making it mandatory
>>>> even where it is known to fail. Enabling it by default, sure, no problem.
>>>> Dropping the option entirely after it is proven to no longer needed,
>>>> also no problem. But force-enabling it even where untested or, worse,
>>>> known to fail, is two steps too far.
>>>
>>> The main reason for enforcing PROVE_RAW_LOCK_NESTING with PROVE_LOCKING is due to the fact that PREEMPT_RT kernel is much less tested than the non-RT kernel. I do agree that we shouldn't force this on arches that don't support PREEMPT_RT. However, once an arch decides to support PREEMPT_RT, they have to fix all these raw_spinlock nesting problems.
>>>
>>
>> config PROVE_RAW_LOCK_NESTING
>> -       bool
>> +       bool "Enable raw_spinlock - spinlock nesting checks" if ARCH_SUPPORTS_RT=n
>>         depends on PROVE_LOCKING
>> -       default y
>> +       default y if ARCH_SUPPORTS_RT
>>
>> would have accomplished that while at the same time making it optional
>> for non-RT architectures.
> 
> I had actually thought about doing exactly that, but decide to keep the current mode for forcing  PROVE_RAW_LOCK_NESTING for arches that support PREEMPT_RT. I won't mind doing this alternative if others agree.
> 

Forcing PROVE_RAW_LOCK_NESTING for arches that support PREEMPT_RT is exactly
what the above does.

	bool "Enable raw_spinlock - spinlock nesting checks" if ARCH_SUPPORTS_RT=n

makes the flag visible (only) if ARCH_SUPPORTS_RT=n, and

	default y if ARCH_SUPPORTS_RT

(force-)enables it if ARCH_SUPPORTS_RT=y.
	
Guenter