linux-kernel - Re: [PATCH] sparc/pci: Make pci_poke_lock a raw_spinlock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b776ca37-d51c-47e2-b3bb-aee8e7910630@roeck-us.net>
Date: Mon, 25 Nov 2024 09:01:33 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
 sparclinux@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, Boqun Feng <boqun.feng@...il.com>,
 Ingo Molnar <mingo@...hat.com>, Peter Zijlstra <peterz@...radead.org>,
 Thomas Gleixner <tglx@...utronix.de>, Waiman Long <longman@...hat.com>,
 Will Deacon <will@...nel.org>, "David S. Miller" <davem@...emloft.net>,
 Andreas Larsson <andreas@...sler.com>
Subject: Re: [PATCH] sparc/pci: Make pci_poke_lock a raw_spinlock_t.

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@...ck-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@...utronix.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