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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/27/24 8:17 PM, Guenter Roeck wrote:
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.

OK, I missed the "if" part after the string. Yes, that do force PREEMPT_RT supporting arches to set PROVE_RAW_LOCK_NESTING while enabling arches that do not support PREEMPT_RT to optionally set it. I will post a v2 patch with that change.

Thanks,
Longman





[Index of Archives]     [Kernel Development]     [DCCP]     [Linux ARM Development]     [Linux]     [Photo]     [Yosemite Help]     [Linux ARM Kernel]     [Linux SCSI]     [Linux x86_64]     [Linux Hams]

  Powered by Linux