[PATCH 0/1] lockdep: Enable PROVE_RAW_LOCK_NESTING with PROVE_LOCKING.

[PATCH 0/1] lockdep: Enable PROVE_RAW_LOCK_NESTING with PROVE_LOCKING.

[Sebastian Andrzej Siewior]

Hi,

PeterZ poked me regarding the status of PROVE_RAW_LOCK_NESTING. To my
best knowledge printk was triggering this during boot. This is gone
since the rework that landed in v6.12.
Before that rework, people booting with `quiet' got reports for "other"
splats and handled them. I booted a few boxes with this option enabled,
and didn't run into any splats.

It might be time to enable it by default.

Sebastian

[PATCH 1/1] lockdep: Enable PROVE_RAW_LOCK_NESTING with PROVE_LOCKING.

[Sebastian Andrzej Siewior]

With the printk issues solved, the last known splat created by
PROVE_RAW_LOCK_NESTING is gone.

Enable PROVE_RAW_LOCK_NESTING by default as part of PROVE_LOCKING. Keep
the defines around in case something serious pops up and it needs to be
disabled.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 lib/Kconfig.debug | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 7315f643817ae..5b67816f4a62f 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1409,22 +1409,14 @@ config PROVE_LOCKING
 	 For more details, see Documentation/locking/lockdep-design.rst.
 
 config PROVE_RAW_LOCK_NESTING
-	bool "Enable raw_spinlock - spinlock nesting checks"
+	bool
 	depends on PROVE_LOCKING
-	default n
+	default y
 	help
 	 Enable the raw_spinlock vs. spinlock nesting checks which ensure
 	 that the lock nesting rules for PREEMPT_RT enabled kernels are
 	 not violated.
 
-	 NOTE: There are known nesting problems. So if you enable this
-	 option expect lockdep splats until these problems have been fully
-	 addressed which is work in progress. This config switch allows to
-	 identify and analyze these problems. It will be removed and the
-	 check permanently enabled once the main issues have been fixed.
-
-	 If unsure, select N.
-
 config LOCK_STAT
 	bool "Lock usage statistics"
 	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
-- 
2.45.2

Re: [PATCH 1/1] lockdep: Enable PROVE_RAW_LOCK_NESTING with PROVE_LOCKING.

[Waiman Long]

On 10/9/24 11:45 AM, Sebastian Andrzej Siewior wrote:
> With the printk issues solved, the last known splat created by
> PROVE_RAW_LOCK_NESTING is gone.
>
> Enable PROVE_RAW_LOCK_NESTING by default as part of PROVE_LOCKING. Keep
> the defines around in case something serious pops up and it needs to be
> disabled.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>   lib/Kconfig.debug | 12 ++----------
>   1 file changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 7315f643817ae..5b67816f4a62f 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1409,22 +1409,14 @@ config PROVE_LOCKING
>   	 For more details, see Documentation/locking/lockdep-design.rst.
>   
>   config PROVE_RAW_LOCK_NESTING
> -	bool "Enable raw_spinlock - spinlock nesting checks"
> +	bool
>   	depends on PROVE_LOCKING
> -	default n
> +	default y
>   	help
>   	 Enable the raw_spinlock vs. spinlock nesting checks which ensure
>   	 that the lock nesting rules for PREEMPT_RT enabled kernels are
>   	 not violated.
>   
> -	 NOTE: There are known nesting problems. So if you enable this
> -	 option expect lockdep splats until these problems have been fully
> -	 addressed which is work in progress. This config switch allows to
> -	 identify and analyze these problems. It will be removed and the
> -	 check permanently enabled once the main issues have been fixed.
> -
> -	 If unsure, select N.
> -
>   config LOCK_STAT
>   	bool "Lock usage statistics"
>   	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
Acked-by: Waiman Long <longman@redhat.com>

Re: [PATCH 1/1] lockdep: Enable PROVE_RAW_LOCK_NESTING with PROVE_LOCKING.

[Boqun Feng]

On Wed, Oct 09, 2024 at 12:50:39PM -0400, Waiman Long wrote:
> 
> On 10/9/24 11:45 AM, Sebastian Andrzej Siewior wrote:
> > With the printk issues solved, the last known splat created by
> > PROVE_RAW_LOCK_NESTING is gone.
> > 
> > Enable PROVE_RAW_LOCK_NESTING by default as part of PROVE_LOCKING. Keep
> > the defines around in case something serious pops up and it needs to be
> > disabled.
> > 
> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > ---
> >   lib/Kconfig.debug | 12 ++----------
> >   1 file changed, 2 insertions(+), 10 deletions(-)
> > 
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index 7315f643817ae..5b67816f4a62f 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -1409,22 +1409,14 @@ config PROVE_LOCKING
> >   	 For more details, see Documentation/locking/lockdep-design.rst.
> >   config PROVE_RAW_LOCK_NESTING
> > -	bool "Enable raw_spinlock - spinlock nesting checks"
> > +	bool
> >   	depends on PROVE_LOCKING
> > -	default n
> > +	default y
> >   	help
> >   	 Enable the raw_spinlock vs. spinlock nesting checks which ensure
> >   	 that the lock nesting rules for PREEMPT_RT enabled kernels are
> >   	 not violated.
> > -	 NOTE: There are known nesting problems. So if you enable this
> > -	 option expect lockdep splats until these problems have been fully
> > -	 addressed which is work in progress. This config switch allows to
> > -	 identify and analyze these problems. It will be removed and the
> > -	 check permanently enabled once the main issues have been fixed.
> > -
> > -	 If unsure, select N.
> > -
> >   config LOCK_STAT
> >   	bool "Lock usage statistics"
> >   	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
> Acked-by: Waiman Long <longman@redhat.com>
> 

Thanks!

Queued in my lockdep branch:

	https://git.kernel.org/pub/scm/linux/kernel/git/boqun/linux.git/log/?h=lockdep-for-tip

will send a PR to tip between -rc3 and -rc4.

Regards,
Boqun

[tip: locking/core] lockdep: Enable PROVE_RAW_LOCK_NESTING with PROVE_LOCKING.

[tip-bot2 for Sebastian Andrzej Siewior]

The following commit has been merged into the locking/core branch of tip:

Commit-ID:     560af5dc839eef08a273908f390cfefefb82aa04
Gitweb:        https://git.kernel.org/tip/560af5dc839eef08a273908f390cfefefb82aa04
Author:        Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate:    Wed, 09 Oct 2024 17:45:03 +02:00
Committer:     Boqun Feng <boqun.feng@gmail.com>
CommitterDate: Thu, 17 Oct 2024 21:21:16 -07:00

lockdep: Enable PROVE_RAW_LOCK_NESTING with PROVE_LOCKING.

With the printk issues solved, the last known splat created by
PROVE_RAW_LOCK_NESTING is gone.

Enable PROVE_RAW_LOCK_NESTING by default as part of PROVE_LOCKING. Keep
the defines around in case something serious pops up and it needs to be
disabled.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Acked-by: Waiman Long <longman@redhat.com>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Link: https://lore.kernel.org/r/20241009161041.1018375-2-bigeasy@linutronix.de
---
 lib/Kconfig.debug | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 7315f64..5b67816 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1409,22 +1409,14 @@ config PROVE_LOCKING
 	 For more details, see Documentation/locking/lockdep-design.rst.
 
 config PROVE_RAW_LOCK_NESTING
-	bool "Enable raw_spinlock - spinlock nesting checks"
+	bool
 	depends on PROVE_LOCKING
-	default n
+	default y
 	help
 	 Enable the raw_spinlock vs. spinlock nesting checks which ensure
 	 that the lock nesting rules for PREEMPT_RT enabled kernels are
 	 not violated.
 
-	 NOTE: There are known nesting problems. So if you enable this
-	 option expect lockdep splats until these problems have been fully
-	 addressed which is work in progress. This config switch allows to
-	 identify and analyze these problems. It will be removed and the
-	 check permanently enabled once the main issues have been fixed.
-
-	 If unsure, select N.
-
 config LOCK_STAT
 	bool "Lock usage statistics"
 	depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT

Re: [PATCH 1/1] lockdep: Enable PROVE_RAW_LOCK_NESTING with PROVE_LOCKING.

[Guenter Roeck]

On Wed, Oct 09, 2024 at 05:45:03PM +0200, Sebastian Andrzej Siewior wrote:
> With the printk issues solved, the last known splat created by
> PROVE_RAW_LOCK_NESTING is gone.
> 
> Enable PROVE_RAW_LOCK_NESTING by default as part of PROVE_LOCKING. Keep
> the defines around in case something serious pops up and it needs to be
> disabled.
> 

On sparc64:

=============================
[ BUG: Invalid wait context ]
6.12.0+ #1 Not tainted
-----------------------------
swapper/0/1 is trying to lock:
0000000001b61428 (pci_poke_lock){....}-{3:3}, at: pci_config_read16+0x8/0x80
other info that might help us debug this:
context-{5:5}
2 locks held by swapper/0/1:
 #0: fffff800042b50f8 (&dev->mutex){....}-{4:4}, at: __driver_attach+0x80/0x160
 #1: 0000000001d201a8 (pci_lock){....}-{2:2}, at: pci_bus_read_config_word+0x18/0x80
stack backtrace:
CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.0+ #1
Call Trace:
[<00000000004e3190>] __lock_acquire+0xa50/0x3160
[<00000000004e63a8>] lock_acquire+0xe8/0x340
[<00000000010ea57c>] _raw_spin_lock_irqsave+0x3c/0x80
[<00000000004434e8>] pci_config_read16+0x8/0x80
[<00000000004440ec>] sun4u_read_pci_cfg+0x12c/0x1a0
[<0000000000b7437c>] pci_bus_read_config_word+0x3c/0x80
[<0000000000b7d878>] pci_find_capability+0x18/0xa0
[<0000000000b772b0>] set_pcie_port_type+0x10/0x160
[<0000000000442758>] pci_of_scan_bus+0x158/0xb00
[<00000000010c0c60>] pci_scan_one_pbm+0xd0/0xf8
[<0000000000445e34>] sabre_probe+0x1f4/0x5c0
[<0000000000c0f9a8>] platform_probe+0x28/0x80
[<0000000000c0d0b8>] really_probe+0xb8/0x340
[<0000000000c0d4e4>] driver_probe_device+0x24/0xe0
[<0000000000c0d70c>] __driver_attach+0x8c/0x160
[<0000000000c0aeb4>] bus_for_each_dev+0x54/0xc0

Is this a problem with the test or with the platform ?

Thanks,
Guenter

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

[Sebastian Andrzej Siewior]

The pci_poke_lock is used underneath of the pci_lock. The pci_lock is a
low level lock used by the core code in sections with disabled
insterrupts. Therefore the pci_poke_lock must be a raw_spinlock_t.

Make pci_poke_lock a raw_spinlock_t.

Reported-by: Guenter Roeck <linux@roeck-us.net>
Closes: https://lore.kernel.org/7656395b-58fc-4874-a9f3-6d934e2ef7ee@roeck-us.net
Fixes: 560af5dc839ee ("lockdep: Enable PROVE_RAW_LOCK_NESTING with PROVE_LOCKING.")
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
On 2024-11-23 08:27:08 [-0800], Guenter Roeck wrote:
> 
> Is this a problem with the test or with the platform ?

platform. The patch below should fix it. It makes no difference unless
used on PREEMPT_RT. Since sparc does not support it I made the fixes
where the default option changed.

Could you test it, please? I don't have a even a compiler for sparc
right now.

 arch/sparc/kernel/pci.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c
index 50a0927a84a6f..12e0b78cded55 100644
--- a/arch/sparc/kernel/pci.c
+++ b/arch/sparc/kernel/pci.c
@@ -42,14 +42,14 @@ volatile int pci_poke_in_progress;
 volatile int pci_poke_cpu = -1;
 volatile int pci_poke_faulted;
 
-static DEFINE_SPINLOCK(pci_poke_lock);
+static DEFINE_RAW_SPINLOCK(pci_poke_lock);
 
 void pci_config_read8(u8 *addr, u8 *ret)
 {
 	unsigned long flags;
 	u8 byte;
 
-	spin_lock_irqsave(&pci_poke_lock, flags);
+	raw_spin_lock_irqsave(&pci_poke_lock, flags);
 	pci_poke_cpu = smp_processor_id();
 	pci_poke_in_progress = 1;
 	pci_poke_faulted = 0;
@@ -63,7 +63,7 @@ void pci_config_read8(u8 *addr, u8 *ret)
 	pci_poke_cpu = -1;
 	if (!pci_poke_faulted)
 		*ret = byte;
-	spin_unlock_irqrestore(&pci_poke_lock, flags);
+	raw_spin_unlock_irqrestore(&pci_poke_lock, flags);
 }
 
 void pci_config_read16(u16 *addr, u16 *ret)
@@ -71,7 +71,7 @@ void pci_config_read16(u16 *addr, u16 *ret)
 	unsigned long flags;
 	u16 word;
 
-	spin_lock_irqsave(&pci_poke_lock, flags);
+	raw_spin_lock_irqsave(&pci_poke_lock, flags);
 	pci_poke_cpu = smp_processor_id();
 	pci_poke_in_progress = 1;
 	pci_poke_faulted = 0;
@@ -85,7 +85,7 @@ void pci_config_read16(u16 *addr, u16 *ret)
 	pci_poke_cpu = -1;
 	if (!pci_poke_faulted)
 		*ret = word;
-	spin_unlock_irqrestore(&pci_poke_lock, flags);
+	raw_spin_unlock_irqrestore(&pci_poke_lock, flags);
 }
 
 void pci_config_read32(u32 *addr, u32 *ret)
@@ -93,7 +93,7 @@ void pci_config_read32(u32 *addr, u32 *ret)
 	unsigned long flags;
 	u32 dword;
 
-	spin_lock_irqsave(&pci_poke_lock, flags);
+	raw_spin_lock_irqsave(&pci_poke_lock, flags);
 	pci_poke_cpu = smp_processor_id();
 	pci_poke_in_progress = 1;
 	pci_poke_faulted = 0;
@@ -107,14 +107,14 @@ void pci_config_read32(u32 *addr, u32 *ret)
 	pci_poke_cpu = -1;
 	if (!pci_poke_faulted)
 		*ret = dword;
-	spin_unlock_irqrestore(&pci_poke_lock, flags);
+	raw_spin_unlock_irqrestore(&pci_poke_lock, flags);
 }
 
 void pci_config_write8(u8 *addr, u8 val)
 {
 	unsigned long flags;
 
-	spin_lock_irqsave(&pci_poke_lock, flags);
+	raw_spin_lock_irqsave(&pci_poke_lock, flags);
 	pci_poke_cpu = smp_processor_id();
 	pci_poke_in_progress = 1;
 	pci_poke_faulted = 0;
@@ -126,14 +126,14 @@ void pci_config_write8(u8 *addr, u8 val)
 			     : "memory");
 	pci_poke_in_progress = 0;
 	pci_poke_cpu = -1;
-	spin_unlock_irqrestore(&pci_poke_lock, flags);
+	raw_spin_unlock_irqrestore(&pci_poke_lock, flags);
 }
 
 void pci_config_write16(u16 *addr, u16 val)
 {
 	unsigned long flags;
 
-	spin_lock_irqsave(&pci_poke_lock, flags);
+	raw_spin_lock_irqsave(&pci_poke_lock, flags);
 	pci_poke_cpu = smp_processor_id();
 	pci_poke_in_progress = 1;
 	pci_poke_faulted = 0;
@@ -145,14 +145,14 @@ void pci_config_write16(u16 *addr, u16 val)
 			     : "memory");
 	pci_poke_in_progress = 0;
 	pci_poke_cpu = -1;
-	spin_unlock_irqrestore(&pci_poke_lock, flags);
+	raw_spin_unlock_irqrestore(&pci_poke_lock, flags);
 }
 
 void pci_config_write32(u32 *addr, u32 val)
 {
 	unsigned long flags;
 
-	spin_lock_irqsave(&pci_poke_lock, flags);
+	raw_spin_lock_irqsave(&pci_poke_lock, flags);
 	pci_poke_cpu = smp_processor_id();
 	pci_poke_in_progress = 1;
 	pci_poke_faulted = 0;
@@ -164,7 +164,7 @@ void pci_config_write32(u32 *addr, u32 val)
 			     : "memory");
 	pci_poke_in_progress = 0;
 	pci_poke_cpu = -1;
-	spin_unlock_irqrestore(&pci_poke_lock, flags);
+	raw_spin_unlock_irqrestore(&pci_poke_lock, flags);
 }
 
 static int ofpci_verbose;
-- 
2.45.2

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

[Guenter Roeck]

On 11/25/24 00:53, Sebastian Andrzej Siewior wrote:
> The pci_poke_lock is used underneath of the pci_lock. The pci_lock is a
> low level lock used by the core code in sections with disabled
> insterrupts. Therefore the pci_poke_lock must be a raw_spinlock_t.
> 
> Make pci_poke_lock a raw_spinlock_t.
> 
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Closes: https://lore.kernel.org/7656395b-58fc-4874-a9f3-6d934e2ef7ee@roeck-us.net
> Fixes: 560af5dc839ee ("lockdep: Enable PROVE_RAW_LOCK_NESTING with PROVE_LOCKING.")
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> On 2024-11-23 08:27:08 [-0800], Guenter Roeck wrote:
>>
>> Is this a problem with the test or with the platform ?
> 
> platform. The patch below should fix it. It makes no difference unless
> used on PREEMPT_RT. Since sparc does not support it I made the fixes
> where the default option changed.
> 
> Could you test it, please? I don't have a even a compiler for sparc
> right now.
> 

Unfortunately it doesn't make a difference.

[    1.050499] =============================
[    1.050801] [ BUG: Invalid wait context ]
[    1.051200] 6.12.0+ #1 Not tainted
[    1.051571] -----------------------------
[    1.051875] swapper/0/1 is trying to lock:
[    1.052201] 0000000001b694c8 (pci_poke_lock){....}-{3:3}, at: pci_config_read16+0x8/0x80
[    1.052994] other info that might help us debug this:
[    1.053331] context-{5:5}
[    1.053641] 2 locks held by swapper/0/1:
[    1.053959]  #0: fffff800042b50f8 (&dev->mutex){....}-{4:4}, at: __driver_attach+0x80/0x160
[    1.054388]  #1: 0000000001d29078 (pci_lock){....}-{2:2}, at: pci_bus_read_config_word+0x18/0x80
[    1.054793] stack backtrace:
[    1.055171] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.0+ #1
[    1.055632] Call Trace:
[    1.055985] [<00000000004e31d0>] __lock_acquire+0xa50/0x3160
[    1.056329] [<00000000004e63e8>] lock_acquire+0xe8/0x340
[    1.056645] [<00000000010f0dfc>] _raw_spin_lock_irqsave+0x3c/0x80
[    1.056966] [<0000000000443828>] pci_config_read16+0x8/0x80
[    1.057278] [<000000000044442c>] sun4u_read_pci_cfg+0x12c/0x1a0
[    1.057593] [<0000000000b7657c>] pci_bus_read_config_word+0x3c/0x80
[    1.057913] [<0000000000b7fa78>] pci_find_capability+0x18/0xa0
[    1.058228] [<0000000000b794b0>] set_pcie_port_type+0x10/0x160
[    1.058543] [<0000000000442a98>] pci_of_scan_bus+0x158/0xb00
[    1.058854] [<00000000010c74a0>] pci_scan_one_pbm+0xd0/0xf8
[    1.059167] [<0000000000446174>] sabre_probe+0x1f4/0x5c0
[    1.059476] [<0000000000c13a48>] platform_probe+0x28/0x80
[    1.059785] [<0000000000c11158>] really_probe+0xb8/0x340
[    1.060098] [<0000000000c11584>] driver_probe_device+0x24/0xe0
[    1.060413] [<0000000000c117ac>] __driver_attach+0x8c/0x160
[    1.060728] [<0000000000c0ef54>] bus_for_each_dev+0x54/0xc0

The original call trace also included _raw_spin_lock_irqsave(), and
I don't have CONFIG_PREEMPT_RT enabled in my sparc64 builds to start with.

FWIW, I don't understand the value of
	pr_warn("context-{%d:%d}\n", curr_inner, curr_inner);
Why print curr_inner twice ?

Thanks,
Guenter

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

[Sebastian Andrzej Siewior]

On 2024-11-25 09:01:33 [-0800], Guenter Roeck wrote:
> Unfortunately it doesn't make a difference.

stunning. It looks like the exact same error message.

> [    1.050499] =============================
> [    1.050801] [ BUG: Invalid wait context ]
> [    1.051200] 6.12.0+ #1 Not tainted
> [    1.051571] -----------------------------
> [    1.051875] swapper/0/1 is trying to lock:
> [    1.052201] 0000000001b694c8 (pci_poke_lock){....}-{3:3}, at: pci_config_read16+0x8/0x80
> [    1.052994] other info that might help us debug this:
> [    1.053331] context-{5:5}
> [    1.053641] 2 locks held by swapper/0/1:
> [    1.053959]  #0: fffff800042b50f8 (&dev->mutex){....}-{4:4}, at: __driver_attach+0x80/0x160
> [    1.054388]  #1: 0000000001d29078 (pci_lock){....}-{2:2}, at: pci_bus_read_config_word+0x18/0x80
> [    1.054793] stack backtrace:
> [    1.055171] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.0+ #1
> [    1.055632] Call Trace:
> [    1.055985] [<00000000004e31d0>] __lock_acquire+0xa50/0x3160
> [    1.056329] [<00000000004e63e8>] lock_acquire+0xe8/0x340
> [    1.056645] [<00000000010f0dfc>] _raw_spin_lock_irqsave+0x3c/0x80
> [    1.056966] [<0000000000443828>] pci_config_read16+0x8/0x80
> [    1.057278] [<000000000044442c>] sun4u_read_pci_cfg+0x12c/0x1a0
> [    1.057593] [<0000000000b7657c>] pci_bus_read_config_word+0x3c/0x80
> [    1.057913] [<0000000000b7fa78>] pci_find_capability+0x18/0xa0
> [    1.058228] [<0000000000b794b0>] set_pcie_port_type+0x10/0x160
> [    1.058543] [<0000000000442a98>] pci_of_scan_bus+0x158/0xb00
> [    1.058854] [<00000000010c74a0>] pci_scan_one_pbm+0xd0/0xf8
> [    1.059167] [<0000000000446174>] sabre_probe+0x1f4/0x5c0
> [    1.059476] [<0000000000c13a48>] platform_probe+0x28/0x80
> [    1.059785] [<0000000000c11158>] really_probe+0xb8/0x340
> [    1.060098] [<0000000000c11584>] driver_probe_device+0x24/0xe0
> [    1.060413] [<0000000000c117ac>] __driver_attach+0x8c/0x160
> [    1.060728] [<0000000000c0ef54>] bus_for_each_dev+0x54/0xc0
> 
> The original call trace also included _raw_spin_lock_irqsave(), and
> I don't have CONFIG_PREEMPT_RT enabled in my sparc64 builds to start with.

You don't have to. "CONFIG_PROVE_RAW_LOCK_NESTING" looks if you try to
acquire raw_spinlock_t -> spinlock_t. Which it did before I made the
patch.
The pci_lock is from drivers/pci/access.c and is defined as
raw_spinlock_t. And I made pci_poke_lock of the same time. But debug
says 3:3 which suggests LD_WAIT_CONFIG. (No patch applied).

> FWIW, I don't understand the value of
> 	pr_warn("context-{%d:%d}\n", curr_inner, curr_inner);
> Why print curr_inner twice ?

The syntax was once (or is) inner:outer. If you look from the top, you
have 4 (mutex_t) followed pci_lock (the raw_spinlock_t) 2. You are at
level 2 now and try to acquire spin_lock_t (3).

> Thanks,
> Guenter

Sebastian

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

[Guenter Roeck]

On 11/25/24 09:43, Sebastian Andrzej Siewior wrote:
> On 2024-11-25 09:01:33 [-0800], Guenter Roeck wrote:
>> Unfortunately it doesn't make a difference.
> 
> stunning. It looks like the exact same error message.
> 

I think it uses

#define spin_lock_irqsave(lock, flags)                          \
do {                                                            \
         raw_spin_lock_irqsave(spinlock_check(lock), flags);     \
} while (0)

from include/linux/spinlock.h, meaning your patch doesn't really make a difference.

>> [    1.050499] =============================
>> [    1.050801] [ BUG: Invalid wait context ]
>> [    1.051200] 6.12.0+ #1 Not tainted
>> [    1.051571] -----------------------------
>> [    1.051875] swapper/0/1 is trying to lock:
>> [    1.052201] 0000000001b694c8 (pci_poke_lock){....}-{3:3}, at: pci_config_read16+0x8/0x80
>> [    1.052994] other info that might help us debug this:
>> [    1.053331] context-{5:5}
>> [    1.053641] 2 locks held by swapper/0/1:
>> [    1.053959]  #0: fffff800042b50f8 (&dev->mutex){....}-{4:4}, at: __driver_attach+0x80/0x160
>> [    1.054388]  #1: 0000000001d29078 (pci_lock){....}-{2:2}, at: pci_bus_read_config_word+0x18/0x80
>> [    1.054793] stack backtrace:
>> [    1.055171] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.0+ #1
>> [    1.055632] Call Trace:
>> [    1.055985] [<00000000004e31d0>] __lock_acquire+0xa50/0x3160
>> [    1.056329] [<00000000004e63e8>] lock_acquire+0xe8/0x340
>> [    1.056645] [<00000000010f0dfc>] _raw_spin_lock_irqsave+0x3c/0x80
>> [    1.056966] [<0000000000443828>] pci_config_read16+0x8/0x80
>> [    1.057278] [<000000000044442c>] sun4u_read_pci_cfg+0x12c/0x1a0
>> [    1.057593] [<0000000000b7657c>] pci_bus_read_config_word+0x3c/0x80
>> [    1.057913] [<0000000000b7fa78>] pci_find_capability+0x18/0xa0
>> [    1.058228] [<0000000000b794b0>] set_pcie_port_type+0x10/0x160
>> [    1.058543] [<0000000000442a98>] pci_of_scan_bus+0x158/0xb00
>> [    1.058854] [<00000000010c74a0>] pci_scan_one_pbm+0xd0/0xf8
>> [    1.059167] [<0000000000446174>] sabre_probe+0x1f4/0x5c0
>> [    1.059476] [<0000000000c13a48>] platform_probe+0x28/0x80
>> [    1.059785] [<0000000000c11158>] really_probe+0xb8/0x340
>> [    1.060098] [<0000000000c11584>] driver_probe_device+0x24/0xe0
>> [    1.060413] [<0000000000c117ac>] __driver_attach+0x8c/0x160
>> [    1.060728] [<0000000000c0ef54>] bus_for_each_dev+0x54/0xc0
>>
>> The original call trace also included _raw_spin_lock_irqsave(), and
>> I don't have CONFIG_PREEMPT_RT enabled in my sparc64 builds to start with.
> 
> You don't have to. "CONFIG_PROVE_RAW_LOCK_NESTING" looks if you try to
> acquire raw_spinlock_t -> spinlock_t. Which it did before I made the
> patch.
> The pci_lock is from drivers/pci/access.c and is defined as
> raw_spinlock_t. And I made pci_poke_lock of the same time. But debug
> says 3:3 which suggests LD_WAIT_CONFIG. (No patch applied).
> 
>> FWIW, I don't understand the value of
>> 	pr_warn("context-{%d:%d}\n", curr_inner, curr_inner);
>> Why print curr_inner twice ?
> 
> The syntax was once (or is) inner:outer. If you look from the top, you
> have 4 (mutex_t) followed pci_lock (the raw_spinlock_t) 2. You are at
> level 2 now and try to acquire spin_lock_t (3).
> 

How does that explain the
	context-{5:5}
which is created from the following ?
	pr_warn("context-{%d:%d}\n", curr_inner, curr_inner);

Again, why print curr_inner twice ?

Thanks,
Guenter

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

[Sebastian Andrzej Siewior]

On 2024-11-25 09:59:09 [-0800], Guenter Roeck wrote:
> On 11/25/24 09:43, Sebastian Andrzej Siewior wrote:
> > On 2024-11-25 09:01:33 [-0800], Guenter Roeck wrote:
> > > Unfortunately it doesn't make a difference.
> > 
> > stunning. It looks like the exact same error message.
> > 
> 
> I think it uses
> 
> #define spin_lock_irqsave(lock, flags)                          \
> do {                                                            \
>         raw_spin_lock_irqsave(spinlock_check(lock), flags);     \
> } while (0)
> 
> from include/linux/spinlock.h, meaning your patch doesn't really make a difference.

The difference comes from DEFINE_SPINLOCK vs DEFINE_RAW_SPINLOCK. There
is the .lock_type init which goes from LD_WAIT_CONFIG to LD_WAIT_SPIN.
And this is all it matters.

> > > [    1.050499] =============================
> > > [    1.050801] [ BUG: Invalid wait context ]
> > > [    1.051200] 6.12.0+ #1 Not tainted
> > > [    1.051571] -----------------------------
> > > [    1.051875] swapper/0/1 is trying to lock:
> > > [    1.052201] 0000000001b694c8 (pci_poke_lock){....}-{3:3}, at: pci_config_read16+0x8/0x80
> > > [    1.052994] other info that might help us debug this:
> > > [    1.053331] context-{5:5}
> > > [    1.053641] 2 locks held by swapper/0/1:
> > > [    1.053959]  #0: fffff800042b50f8 (&dev->mutex){....}-{4:4}, at: __driver_attach+0x80/0x160
> > > [    1.054388]  #1: 0000000001d29078 (pci_lock){....}-{2:2}, at: pci_bus_read_config_word+0x18/0x80
> > > [    1.054793] stack backtrace:
> > > [    1.055171] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.0+ #1
> > > [    1.055632] Call Trace:
> > > [    1.055985] [<00000000004e31d0>] __lock_acquire+0xa50/0x3160
> > > [    1.056329] [<00000000004e63e8>] lock_acquire+0xe8/0x340
> > > [    1.056645] [<00000000010f0dfc>] _raw_spin_lock_irqsave+0x3c/0x80
> > > [    1.056966] [<0000000000443828>] pci_config_read16+0x8/0x80
> > > [    1.057278] [<000000000044442c>] sun4u_read_pci_cfg+0x12c/0x1a0
> > > [    1.057593] [<0000000000b7657c>] pci_bus_read_config_word+0x3c/0x80
> > > [    1.057913] [<0000000000b7fa78>] pci_find_capability+0x18/0xa0
> > > [    1.058228] [<0000000000b794b0>] set_pcie_port_type+0x10/0x160
> > > [    1.058543] [<0000000000442a98>] pci_of_scan_bus+0x158/0xb00
> > > [    1.058854] [<00000000010c74a0>] pci_scan_one_pbm+0xd0/0xf8
> > > [    1.059167] [<0000000000446174>] sabre_probe+0x1f4/0x5c0
> > > [    1.059476] [<0000000000c13a48>] platform_probe+0x28/0x80
> > > [    1.059785] [<0000000000c11158>] really_probe+0xb8/0x340
> > > [    1.060098] [<0000000000c11584>] driver_probe_device+0x24/0xe0
> > > [    1.060413] [<0000000000c117ac>] __driver_attach+0x8c/0x160
> > > [    1.060728] [<0000000000c0ef54>] bus_for_each_dev+0x54/0xc0
> > > 
> > > The original call trace also included _raw_spin_lock_irqsave(), and
> > > I don't have CONFIG_PREEMPT_RT enabled in my sparc64 builds to start with.
> > 
> > You don't have to. "CONFIG_PROVE_RAW_LOCK_NESTING" looks if you try to
> > acquire raw_spinlock_t -> spinlock_t. Which it did before I made the
> > patch.
> > The pci_lock is from drivers/pci/access.c and is defined as
> > raw_spinlock_t. And I made pci_poke_lock of the same time. But debug
> > says 3:3 which suggests LD_WAIT_CONFIG. (No patch applied).
> > 
> > > FWIW, I don't understand the value of
> > > 	pr_warn("context-{%d:%d}\n", curr_inner, curr_inner);
> > > Why print curr_inner twice ?
> > 
> > The syntax was once (or is) inner:outer. If you look from the top, you
> > have 4 (mutex_t) followed pci_lock (the raw_spinlock_t) 2. You are at
> > level 2 now and try to acquire spin_lock_t (3).
> > 
> 
> How does that explain the
> 	context-{5:5}

This is max value based on context. Your context is a simple process.
Not handling an interrupt or anything of this kind.
The culprit is 
|  swapper/0/1 is trying to lock:
|  0000000001b694c8 (pci_poke_lock){....}-{3:3}, at: pci_config_read16+0x8/0x80

where you have pci_poke_lock classified as a 3. The context allows a 5
so based on the context, the 3 would fly. But since pci_lock is a 2 we
have the splat here.

> which is created from the following ?
> 	pr_warn("context-{%d:%d}\n", curr_inner, curr_inner);
> 
> Again, why print curr_inner twice ?

It is the same syntax as in print_lock_name(). Except here, we don't
have an outer type. The difference is RCU because it has a lower type
than a spinlock_t and you can acquire a spinlock_t within an RCU
section and lockdep is fine with it. It comes yelling once you try this
with a mutex_t.

> Thanks,
> Guenter

Sebastian

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

[Guenter Roeck]

On 11/25/24 10:12, Sebastian Andrzej Siewior wrote:
> On 2024-11-25 09:59:09 [-0800], Guenter Roeck wrote:
>> On 11/25/24 09:43, Sebastian Andrzej Siewior wrote:
>>> On 2024-11-25 09:01:33 [-0800], Guenter Roeck wrote:
>>>> Unfortunately it doesn't make a difference.
>>>
>>> stunning. It looks like the exact same error message.
>>>
>>
>> I think it uses
>>
>> #define spin_lock_irqsave(lock, flags)                          \
>> do {                                                            \
>>          raw_spin_lock_irqsave(spinlock_check(lock), flags);     \
>> } while (0)
>>
>> from include/linux/spinlock.h, meaning your patch doesn't really make a difference.
> 
> The difference comes from DEFINE_SPINLOCK vs DEFINE_RAW_SPINLOCK. There
> is the .lock_type init which goes from LD_WAIT_CONFIG to LD_WAIT_SPIN.
> And this is all it matters.
> 

Ah, now I get it. Thanks for the explanation. And it turns out my log was wrong.
I must have taken it from the old image. Sorry for that.

That specific backtrace isn't seen anymore. But there is another one.

[    1.779653] =============================
[    1.779860] [ BUG: Invalid wait context ]
[    1.780139] 6.12.0+ #1 Not tainted
[    1.780394] -----------------------------
[    1.780600] swapper/0/1 is trying to lock:
[    1.780824] 0000000001b68888 (cpu_map_lock){....}-{3:3}, at: map_to_cpu+0x10/0x80
[    1.781393] other info that might help us debug this:
[    1.781624] context-{5:5}
[    1.781838] 3 locks held by swapper/0/1:
[    1.782055]  #0: fffff800042b90f8 (&dev->mutex){....}-{4:4}, at: __driver_attach+0x80/0x160
[    1.782345]  #1: fffff800040f2c18 (&desc->request_mutex){+.+.}-{4:4}, at: __setup_irq+0xa0/0x6e0
[    1.782632]  #2: fffff800040f2ab0 (&irq_desc_lock_class){....}-{2:2}, at: __setup_irq+0xc8/0x6e0
[    1.782912] stack backtrace:
[    1.783172] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.0+ #1
[    1.783498] Call Trace:
[    1.783734] [<00000000004e31d0>] __lock_acquire+0xa50/0x3160
[    1.783971] [<00000000004e63e8>] lock_acquire+0xe8/0x340
[    1.784191] [<00000000010f0dbc>] _raw_spin_lock_irqsave+0x3c/0x80
[    1.784417] [<000000000043ed90>] map_to_cpu+0x10/0x80
[    1.784633] [<000000000042b2b8>] sun4u_irq_enable+0x18/0x80
[    1.784854] [<00000000004fb6b4>] irq_enable+0x34/0xc0
[    1.785069] [<00000000004fb7b8>] __irq_startup+0x78/0xe0
[    1.785287] [<00000000004fb8f0>] irq_startup+0xd0/0x1a0
[    1.785503] [<00000000004f85b4>] __setup_irq+0x5f4/0x6e0
[    1.785726] [<00000000004f8754>] request_threaded_irq+0xb4/0x1a0
[    1.785950] [<0000000000439930>] power_probe+0x70/0xe0
[    1.786165] [<0000000000c13a68>] platform_probe+0x28/0x80
[    1.786382] [<0000000000c11178>] really_probe+0xb8/0x340
[    1.786599] [<0000000000c115a4>] driver_probe_device+0x24/0xe0
[    1.786820] [<0000000000c117cc>] __driver_attach+0x8c/0x160
[    1.787039] [<0000000000c0ef74>] bus_for_each_dev+0x54/0xc0

After replacing cpu_map_lock with a raw spinlock, I get:

[    2.015140] =============================
[    2.015247] [ BUG: Invalid wait context ]
[    2.015419] 6.12.0+ #1 Not tainted
[    2.015564] -----------------------------
[    2.015668] swapper/0/1 is trying to lock:
[    2.015791] fffff80004870610 (&mm->context.lock){....}-{3:3}, at: __schedule+0x410/0x5b0
[    2.016306] other info that might help us debug this:
[    2.016451] context-{5:5}
[    2.016539] 3 locks held by swapper/0/1:
[    2.016652]  #0: 0000000001d11f38 (key_types_sem){++++}-{4:4}, at: __key_create_or_update+0x5c/0x4c0
[    2.016934]  #1: 0000000001d1b850 (asymmetric_key_parsers_sem){++++}-{4:4}, at: asymmetric_key_preparse+0x18/0xa0
[    2.017197]  #2: fffff8001f811a98 (&rq->__lock){-.-.}-{2:2}, at: __schedule+0xdc/0x5b0
[    2.017412] stack backtrace:
[    2.017551] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.0+ #1
[    2.017800] Call Trace:
[    2.017910] [<00000000004e31d0>] __lock_acquire+0xa50/0x3160
[    2.018062] [<00000000004e63e8>] lock_acquire+0xe8/0x340
[    2.018192] [<00000000010f0dbc>] _raw_spin_lock_irqsave+0x3c/0x80
[    2.018341] [<00000000010e5050>] __schedule+0x410/0x5b0
[    2.018469] [<00000000010e5ae4>] schedule+0x44/0x1c0
[    2.018591] [<00000000010f0684>] schedule_timeout+0xa4/0x100
[    2.018730] [<00000000010e668c>] __wait_for_common+0xac/0x1a0
[    2.018869] [<00000000010e6878>] wait_for_completion_state+0x18/0x40
[    2.019022] [<000000000048ad18>] call_usermodehelper_exec+0x138/0x1c0
[    2.019177] [<000000000052eb40>] __request_module+0x160/0x2e0
[    2.019316] [<00000000009ba6dc>] crypto_alg_mod_lookup+0x17c/0x280
[    2.019466] [<00000000009ba990>] crypto_alloc_tfm_node+0x30/0x100
[    2.019614] [<00000000009dcc5c>] public_key_verify_signature+0xbc/0x260
[    2.019772] [<00000000009ded8c>] x509_check_for_self_signed+0xac/0x280
[    2.019928] [<00000000009dddec>] x509_cert_parse+0x14c/0x220
[    2.020065] [<00000000009dea08>] x509_key_preparse+0x8/0x1e0

The problem here is

typedef struct {
         spinlock_t              lock;		<--
         unsigned long           sparc64_ctx_val;
         unsigned long           hugetlb_pte_count;
         unsigned long           thp_pte_count;
         struct tsb_config       tsb_block[MM_NUM_TSBS];
         struct hv_tsb_descr     tsb_descr[MM_NUM_TSBS];
         void                    *vdso;
         bool                    adi;
         tag_storage_desc_t      *tag_store;
         spinlock_t              tag_lock;
} mm_context_t;

Replacing that with a raw spinlock just triggers the next one.

[    2.035384] =============================
[    2.035490] [ BUG: Invalid wait context ]
[    2.035660] 6.12.0+ #3 Not tainted
[    2.035802] -----------------------------
[    2.035906] kworker/u4:3/48 is trying to lock:
[    2.036036] 0000000001b6a790 (ctx_alloc_lock){....}-{3:3}, at: get_new_mmu_context+0x14/0x280
[    2.036558] other info that might help us debug this:
[    2.036697] context-{5:5}
[    2.036784] 4 locks held by kworker/u4:3/48:
[    2.036906]  #0: fffff80004838a70 (&sig->cred_guard_mutex){+.+.}-{4:4}, at: bprm_execve+0xc/0x8e0
[    2.037169]  #1: fffff80004838b08 (&sig->exec_update_lock){+.+.}-{4:4}, at: begin_new_exec+0x344/0xbe0
[    2.037411]  #2: fffff800047fc940 (&p->alloc_lock){+.+.}-{3:3}, at: begin_new_exec+0x3a0/0xbe0
[    2.037639]  #3: fffff80004848610 (&mm->context.lock){....}-{2:2}, at: begin_new_exec+0x41c/0xbe0

Fixing that finally gives me a clean run. Nevertheless, that makes me wonder:
Should I just disable CONFIG_PROVE_RAW_LOCK_NESTING for sparc runtime tests ?

Thanks,
Guenter

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

[Waiman Long]

On 11/25/24 2:23 PM, Guenter Roeck wrote:
> On 11/25/24 10:12, Sebastian Andrzej Siewior wrote:
>> On 2024-11-25 09:59:09 [-0800], Guenter Roeck wrote:
>>> On 11/25/24 09:43, Sebastian Andrzej Siewior wrote:
>>>> On 2024-11-25 09:01:33 [-0800], Guenter Roeck wrote:
>>>>> Unfortunately it doesn't make a difference.
>>>>
>>>> stunning. It looks like the exact same error message.
>>>>
>>>
>>> I think it uses
>>>
>>> #define spin_lock_irqsave(lock, flags)                          \
>>> do {                                                            \
>>>          raw_spin_lock_irqsave(spinlock_check(lock), flags);     \
>>> } while (0)
>>>
>>> from include/linux/spinlock.h, meaning your patch doesn't really 
>>> make a difference.
>>
>> The difference comes from DEFINE_SPINLOCK vs DEFINE_RAW_SPINLOCK. There
>> is the .lock_type init which goes from LD_WAIT_CONFIG to LD_WAIT_SPIN.
>> And this is all it matters.
>>
>
> Ah, now I get it. Thanks for the explanation. And it turns out my log 
> was wrong.
> I must have taken it from the old image. Sorry for that.
>
> That specific backtrace isn't seen anymore. But there is another one.
>
> [    1.779653] =============================
> [    1.779860] [ BUG: Invalid wait context ]
> [    1.780139] 6.12.0+ #1 Not tainted
> [    1.780394] -----------------------------
> [    1.780600] swapper/0/1 is trying to lock:
> [    1.780824] 0000000001b68888 (cpu_map_lock){....}-{3:3}, at: 
> map_to_cpu+0x10/0x80
> [    1.781393] other info that might help us debug this:
> [    1.781624] context-{5:5}
> [    1.781838] 3 locks held by swapper/0/1:
> [    1.782055]  #0: fffff800042b90f8 (&dev->mutex){....}-{4:4}, at: 
> __driver_attach+0x80/0x160
> [    1.782345]  #1: fffff800040f2c18 
> (&desc->request_mutex){+.+.}-{4:4}, at: __setup_irq+0xa0/0x6e0
> [    1.782632]  #2: fffff800040f2ab0 
> (&irq_desc_lock_class){....}-{2:2}, at: __setup_irq+0xc8/0x6e0
> [    1.782912] stack backtrace:
> [    1.783172] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 
> 6.12.0+ #1
> [    1.783498] Call Trace:
> [    1.783734] [<00000000004e31d0>] __lock_acquire+0xa50/0x3160
> [    1.783971] [<00000000004e63e8>] lock_acquire+0xe8/0x340
> [    1.784191] [<00000000010f0dbc>] _raw_spin_lock_irqsave+0x3c/0x80
> [    1.784417] [<000000000043ed90>] map_to_cpu+0x10/0x80
> [    1.784633] [<000000000042b2b8>] sun4u_irq_enable+0x18/0x80
> [    1.784854] [<00000000004fb6b4>] irq_enable+0x34/0xc0
> [    1.785069] [<00000000004fb7b8>] __irq_startup+0x78/0xe0
> [    1.785287] [<00000000004fb8f0>] irq_startup+0xd0/0x1a0
> [    1.785503] [<00000000004f85b4>] __setup_irq+0x5f4/0x6e0
> [    1.785726] [<00000000004f8754>] request_threaded_irq+0xb4/0x1a0
> [    1.785950] [<0000000000439930>] power_probe+0x70/0xe0
> [    1.786165] [<0000000000c13a68>] platform_probe+0x28/0x80
> [    1.786382] [<0000000000c11178>] really_probe+0xb8/0x340
> [    1.786599] [<0000000000c115a4>] driver_probe_device+0x24/0xe0
> [    1.786820] [<0000000000c117cc>] __driver_attach+0x8c/0x160
> [    1.787039] [<0000000000c0ef74>] bus_for_each_dev+0x54/0xc0
>
> After replacing cpu_map_lock with a raw spinlock, I get:
>
> [    2.015140] =============================
> [    2.015247] [ BUG: Invalid wait context ]
> [    2.015419] 6.12.0+ #1 Not tainted
> [    2.015564] -----------------------------
> [    2.015668] swapper/0/1 is trying to lock:
> [    2.015791] fffff80004870610 (&mm->context.lock){....}-{3:3}, at: 
> __schedule+0x410/0x5b0
> [    2.016306] other info that might help us debug this:
> [    2.016451] context-{5:5}
> [    2.016539] 3 locks held by swapper/0/1:
> [    2.016652]  #0: 0000000001d11f38 (key_types_sem){++++}-{4:4}, at: 
> __key_create_or_update+0x5c/0x4c0
> [    2.016934]  #1: 0000000001d1b850 
> (asymmetric_key_parsers_sem){++++}-{4:4}, at: 
> asymmetric_key_preparse+0x18/0xa0
> [    2.017197]  #2: fffff8001f811a98 (&rq->__lock){-.-.}-{2:2}, at: 
> __schedule+0xdc/0x5b0
> [    2.017412] stack backtrace:
> [    2.017551] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 
> 6.12.0+ #1
> [    2.017800] Call Trace:
> [    2.017910] [<00000000004e31d0>] __lock_acquire+0xa50/0x3160
> [    2.018062] [<00000000004e63e8>] lock_acquire+0xe8/0x340
> [    2.018192] [<00000000010f0dbc>] _raw_spin_lock_irqsave+0x3c/0x80
> [    2.018341] [<00000000010e5050>] __schedule+0x410/0x5b0
> [    2.018469] [<00000000010e5ae4>] schedule+0x44/0x1c0
> [    2.018591] [<00000000010f0684>] schedule_timeout+0xa4/0x100
> [    2.018730] [<00000000010e668c>] __wait_for_common+0xac/0x1a0
> [    2.018869] [<00000000010e6878>] wait_for_completion_state+0x18/0x40
> [    2.019022] [<000000000048ad18>] call_usermodehelper_exec+0x138/0x1c0
> [    2.019177] [<000000000052eb40>] __request_module+0x160/0x2e0
> [    2.019316] [<00000000009ba6dc>] crypto_alg_mod_lookup+0x17c/0x280
> [    2.019466] [<00000000009ba990>] crypto_alloc_tfm_node+0x30/0x100
> [    2.019614] [<00000000009dcc5c>] 
> public_key_verify_signature+0xbc/0x260
> [    2.019772] [<00000000009ded8c>] x509_check_for_self_signed+0xac/0x280
> [    2.019928] [<00000000009dddec>] x509_cert_parse+0x14c/0x220
> [    2.020065] [<00000000009dea08>] x509_key_preparse+0x8/0x1e0
>
> The problem here is
>
> typedef struct {
>         spinlock_t              lock;        <--
>         unsigned long           sparc64_ctx_val;
>         unsigned long           hugetlb_pte_count;
>         unsigned long           thp_pte_count;
>         struct tsb_config       tsb_block[MM_NUM_TSBS];
>         struct hv_tsb_descr     tsb_descr[MM_NUM_TSBS];
>         void                    *vdso;
>         bool                    adi;
>         tag_storage_desc_t      *tag_store;
>         spinlock_t              tag_lock;
> } mm_context_t;
>
> Replacing that with a raw spinlock just triggers the next one.
>
> [    2.035384] =============================
> [    2.035490] [ BUG: Invalid wait context ]
> [    2.035660] 6.12.0+ #3 Not tainted
> [    2.035802] -----------------------------
> [    2.035906] kworker/u4:3/48 is trying to lock:
> [    2.036036] 0000000001b6a790 (ctx_alloc_lock){....}-{3:3}, at: 
> get_new_mmu_context+0x14/0x280
> [    2.036558] other info that might help us debug this:
> [    2.036697] context-{5:5}
> [    2.036784] 4 locks held by kworker/u4:3/48:
> [    2.036906]  #0: fffff80004838a70 
> (&sig->cred_guard_mutex){+.+.}-{4:4}, at: bprm_execve+0xc/0x8e0
> [    2.037169]  #1: fffff80004838b08 
> (&sig->exec_update_lock){+.+.}-{4:4}, at: begin_new_exec+0x344/0xbe0
> [    2.037411]  #2: fffff800047fc940 (&p->alloc_lock){+.+.}-{3:3}, at: 
> begin_new_exec+0x3a0/0xbe0
> [    2.037639]  #3: fffff80004848610 (&mm->context.lock){....}-{2:2}, 
> at: begin_new_exec+0x41c/0xbe0
>
> Fixing that finally gives me a clean run. Nevertheless, that makes me 
> wonder:
> Should I just disable CONFIG_PROVE_RAW_LOCK_NESTING for sparc runtime 
> tests ?

If no one is tryng to ever enable PREEMPT_RT on SPARC, I suppose you 
could disable CONFIG_PROVE_RAW_LOCK_NESTING to avoid the trouble.

Cheers,
Longman

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

[Guenter Roeck]

On 11/25/24 11:33, Waiman Long wrote:
[ ... ]
>> Fixing that finally gives me a clean run. Nevertheless, that makes me wonder:
>> Should I just disable CONFIG_PROVE_RAW_LOCK_NESTING for sparc runtime tests ?
> 
> If no one is tryng to ever enable PREEMPT_RT on SPARC, I suppose you could disable CONFIG_PROVE_RAW_LOCK_NESTING to avoid the trouble.
> 

SGTM. I'll do that unless someone gives me a good reason to keep it enabled.

Thanks,
Guenter

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

[Guenter Roeck]

On 11/25/24 12:06, Guenter Roeck wrote:
> On 11/25/24 11:33, Waiman Long wrote:
> [ ... ]
>>> Fixing that finally gives me a clean run. Nevertheless, that makes me wonder:
>>> Should I just disable CONFIG_PROVE_RAW_LOCK_NESTING for sparc runtime tests ?
>>
>> If no one is tryng to ever enable PREEMPT_RT on SPARC, I suppose you could disable CONFIG_PROVE_RAW_LOCK_NESTING to avoid the trouble.
>>
> 
> SGTM. I'll do that unless someone gives me a good reason to keep it enabled.
> 

Actually it can not be disabled with a configuration flag. It is
automatically enabled. I'll have to disable PROVE_LOCKING to disable it.

config PROVE_RAW_LOCK_NESTING
         bool					<---- no longer user configurable
         depends on PROVE_LOCKING
         default y
         help
          Enable the raw_spinlock vs. spinlock nesting checks which ensure
          that the lock nesting rules for PREEMPT_RT enabled kernels are
          not violated.

I don't really like that, and I don't understand the logic behind it,
but it is what it is.

FWIW, the description of commit 560af5dc839 is misleading. It says "Enable
PROVE_RAW_LOCK_NESTING _by default_" (emphasis mine). That is not what the
commit does. It force-enables PROVE_RAW_LOCK_NESTING if PROVE_LOCKING is
enabled. It is all or nothing.

Guenter

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

[Waiman Long]

On 11/25/24 3:23 PM, Guenter Roeck wrote:
> On 11/25/24 12:06, Guenter Roeck wrote:
>> On 11/25/24 11:33, Waiman Long wrote:
>> [ ... ]
>>>> Fixing that finally gives me a clean run. Nevertheless, that makes 
>>>> me wonder:
>>>> Should I just disable CONFIG_PROVE_RAW_LOCK_NESTING for sparc 
>>>> runtime tests ?
>>>
>>> If no one is tryng to ever enable PREEMPT_RT on SPARC, I suppose you 
>>> could disable CONFIG_PROVE_RAW_LOCK_NESTING to avoid the trouble.
>>>
>>
>> SGTM. I'll do that unless someone gives me a good reason to keep it 
>> enabled.
>>
>
> Actually it can not be disabled with a configuration flag. It is
> automatically enabled. I'll have to disable PROVE_LOCKING to disable it.
>
> config PROVE_RAW_LOCK_NESTING
>         bool                    <---- no longer user configurable
>         depends on PROVE_LOCKING
>         default y
>         help
>          Enable the raw_spinlock vs. spinlock nesting checks which ensure
>          that the lock nesting rules for PREEMPT_RT enabled kernels are
>          not violated.
>
> I don't really like that, and I don't understand the logic behind it,
> but it is what it is.
>
> FWIW, the description of commit 560af5dc839 is misleading. It says 
> "Enable
> PROVE_RAW_LOCK_NESTING _by default_" (emphasis mine). That is not what 
> the
> commit does. It force-enables PROVE_RAW_LOCK_NESTING if PROVE_LOCKING is
> enabled. It is all or nothing.
>
I think we can relax it by

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 5d9eca035d47..bfdbd3fa2d29 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1399,7 +1399,7 @@ config PROVE_LOCKING
  config PROVE_RAW_LOCK_NESTING
         bool
         depends on PROVE_LOCKING
-       default y
+       default y if ARCH_SUPPORTS_RT
         help
          Enable the raw_spinlock vs. spinlock nesting checks which ensure
          that the lock nesting rules for PREEMPT_RT enabled kernels are

Sebastian, what do you think?

Cheers,
Longman

> Guenter
>

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

[Guenter Roeck]

On 11/25/24 12:54, Waiman Long wrote:
> 
> On 11/25/24 3:23 PM, Guenter Roeck wrote:
>> On 11/25/24 12:06, Guenter Roeck wrote:
>>> On 11/25/24 11:33, Waiman Long wrote:
>>> [ ... ]
>>>>> Fixing that finally gives me a clean run. Nevertheless, that makes me wonder:
>>>>> Should I just disable CONFIG_PROVE_RAW_LOCK_NESTING for sparc runtime tests ?
>>>>
>>>> If no one is tryng to ever enable PREEMPT_RT on SPARC, I suppose you could disable CONFIG_PROVE_RAW_LOCK_NESTING to avoid the trouble.
>>>>
>>>
>>> SGTM. I'll do that unless someone gives me a good reason to keep it enabled.
>>>
>>
>> Actually it can not be disabled with a configuration flag. It is
>> automatically enabled. I'll have to disable PROVE_LOCKING to disable it.
>>
>> config PROVE_RAW_LOCK_NESTING
>>         bool                    <---- no longer user configurable
>>         depends on PROVE_LOCKING
>>         default y
>>         help
>>          Enable the raw_spinlock vs. spinlock nesting checks which ensure
>>          that the lock nesting rules for PREEMPT_RT enabled kernels are
>>          not violated.
>>
>> I don't really like that, and I don't understand the logic behind it,
>> but it is what it is.
>>
>> FWIW, the description of commit 560af5dc839 is misleading. It says "Enable
>> PROVE_RAW_LOCK_NESTING _by default_" (emphasis mine). That is not what the
>> commit does. It force-enables PROVE_RAW_LOCK_NESTING if PROVE_LOCKING is
>> enabled. It is all or nothing.
>>
> I think we can relax it by
> 
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 5d9eca035d47..bfdbd3fa2d29 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1399,7 +1399,7 @@ config PROVE_LOCKING
>   config PROVE_RAW_LOCK_NESTING
>          bool
>          depends on PROVE_LOCKING
> -       default y
> +       default y if ARCH_SUPPORTS_RT
>          help
>           Enable the raw_spinlock vs. spinlock nesting checks which ensure
>           that the lock nesting rules for PREEMPT_RT enabled kernels are
> 
> Sebastian, what do you think?
> 

	depends on PROVE_LOCKING && ARCH_SUPPORTS_RT

seems to make more sense to me.

Guenter

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

[Waiman Long]

On 11/25/24 4:25 PM, Guenter Roeck wrote:
> On 11/25/24 12:54, Waiman Long wrote:
>>
>> On 11/25/24 3:23 PM, Guenter Roeck wrote:
>>> On 11/25/24 12:06, Guenter Roeck wrote:
>>>> On 11/25/24 11:33, Waiman Long wrote:
>>>> [ ... ]
>>>>>> Fixing that finally gives me a clean run. Nevertheless, that 
>>>>>> makes me wonder:
>>>>>> Should I just disable CONFIG_PROVE_RAW_LOCK_NESTING for sparc 
>>>>>> runtime tests ?
>>>>>
>>>>> If no one is tryng to ever enable PREEMPT_RT on SPARC, I suppose 
>>>>> you could disable CONFIG_PROVE_RAW_LOCK_NESTING to avoid the trouble.
>>>>>
>>>>
>>>> SGTM. I'll do that unless someone gives me a good reason to keep it 
>>>> enabled.
>>>>
>>>
>>> Actually it can not be disabled with a configuration flag. It is
>>> automatically enabled. I'll have to disable PROVE_LOCKING to disable 
>>> it.
>>>
>>> config PROVE_RAW_LOCK_NESTING
>>>         bool                    <---- no longer user configurable
>>>         depends on PROVE_LOCKING
>>>         default y
>>>         help
>>>          Enable the raw_spinlock vs. spinlock nesting checks which 
>>> ensure
>>>          that the lock nesting rules for PREEMPT_RT enabled kernels are
>>>          not violated.
>>>
>>> I don't really like that, and I don't understand the logic behind it,
>>> but it is what it is.
>>>
>>> FWIW, the description of commit 560af5dc839 is misleading. It says 
>>> "Enable
>>> PROVE_RAW_LOCK_NESTING _by default_" (emphasis mine). That is not 
>>> what the
>>> commit does. It force-enables PROVE_RAW_LOCK_NESTING if 
>>> PROVE_LOCKING is
>>> enabled. It is all or nothing.
>>>
>> I think we can relax it by
>>
>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>> index 5d9eca035d47..bfdbd3fa2d29 100644
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -1399,7 +1399,7 @@ config PROVE_LOCKING
>>   config PROVE_RAW_LOCK_NESTING
>>          bool
>>          depends on PROVE_LOCKING
>> -       default y
>> +       default y if ARCH_SUPPORTS_RT
>>          help
>>           Enable the raw_spinlock vs. spinlock nesting checks which 
>> ensure
>>           that the lock nesting rules for PREEMPT_RT enabled kernels are
>>
>> Sebastian, what do you think?
>>
>
>     depends on PROVE_LOCKING && ARCH_SUPPORTS_RT
>
> seems to make more sense to me.

That will work too, but that will enforce that arches with no 
ARCH_SUPPORTS_RT will not be able to enable PROVE_RAW_LOCK_NESTING even 
if people want to try it out.

Cheers,
Longman

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

[Guenter Roeck]

On 11/25/24 13:29, Waiman Long wrote:
> 
> On 11/25/24 4:25 PM, Guenter Roeck wrote:
>> On 11/25/24 12:54, Waiman Long wrote:
>>>
>>> On 11/25/24 3:23 PM, Guenter Roeck wrote:
>>>> On 11/25/24 12:06, Guenter Roeck wrote:
>>>>> On 11/25/24 11:33, Waiman Long wrote:
>>>>> [ ... ]
>>>>>>> Fixing that finally gives me a clean run. Nevertheless, that makes me wonder:
>>>>>>> Should I just disable CONFIG_PROVE_RAW_LOCK_NESTING for sparc runtime tests ?
>>>>>>
>>>>>> If no one is tryng to ever enable PREEMPT_RT on SPARC, I suppose you could disable CONFIG_PROVE_RAW_LOCK_NESTING to avoid the trouble.
>>>>>>
>>>>>
>>>>> SGTM. I'll do that unless someone gives me a good reason to keep it enabled.
>>>>>
>>>>
>>>> Actually it can not be disabled with a configuration flag. It is
>>>> automatically enabled. I'll have to disable PROVE_LOCKING to disable it.
>>>>
>>>> config PROVE_RAW_LOCK_NESTING
>>>>         bool                    <---- no longer user configurable
>>>>         depends on PROVE_LOCKING
>>>>         default y
>>>>         help
>>>>          Enable the raw_spinlock vs. spinlock nesting checks which ensure
>>>>          that the lock nesting rules for PREEMPT_RT enabled kernels are
>>>>          not violated.
>>>>
>>>> I don't really like that, and I don't understand the logic behind it,
>>>> but it is what it is.
>>>>
>>>> FWIW, the description of commit 560af5dc839 is misleading. It says "Enable
>>>> PROVE_RAW_LOCK_NESTING _by default_" (emphasis mine). That is not what the
>>>> commit does. It force-enables PROVE_RAW_LOCK_NESTING if PROVE_LOCKING is
>>>> enabled. It is all or nothing.
>>>>
>>> I think we can relax it by
>>>
>>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>>> index 5d9eca035d47..bfdbd3fa2d29 100644
>>> --- a/lib/Kconfig.debug
>>> +++ b/lib/Kconfig.debug
>>> @@ -1399,7 +1399,7 @@ config PROVE_LOCKING
>>>   config PROVE_RAW_LOCK_NESTING
>>>          bool
>>>          depends on PROVE_LOCKING
>>> -       default y
>>> +       default y if ARCH_SUPPORTS_RT
>>>          help
>>>           Enable the raw_spinlock vs. spinlock nesting checks which ensure
>>>           that the lock nesting rules for PREEMPT_RT enabled kernels are
>>>
>>> Sebastian, what do you think?
>>>
>>
>>     depends on PROVE_LOCKING && ARCH_SUPPORTS_RT
>>
>> seems to make more sense to me.
> 
> That will work too, but that will enforce that arches with no ARCH_SUPPORTS_RT will not be able to enable PROVE_RAW_LOCK_NESTING even if people want to try it out.
> 

No architecture will be able to enable anything because "bool" has no
string associated with it. As mentioned before, it is all or nothing.
Otherwise I could just configure "CONFIG_PROVE_RAW_LOCK_NESTING=n"
for sparc and be done.

Guenter

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

[Waiman Long]

On 11/25/24 4:54 PM, Guenter Roeck wrote:
> On 11/25/24 13:29, Waiman Long wrote:
>>
>> On 11/25/24 4:25 PM, Guenter Roeck wrote:
>>> On 11/25/24 12:54, Waiman Long wrote:
>>>>
>>>> On 11/25/24 3:23 PM, Guenter Roeck wrote:
>>>>> On 11/25/24 12:06, Guenter Roeck wrote:
>>>>>> On 11/25/24 11:33, Waiman Long wrote:
>>>>>> [ ... ]
>>>>>>>> Fixing that finally gives me a clean run. Nevertheless, that 
>>>>>>>> makes me wonder:
>>>>>>>> Should I just disable CONFIG_PROVE_RAW_LOCK_NESTING for sparc 
>>>>>>>> runtime tests ?
>>>>>>>
>>>>>>> If no one is tryng to ever enable PREEMPT_RT on SPARC, I suppose 
>>>>>>> you could disable CONFIG_PROVE_RAW_LOCK_NESTING to avoid the 
>>>>>>> trouble.
>>>>>>>
>>>>>>
>>>>>> SGTM. I'll do that unless someone gives me a good reason to keep 
>>>>>> it enabled.
>>>>>>
>>>>>
>>>>> Actually it can not be disabled with a configuration flag. It is
>>>>> automatically enabled. I'll have to disable PROVE_LOCKING to 
>>>>> disable it.
>>>>>
>>>>> config PROVE_RAW_LOCK_NESTING
>>>>>         bool                    <---- no longer user configurable
>>>>>         depends on PROVE_LOCKING
>>>>>         default y
>>>>>         help
>>>>>          Enable the raw_spinlock vs. spinlock nesting checks which 
>>>>> ensure
>>>>>          that the lock nesting rules for PREEMPT_RT enabled 
>>>>> kernels are
>>>>>          not violated.
>>>>>
>>>>> I don't really like that, and I don't understand the logic behind it,
>>>>> but it is what it is.
>>>>>
>>>>> FWIW, the description of commit 560af5dc839 is misleading. It says 
>>>>> "Enable
>>>>> PROVE_RAW_LOCK_NESTING _by default_" (emphasis mine). That is not 
>>>>> what the
>>>>> commit does. It force-enables PROVE_RAW_LOCK_NESTING if 
>>>>> PROVE_LOCKING is
>>>>> enabled. It is all or nothing.
>>>>>
>>>> I think we can relax it by
>>>>
>>>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>>>> index 5d9eca035d47..bfdbd3fa2d29 100644
>>>> --- a/lib/Kconfig.debug
>>>> +++ b/lib/Kconfig.debug
>>>> @@ -1399,7 +1399,7 @@ config PROVE_LOCKING
>>>>   config PROVE_RAW_LOCK_NESTING
>>>>          bool
>>>>          depends on PROVE_LOCKING
>>>> -       default y
>>>> +       default y if ARCH_SUPPORTS_RT
>>>>          help
>>>>           Enable the raw_spinlock vs. spinlock nesting checks which 
>>>> ensure
>>>>           that the lock nesting rules for PREEMPT_RT enabled 
>>>> kernels are
>>>>
>>>> Sebastian, what do you think?
>>>>
>>>
>>>     depends on PROVE_LOCKING && ARCH_SUPPORTS_RT
>>>
>>> seems to make more sense to me.
>>
>> That will work too, but that will enforce that arches with no 
>> ARCH_SUPPORTS_RT will not be able to enable PROVE_RAW_LOCK_NESTING 
>> even if people want to try it out.
>>
>
> No architecture will be able to enable anything because "bool" has no
> string associated with it. As mentioned before, it is all or nothing.
> Otherwise I could just configure "CONFIG_PROVE_RAW_LOCK_NESTING=n"
> for sparc and be done.

Yes, you are right.

Cheers,
Longman

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

[Sebastian Andrzej Siewior]

On 2024-11-25 15:54:48 [-0500], Waiman Long wrote:
> > FWIW, the description of commit 560af5dc839 is misleading. It says
> > "Enable
> > PROVE_RAW_LOCK_NESTING _by default_" (emphasis mine). That is not what
> > the
> > commit does. It force-enables PROVE_RAW_LOCK_NESTING if PROVE_LOCKING is
> > enabled. It is all or nothing.
> > 
> I think we can relax it by
> 
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 5d9eca035d47..bfdbd3fa2d29 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1399,7 +1399,7 @@ config PROVE_LOCKING
>  config PROVE_RAW_LOCK_NESTING
>         bool
>         depends on PROVE_LOCKING
> -       default y
> +       default y if ARCH_SUPPORTS_RT
>         help
>          Enable the raw_spinlock vs. spinlock nesting checks which ensure
>          that the lock nesting rules for PREEMPT_RT enabled kernels are
> 
> Sebastian, what do you think?

All the changes Guenter proposed make sense and were limited to sparc.
So we could apply that. Limiting the option to the RT architectures
would silence the warnings. If there is no interest in getting RT on
sparc there is probably no interest in getting the lock ordering
straight.
I remember PeterZ did not like the option in the beginning but there was
no way around it especially since printk triggered it on boot.
I'm fine with both solutions (fixing sparc or limiting
PROVE_RAW_LOCK_NESTING). I leave the final judgment to the locking
people.

> Cheers,
> Longman

Sebastian

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

[Waiman Long]

On 11/26/24 6:20 AM, Sebastian Andrzej Siewior wrote:
> On 2024-11-25 15:54:48 [-0500], Waiman Long wrote:
>>> FWIW, the description of commit 560af5dc839 is misleading. It says
>>> "Enable
>>> PROVE_RAW_LOCK_NESTING _by default_" (emphasis mine). That is not what
>>> the
>>> commit does. It force-enables PROVE_RAW_LOCK_NESTING if PROVE_LOCKING is
>>> enabled. It is all or nothing.
>>>
>> I think we can relax it by
>>
>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>> index 5d9eca035d47..bfdbd3fa2d29 100644
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -1399,7 +1399,7 @@ config PROVE_LOCKING
>>   config PROVE_RAW_LOCK_NESTING
>>          bool
>>          depends on PROVE_LOCKING
>> -       default y
>> +       default y if ARCH_SUPPORTS_RT
>>          help
>>           Enable the raw_spinlock vs. spinlock nesting checks which ensure
>>           that the lock nesting rules for PREEMPT_RT enabled kernels are
>>
>> Sebastian, what do you think?
> All the changes Guenter proposed make sense and were limited to sparc.
> So we could apply that. Limiting the option to the RT architectures
> would silence the warnings. If there is no interest in getting RT on
> sparc there is probably no interest in getting the lock ordering
> straight.
> I remember PeterZ did not like the option in the beginning but there was
> no way around it especially since printk triggered it on boot.
> I'm fine with both solutions (fixing sparc or limiting
> PROVE_RAW_LOCK_NESTING). I leave the final judgment to the locking
> people.

Right now, ARCH_SUPPORTS_RT is defined for most of the major arches 
where most of the testings are being done. So even if we limit this to 
just those arches, we will not lose much testing anyway. This does have 
the advantage of not forcing other legacy arches from doing extra works 
with no real gain from their point of view.

Cheers,
Longman

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

[Andreas Larsson]

On 2024-11-26 17:59, Waiman Long wrote:
> 
> On 11/26/24 6:20 AM, Sebastian Andrzej Siewior wrote:
>> On 2024-11-25 15:54:48 [-0500], Waiman Long wrote:
>>>> FWIW, the description of commit 560af5dc839 is misleading. It says
>>>> "Enable
>>>> PROVE_RAW_LOCK_NESTING _by default_" (emphasis mine). That is not what
>>>> the
>>>> commit does. It force-enables PROVE_RAW_LOCK_NESTING if PROVE_LOCKING is
>>>> enabled. It is all or nothing.
>>>>
>>> I think we can relax it by
>>>
>>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>>> index 5d9eca035d47..bfdbd3fa2d29 100644
>>> --- a/lib/Kconfig.debug
>>> +++ b/lib/Kconfig.debug
>>> @@ -1399,7 +1399,7 @@ config PROVE_LOCKING
>>>   config PROVE_RAW_LOCK_NESTING
>>>          bool
>>>          depends on PROVE_LOCKING
>>> -       default y
>>> +       default y if ARCH_SUPPORTS_RT
>>>          help
>>>           Enable the raw_spinlock vs. spinlock nesting checks which ensure
>>>           that the lock nesting rules for PREEMPT_RT enabled kernels are
>>>
>>> Sebastian, what do you think?
>> All the changes Guenter proposed make sense and were limited to sparc.
>> So we could apply that. Limiting the option to the RT architectures
>> would silence the warnings. If there is no interest in getting RT on
>> sparc there is probably no interest in getting the lock ordering
>> straight.
>> I remember PeterZ did not like the option in the beginning but there was
>> no way around it especially since printk triggered it on boot.
>> I'm fine with both solutions (fixing sparc or limiting
>> PROVE_RAW_LOCK_NESTING). I leave the final judgment to the locking
>> people.
> 
> Right now, ARCH_SUPPORTS_RT is defined for most of the major arches
> where most of the testings are being done. So even if we limit this to
> just those arches, we will not lose much testing anyway. This does have
> the advantage of not forcing other legacy arches from doing extra works
> with no real gain from their point of view.

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.

I don't know what the situation is for other architectures that does not
support RT.

Cheers,
Andreas

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

[Guenter Roeck]

On 11/27/24 07:39, Andreas Larsson wrote:
> On 2024-11-26 17:59, Waiman Long wrote:
>>
>> On 11/26/24 6:20 AM, Sebastian Andrzej Siewior wrote:
>>> On 2024-11-25 15:54:48 [-0500], Waiman Long wrote:
>>>>> FWIW, the description of commit 560af5dc839 is misleading. It says
>>>>> "Enable
>>>>> PROVE_RAW_LOCK_NESTING _by default_" (emphasis mine). That is not what
>>>>> the
>>>>> commit does. It force-enables PROVE_RAW_LOCK_NESTING if PROVE_LOCKING is
>>>>> enabled. It is all or nothing.
>>>>>
>>>> I think we can relax it by
>>>>
>>>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>>>> index 5d9eca035d47..bfdbd3fa2d29 100644
>>>> --- a/lib/Kconfig.debug
>>>> +++ b/lib/Kconfig.debug
>>>> @@ -1399,7 +1399,7 @@ config PROVE_LOCKING
>>>>    config PROVE_RAW_LOCK_NESTING
>>>>           bool
>>>>           depends on PROVE_LOCKING
>>>> -       default y
>>>> +       default y if ARCH_SUPPORTS_RT
>>>>           help
>>>>            Enable the raw_spinlock vs. spinlock nesting checks which ensure
>>>>            that the lock nesting rules for PREEMPT_RT enabled kernels are
>>>>
>>>> Sebastian, what do you think?
>>> All the changes Guenter proposed make sense and were limited to sparc.
>>> So we could apply that. Limiting the option to the RT architectures
>>> would silence the warnings. If there is no interest in getting RT on
>>> sparc there is probably no interest in getting the lock ordering
>>> straight.
>>> I remember PeterZ did not like the option in the beginning but there was
>>> no way around it especially since printk triggered it on boot.
>>> I'm fine with both solutions (fixing sparc or limiting
>>> PROVE_RAW_LOCK_NESTING). I leave the final judgment to the locking
>>> people.
>>
>> Right now, ARCH_SUPPORTS_RT is defined for most of the major arches
>> where most of the testings are being done. So even if we limit this to
>> just those arches, we will not lose much testing anyway. This does have
>> the advantage of not forcing other legacy arches from doing extra works
>> with no real gain from their point of view.
> 
> 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.
> 
> 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.

So, if people want to keep mandating PROVE_RAW_LOCK_NESTING together with
PROVE_LOCKING for all architectures, I'll disable PROVE_LOCKING for sparc
in my testing. NP, just let me know. I'll then do the same for other
architectures not supporting RT if I hit the same problem there.

Guenter

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

[Sebastian Andrzej Siewior]

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.

> So, if people want to keep mandating PROVE_RAW_LOCK_NESTING together with
> PROVE_LOCKING for all architectures, I'll disable PROVE_LOCKING for sparc
> in my testing. NP, just let me know. I'll then do the same for other
> architectures not supporting RT if I hit the same problem there.

Waiman posted a patch to disable it on architectures that don't support
PREEMPT_RT. You could also post the patches you discussed. Andreas does
not seem to be against it (but then I don't know if he is a 32 or 64bit
guy). I did not year from other architectures so far.

> Guenter

Sebastian

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

[Guenter Roeck]

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.

>> So, if people want to keep mandating PROVE_RAW_LOCK_NESTING together with
>> PROVE_LOCKING for all architectures, I'll disable PROVE_LOCKING for sparc
>> in my testing. NP, just let me know. I'll then do the same for other
>> architectures not supporting RT if I hit the same problem there.
> 
> Waiman posted a patch to disable it on architectures that don't support
> PREEMPT_RT. You could also post the patches you discussed. Andreas does
> not seem to be against it (but then I don't know if he is a 32 or 64bit
> guy). I did not year from other architectures so far.
> 

I don't have time to work through all the issues, much less to argue with
affected maintainers to get the various patches accepted. I am open to fixing
regressions if I can, but it appears that this is not considered to be a
regression. I'll just disable PROVE_LOCKING for affected architectures
in my testing after -rc1 is out.

Thanks,
Guenter

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

[Waiman Long]

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.

Cheers,
Longman

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

[Guenter Roeck]

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.

Guenter

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

[Waiman Long]

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

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

[Guenter Roeck]

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

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

[Waiman Long]

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