Re: [PATCH] sparc/pci: Make pci_poke_lock a raw_spinlock_t.

Waiman Long
2024-11-28T00:31:37Z

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.

Cheers,
Longman