powerpc/eeh: fix recursive pci_lock_rescan_remove locking in EEH event handling

PATCH Applied Narayana Murty N 5 participants 9 messages Dec 10, 2025 – Jan 14

Subsystems (2)

PowerPC PCI enhanced error handling (eeh) for PowerPC

lore

Share & tools

lore

Narayana Murty N

author

2025-12-10 18:26 UTC #0 lore raw

PATCH v2 1/1 powerpc/eeh: fix recursive pci_lock_rescan_remove locking in EEH event handling

The recent commit 1010b4c012b0 ("powerpc/eeh: Make EEH driver device hotplug safe") restructured the EEH driver to improve synchronization with the PCI hotplug layer.

However, it inadvertently moved pci_lock_rescan_remove() outside its intended scope in eeh_handle_normal_event(), leading to broken PCI error reporting and improper EEH event triggering. Specifically, eeh_handle_normal_event() acquired pci_lock_rescan_remove() before calling eeh_pe_bus_get(), but eeh_pe_bus_get() itself attempts to acquire the same lock internally, causing nested locking and disrupting normal EEH event handling paths.

This patch adds a boolean parameter do_lock to _eeh_pe_bus_get(), with two public wrappers:
eeh_pe_bus_get() with locking enabled. eeh_pe_bus_get_nolock() that skips locking.

Callers that already hold pci_lock_rescan_remove() now use eeh_pe_bus_get_nolock() to avoid recursive lock acquisition.

Additionally, pci_lock_rescan_remove() calls are restored to the correct position—after eeh_pe_bus_get() and immediately before iterating affected PEs and devices. This ensures EEH-triggered PCI removes occur under proper bus rescan locking without recursive lock contention.

The eeh_pe_loc_get() function has been split into two functions:
eeh_pe_loc_get(struct eeh_pe *pe) which retrieves the loc for given PE. eeh_pe_loc_get_bus(struct pci_bus *bus) which retrieves the location code for given bus.

This resolves lockdep warnings such as:
<snip>

[   84.964298] [    T928] ============================================
[   84.964304] [    T928] WARNING: possible recursive locking detected
[   84.964311] [    T928] 6.18.0-rc3 #51 Not tainted
[   84.964315] [    T928] --------------------------------------------
[   84.964320] [    T928] eehd/928 is trying to acquire lock:
[   84.964324] [    T928] c000000003b29d58 (pci_rescan_remove_lock){+.+.}-{3:3}, at: pci_lock_rescan_remove+0x28/0x40
[   84.964342] [    T928]
                       but task is already holding lock:
[   84.964347] [    T928] c000000003b29d58 (pci_rescan_remove_lock){+.+.}-{3:3}, at: pci_lock_rescan_remove+0x28/0x40
[   84.964357] [    T928]
                       other info that might help us debug this:
[   84.964363] [    T928]  Possible unsafe locking scenario:

[   84.964367] [    T928]        CPU0
[   84.964370] [    T928]        ----
[   84.964373] [    T928]   lock(pci_rescan_remove_lock);
[   84.964378] [    T928]   lock(pci_rescan_remove_lock);
[   84.964383] [    T928]
                       *** DEADLOCK ***

[   84.964388] [    T928]  May be due to missing lock nesting notation

[   84.964393] [    T928] 1 lock held by eehd/928:
[   84.964397] [    T928]  #0: c000000003b29d58 (pci_rescan_remove_lock){+.+.}-{3:3}, at: pci_lock_rescan_remove+0x28/0x40
[   84.964408] [    T928]
                       stack backtrace:
[   84.964414] [    T928] CPU: 2 UID: 0 PID: 928 Comm: eehd Not tainted 6.18.0-rc3 #51 VOLUNTARY
[   84.964417] [    T928] Hardware name: IBM,9080-HEX POWER10 (architected) 0x800200 0xf000006 of:IBM,FW1060.00 (NH1060_022) hv:phyp pSeries
[   84.964419] [    T928] Call Trace:
[   84.964420] [    T928] [c0000011a7157990] [c000000001705de4] dump_stack_lvl+0xc8/0x130 (unreliable)
[   84.964424] [    T928] [c0000011a71579d0] [c0000000002f66e0] print_deadlock_bug+0x430/0x440
[   84.964428] [    T928] [c0000011a7157a70] [c0000000002fd0c0] __lock_acquire+0x1530/0x2d80
[   84.964431] [    T928] [c0000011a7157ba0] [c0000000002fea54] lock_acquire+0x144/0x410
[   84.964433] [    T928] [c0000011a7157cb0] [c0000011a7157cb0] __mutex_lock+0xf4/0x1050
[   84.964436] [    T928] [c0000011a7157e00] [c000000000de21d8] pci_lock_rescan_remove+0x28/0x40
[   84.964439] [    T928] [c0000011a7157e20] [c00000000004ed98] eeh_pe_bus_get+0x48/0xc0
[   84.964442] [    T928] [c0000011a7157e50] [c000000000050434] eeh_handle_normal_event+0x64/0xa60
[   84.964446] [    T928] [c0000011a7157f30] [c000000000051de8] eeh_event_handler+0xf8/0x190
[   84.964450] [    T928] [c0000011a7157f90] [c0000000002747ac] kthread+0x16c/0x180
[   84.964453] [    T928] [c0000011a7157fe0] [c00000000000ded8] start_kernel_thread+0x14/0x18

</snip>

Fixes 1010b4c012b0 ("powerpc/eeh: Make EEH driver device hotplug safe")

Signed-off-by Narayana Murty N <[email protected]>

Reviewed-by Sourabh Jain <[email protected]>

Reviewed-by Mahesh Salgaonkar <[email protected]>

---
Changelog:
https://lore.kernel.org/all/[email protected]/
V1:
* Split eeh_pe_loc_get() into two functions — eeh_pe_loc_get() and
* eeh_pe_loc_get_bus() to separate PE-to-bus lookup from PCI bus location
* code retrieval, per code review suggestions.
V2:
* Change in commit message

 arch/powerpc/include/asm/eeh.h   |  2 +
 arch/powerpc/kernel/eeh_driver.c | 11 ++---
 arch/powerpc/kernel/eeh_pe.c     | 74 ++++++++++++++++++++++++++++++--
 3 files changed, 78 insertions(+), 9 deletions(-)

arch/powerpc/include/asm/eeh.h +2 -0

index 5e34611de9ef..b7ebb4ac2c71 100644
@@ -289,6 +289,8 @@ void eeh_pe_dev_traverse(struct eeh_pe *root,
 void eeh_pe_restore_bars(struct eeh_pe *pe);
 const char *eeh_pe_loc_get(struct eeh_pe *pe);
 struct pci_bus *eeh_pe_bus_get(struct eeh_pe *pe);
+const char *eeh_pe_loc_get_bus(struct pci_bus *bus);
+struct pci_bus *eeh_pe_bus_get_nolock(struct eeh_pe *pe);
 
 void eeh_show_enabled(void);
 int __init eeh_init(struct eeh_ops *ops);

arch/powerpc/kernel/eeh_driver.c +6 -5

index ef78ff77cf8f..028f69158532 100644
@@ -846,7 +846,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
 
 	pci_lock_rescan_remove();
 
-	bus = eeh_pe_bus_get(pe);
+	bus = eeh_pe_bus_get_nolock(pe);
 	if (!bus) {
 		pr_err("%s: Cannot find PCI bus for PHB#%x-PE#%x\n",
 			__func__, pe->phb->global_number, pe->addr);

@@ -886,14 +886,15 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
 	/* Log the event */
 	if (pe->type & EEH_PE_PHB) {
 		pr_err("EEH: Recovering PHB#%x, location: %s\n",
-			pe->phb->global_number, eeh_pe_loc_get(pe));
+			pe->phb->global_number, eeh_pe_loc_get_bus(bus));
 	} else {
 		struct eeh_pe *phb_pe = eeh_phb_pe_get(pe->phb);
 
 		pr_err("EEH: Recovering PHB#%x-PE#%x\n",
 		       pe->phb->global_number, pe->addr);
 		pr_err("EEH: PE location: %s, PHB location: %s\n",
-		       eeh_pe_loc_get(pe), eeh_pe_loc_get(phb_pe));
+		       eeh_pe_loc_get_bus(bus),
+		       eeh_pe_loc_get_bus(eeh_pe_bus_get_nolock(phb_pe)));
 	}
 
 #ifdef CONFIG_STACKTRACE

@@ -1098,7 +1099,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
 		eeh_pe_state_clear(pe, EEH_PE_PRI_BUS, true);
 		eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
 
-		bus = eeh_pe_bus_get(pe);
+		bus = eeh_pe_bus_get_nolock(pe);
 		if (bus)
 			pci_hp_remove_devices(bus);
 		else

@@ -1222,7 +1223,7 @@ void eeh_handle_special_event(void)
 				    (phb_pe->state & EEH_PE_RECOVERING))
 					continue;
 
-				bus = eeh_pe_bus_get(phb_pe);
+				bus = eeh_pe_bus_get_nolock(phb_pe);
 				if (!bus) {
 					pr_err("%s: Cannot find PCI bus for "
 					       "PHB#%x-PE#%x\n",

arch/powerpc/kernel/eeh_pe.c +70 -4

102 lines not on this page · raw message

Signature · 1 line

2.51.1

Bjorn Helgaas

2025-12-10 21:46 UTC #1 lore raw

Quoted 64 lines · Narayana Murty N

The recent commit 1010b4c012b0 ("powerpc/eeh: Make EEH driver device hotplug safe") restructured the EEH driver to improve synchronization with the PCI hotplug layer.
However, it inadvertently moved pci_lock_rescan_remove() outside its intended scope in eeh_handle_normal_event(), leading to broken PCI error reporting and improper EEH event triggering. Specifically, eeh_handle_normal_event() acquired pci_lock_rescan_remove() before calling eeh_pe_bus_get(), but eeh_pe_bus_get() itself attempts to acquire the same lock internally, causing nested locking and disrupting normal EEH event handling paths.
This patch adds a boolean parameter do_lock to _eeh_pe_bus_get(), with two public wrappers:
eeh_pe_bus_get() with locking enabled. eeh_pe_bus_get_nolock() that skips locking.
Callers that already hold pci_lock_rescan_remove() now use eeh_pe_bus_get_nolock() to avoid recursive lock acquisition.
Additionally, pci_lock_rescan_remove() calls are restored to the correct position—after eeh_pe_bus_get() and immediately before iterating affected PEs and devices. This ensures EEH-triggered PCI removes occur under proper bus rescan locking without recursive lock contention.
The eeh_pe_loc_get() function has been split into two functions:
eeh_pe_loc_get(struct eeh_pe *pe) which retrieves the loc for given PE. eeh_pe_loc_get_bus(struct pci_bus *bus) which retrieves the location code for given bus.
This resolves lockdep warnings such as:
<snip>
[   84.964298] [    T928] ============================================
[   84.964304] [    T928] WARNING: possible recursive locking detected
[   84.964311] [    T928] 6.18.0-rc3 #51 Not tainted
[   84.964315] [    T928] --------------------------------------------
[   84.964320] [    T928] eehd/928 is trying to acquire lock:
[   84.964324] [    T928] c000000003b29d58 (pci_rescan_remove_lock){+.+.}-{3:3}, at: pci_lock_rescan_remove+0x28/0x40
[   84.964342] [    T928]
                       but task is already holding lock:
[   84.964347] [    T928] c000000003b29d58 (pci_rescan_remove_lock){+.+.}-{3:3}, at: pci_lock_rescan_remove+0x28/0x40
[   84.964357] [    T928]
                       other info that might help us debug this:
[   84.964363] [    T928]  Possible unsafe locking scenario:
[   84.964367] [    T928]        CPU0
[   84.964370] [    T928]        ----
[   84.964373] [    T928]   lock(pci_rescan_remove_lock);
[   84.964378] [    T928]   lock(pci_rescan_remove_lock);
[   84.964383] [    T928]
                       *** DEADLOCK ***
[   84.964388] [    T928]  May be due to missing lock nesting notation
[   84.964393] [    T928] 1 lock held by eehd/928:
[   84.964397] [    T928]  #0: c000000003b29d58 (pci_rescan_remove_lock){+.+.}-{3:3}, at: pci_lock_rescan_remove+0x28/0x40
[   84.964408] [    T928]
                       stack backtrace:
[   84.964414] [    T928] CPU: 2 UID: 0 PID: 928 Comm: eehd Not tainted 6.18.0-rc3 #51 VOLUNTARY
[   84.964417] [    T928] Hardware name: IBM,9080-HEX POWER10 (architected) 0x800200 0xf000006 of:IBM,FW1060.00 (NH1060_022) hv:phyp pSeries
[   84.964419] [    T928] Call Trace:
[   84.964420] [    T928] [c0000011a7157990] [c000000001705de4] dump_stack_lvl+0xc8/0x130 (unreliable)
[   84.964424] [    T928] [c0000011a71579d0] [c0000000002f66e0] print_deadlock_bug+0x430/0x440
[   84.964428] [    T928] [c0000011a7157a70] [c0000000002fd0c0] __lock_acquire+0x1530/0x2d80

Quoted 8 lines

[   84.964431] [    T928] [c0000011a7157ba0] [c0000000002fea54] lock_acquire+0x144/0x410
[   84.964433] [    T928] [c0000011a7157cb0] [c0000011a7157cb0] __mutex_lock+0xf4/0x1050
[   84.964436] [    T928] [c0000011a7157e00] [c000000000de21d8] pci_lock_rescan_remove+0x28/0x40
[   84.964439] [    T928] [c0000011a7157e20] [c00000000004ed98] eeh_pe_bus_get+0x48/0xc0
[   84.964442] [    T928] [c0000011a7157e50] [c000000000050434] eeh_handle_normal_event+0x64/0xa60
[   84.964446] [    T928] [c0000011a7157f30] [c000000000051de8] eeh_event_handler+0xf8/0x190
[   84.964450] [    T928] [c0000011a7157f90] [c0000000002747ac] kthread+0x16c/0x180
[   84.964453] [    T928] [c0000011a7157fe0] [c00000000000ded8] start_kernel_thread+0x14/0x18

I have no comment on the patch itself, but the timestamps above aren't relevant and could be removed. Maybe also the "T928" part.

Bjorn

Narayana Murty N

author

2025-12-15 06:36 UTC #2 lore raw

Quoted 73 lines · Bjorn Helgaas

Quoted 64 lines · Narayana Murty N
The recent commit 1010b4c012b0 ("powerpc/eeh: Make EEH driver device hotplug safe") restructured the EEH driver to improve synchronization with the PCI hotplug layer.
However, it inadvertently moved pci_lock_rescan_remove() outside its intended scope in eeh_handle_normal_event(), leading to broken PCI error reporting and improper EEH event triggering. Specifically, eeh_handle_normal_event() acquired pci_lock_rescan_remove() before calling eeh_pe_bus_get(), but eeh_pe_bus_get() itself attempts to acquire the same lock internally, causing nested locking and disrupting normal EEH event handling paths.
This patch adds a boolean parameter do_lock to _eeh_pe_bus_get(), with two public wrappers:
eeh_pe_bus_get() with locking enabled. eeh_pe_bus_get_nolock() that skips locking.
Callers that already hold pci_lock_rescan_remove() now use eeh_pe_bus_get_nolock() to avoid recursive lock acquisition.
Additionally, pci_lock_rescan_remove() calls are restored to the correct position—after eeh_pe_bus_get() and immediately before iterating affected PEs and devices. This ensures EEH-triggered PCI removes occur under proper bus rescan locking without recursive lock contention.
The eeh_pe_loc_get() function has been split into two functions:
eeh_pe_loc_get(struct eeh_pe *pe) which retrieves the loc for given PE. eeh_pe_loc_get_bus(struct pci_bus *bus) which retrieves the location code for given bus.
This resolves lockdep warnings such as:
<snip>
[   84.964298] [    T928] ============================================
[   84.964304] [    T928] WARNING: possible recursive locking detected
[   84.964311] [    T928] 6.18.0-rc3 #51 Not tainted
[   84.964315] [    T928] --------------------------------------------
[   84.964320] [    T928] eehd/928 is trying to acquire lock:
[   84.964324] [    T928] c000000003b29d58 (pci_rescan_remove_lock){+.+.}-{3:3}, at: pci_lock_rescan_remove+0x28/0x40
[   84.964342] [    T928]
                        but task is already holding lock:
[   84.964347] [    T928] c000000003b29d58 (pci_rescan_remove_lock){+.+.}-{3:3}, at: pci_lock_rescan_remove+0x28/0x40
[   84.964357] [    T928]
                        other info that might help us debug this:
[   84.964363] [    T928]  Possible unsafe locking scenario:
[   84.964367] [    T928]        CPU0
[   84.964370] [    T928]        ----
[   84.964373] [    T928]   lock(pci_rescan_remove_lock);
[   84.964378] [    T928]   lock(pci_rescan_remove_lock);
[   84.964383] [    T928]
                        *** DEADLOCK ***
[   84.964388] [    T928]  May be due to missing lock nesting notation
[   84.964393] [    T928] 1 lock held by eehd/928:
[   84.964397] [    T928]  #0: c000000003b29d58 (pci_rescan_remove_lock){+.+.}-{3:3}, at: pci_lock_rescan_remove+0x28/0x40
[   84.964408] [    T928]
                        stack backtrace:
[   84.964414] [    T928] CPU: 2 UID: 0 PID: 928 Comm: eehd Not tainted 6.18.0-rc3 #51 VOLUNTARY
[   84.964417] [    T928] Hardware name: IBM,9080-HEX POWER10 (architected) 0x800200 0xf000006 of:IBM,FW1060.00 (NH1060_022) hv:phyp pSeries
[   84.964419] [    T928] Call Trace:
[   84.964420] [    T928] [c0000011a7157990] [c000000001705de4] dump_stack_lvl+0xc8/0x130 (unreliable)
[   84.964424] [    T928] [c0000011a71579d0] [c0000000002f66e0] print_deadlock_bug+0x430/0x440
[   84.964428] [    T928] [c0000011a7157a70] [c0000000002fd0c0] __lock_acquire+0x1530/0x2d80
Quoted 8 lines
[   84.964431] [    T928] [c0000011a7157ba0] [c0000000002fea54] lock_acquire+0x144/0x410
[   84.964433] [    T928] [c0000011a7157cb0] [c0000011a7157cb0] __mutex_lock+0xf4/0x1050
[   84.964436] [    T928] [c0000011a7157e00] [c000000000de21d8] pci_lock_rescan_remove+0x28/0x40
[   84.964439] [    T928] [c0000011a7157e20] [c00000000004ed98] eeh_pe_bus_get+0x48/0xc0
[   84.964442] [    T928] [c0000011a7157e50] [c000000000050434] eeh_handle_normal_event+0x64/0xa60
[   84.964446] [    T928] [c0000011a7157f30] [c000000000051de8] eeh_event_handler+0xf8/0x190
[   84.964450] [    T928] [c0000011a7157f90] [c0000000002747ac] kthread+0x16c/0x180
[   84.964453] [    T928] [c0000011a7157fe0] [c00000000000ded8] start_kernel_thread+0x14/0x18

Quoted 4 lines

I have no comment on the patch itself, but the timestamps above aren't relevant and could be removed. Maybe also the "T928" part.
Bjorn

Thank you for your suggestion, I will cleanup the commit message

Narayana Murty

Timothy Pearson

2025-12-11 15:45 UTC #3 lore raw

In reply to Narayana Murty N

----- Original Message -----

Quoted 34 lines

From: "Narayana Murty N" <[email protected]>
To: "mahesh" <[email protected]>, "Oliver" <[email protected]>, "Madhavan Srinivasan" <[email protected]>, "Michael Ellerman" <[email protected]>, "npiggin" <[email protected]>, "christophe leroy" <[email protected]>
Cc: "Bjorn Helgaas" <[email protected]>, "Timothy Pearson" <[email protected]>, "linuxppc-dev" <[email protected]>, "linux-kernel" <[email protected]>, "vaibhav" <[email protected]>, "Shivaprasad G Bhat" <[email protected]>, [email protected]
Sent: Wednesday, December 10, 2025 8:25:59 AM
Subject: [PATCH v2 1/1] powerpc/eeh: fix recursive pci_lock_rescan_remove locking in EEH event handling
The recent commit 1010b4c012b0 ("powerpc/eeh: Make EEH driver device hotplug safe") restructured the EEH driver to improve synchronization with the PCI hotplug layer.
However, it inadvertently moved pci_lock_rescan_remove() outside its intended scope in eeh_handle_normal_event(), leading to broken PCI error reporting and improper EEH event triggering. Specifically, eeh_handle_normal_event() acquired pci_lock_rescan_remove() before calling eeh_pe_bus_get(), but eeh_pe_bus_get() itself attempts to acquire the same lock internally, causing nested locking and disrupting normal EEH event handling paths.
This patch adds a boolean parameter do_lock to _eeh_pe_bus_get(), with two public wrappers:
eeh_pe_bus_get() with locking enabled. eeh_pe_bus_get_nolock() that skips locking.
Callers that already hold pci_lock_rescan_remove() now use eeh_pe_bus_get_nolock() to avoid recursive lock acquisition.
Additionally, pci_lock_rescan_remove() calls are restored to the correct position—after eeh_pe_bus_get() and immediately before iterating affected PEs and devices. This ensures EEH-triggered PCI removes occur under proper bus rescan locking without recursive lock contention.

Quoted 4 lines

The eeh_pe_loc_get() function has been split into two functions:
eeh_pe_loc_get(struct eeh_pe *pe) which retrieves the loc for given PE. eeh_pe_loc_get_bus(struct pci_bus *bus) which retrieves the location code for given bus.

Conceptually the patch sounds OK, but given the complexity of these subsystems it's difficult to forsee all interactions. Was the patch verified not to break NVMe hotplug on PowerNV systems using actual hardware? If not, I will need to do so before sending an ack. Thanks!

Narayana Murty N

author

2025-12-17 05:01 UTC #4 lore raw

Quoted 35 lines · Timothy Pearson

----- Original Message -----
Quoted 33 lines
From: "Narayana Murty N" <[email protected]>
To: "mahesh" <[email protected]>, "Oliver" <[email protected]>, "Madhavan Srinivasan" <[email protected]>, "Michael Ellerman" <[email protected]>, "npiggin" <[email protected]>, "christophe leroy" <[email protected]>
Cc: "Bjorn Helgaas" <[email protected]>, "Timothy Pearson" <[email protected]>, "linuxppc-dev" <[email protected]>, "linux-kernel" <[email protected]>, "vaibhav" <[email protected]>, "Shivaprasad G Bhat" <[email protected]>, [email protected]
Sent: Wednesday, December 10, 2025 8:25:59 AM
Subject: [PATCH v2 1/1] powerpc/eeh: fix recursive pci_lock_rescan_remove locking in EEH event handling The recent commit 1010b4c012b0 ("powerpc/eeh: Make EEH driver device
hotplug safe") restructured the EEH driver to improve synchronization
with the PCI hotplug layer.
However, it inadvertently moved pci_lock_rescan_remove() outside its intended scope in eeh_handle_normal_event(), leading to broken PCI error reporting and improper EEH event triggering. Specifically, eeh_handle_normal_event() acquired pci_lock_rescan_remove() before calling eeh_pe_bus_get(), but eeh_pe_bus_get() itself attempts to acquire the same lock internally, causing nested locking and disrupting normal EEH event handling paths.
This patch adds a boolean parameter do_lock to _eeh_pe_bus_get(), with two public wrappers:
eeh_pe_bus_get() with locking enabled. eeh_pe_bus_get_nolock() that skips locking.
Callers that already hold pci_lock_rescan_remove() now use eeh_pe_bus_get_nolock() to avoid recursive lock acquisition.
Additionally, pci_lock_rescan_remove() calls are restored to the correct position—after eeh_pe_bus_get() and immediately before iterating affected PEs and devices. This ensures EEH-triggered PCI removes occur under proper bus rescan locking without recursive lock contention.

Quoted 5 lines

Quoted 4 lines
The eeh_pe_loc_get() function has been split into two functions:
eeh_pe_loc_get(struct eeh_pe *pe) which retrieves the loc for given PE. eeh_pe_loc_get_bus(struct pci_bus *bus) which retrieves the location code for given bus.
Conceptually the patch sounds OK, but given the complexity of these subsystems it's difficult to forsee all interactions. Was the patch verified not to break NVMe hotplug on PowerNV systems using actual hardware? If not, I will need to do so before sending an ack. Thanks!

It has not been specifically tested for NVMe hotplug on PowerNV hardware.

However, this change does not remove or relax any of the existing locking

around EEH handling, so the NVMe hotplug paths should continue to see

the same serialization as before.

If you have a convenient setup for NVMe hotplug on PowerNV, additional testing

there would definitely be helpful before merging.

Thanks,
Narayana Murty

Nilay Shroff

2025-12-22 05:35 UTC #5 lore raw

Quoted 46 lines · Narayana Murty N

Quoted 35 lines · Timothy Pearson
----- Original Message -----
Quoted 33 lines
From: "Narayana Murty N" <[email protected]>
To: "mahesh" <[email protected]>, "Oliver" <[email protected]>, "Madhavan Srinivasan" <[email protected]>, "Michael Ellerman" <[email protected]>, "npiggin" <[email protected]>, "christophe leroy" <[email protected]>
Cc: "Bjorn Helgaas" <[email protected]>, "Timothy Pearson" <[email protected]>, "linuxppc-dev" <[email protected]>, "linux-kernel" <[email protected]>, "vaibhav" <[email protected]>, "Shivaprasad G Bhat" <[email protected]>, [email protected]
Sent: Wednesday, December 10, 2025 8:25:59 AM
Subject: [PATCH v2 1/1] powerpc/eeh: fix recursive pci_lock_rescan_remove locking in EEH event handling The recent commit 1010b4c012b0 ("powerpc/eeh: Make EEH driver device
hotplug safe") restructured the EEH driver to improve synchronization
with the PCI hotplug layer.
However, it inadvertently moved pci_lock_rescan_remove() outside its intended scope in eeh_handle_normal_event(), leading to broken PCI error reporting and improper EEH event triggering. Specifically, eeh_handle_normal_event() acquired pci_lock_rescan_remove() before calling eeh_pe_bus_get(), but eeh_pe_bus_get() itself attempts to acquire the same lock internally, causing nested locking and disrupting normal EEH event handling paths.
This patch adds a boolean parameter do_lock to _eeh_pe_bus_get(), with two public wrappers:
eeh_pe_bus_get() with locking enabled. eeh_pe_bus_get_nolock() that skips locking.
Callers that already hold pci_lock_rescan_remove() now use eeh_pe_bus_get_nolock() to avoid recursive lock acquisition.
Additionally, pci_lock_rescan_remove() calls are restored to the correct position—after eeh_pe_bus_get() and immediately before iterating affected PEs and devices. This ensures EEH-triggered PCI removes occur under proper bus rescan locking without recursive lock contention.
Quoted 5 lines
Quoted 4 lines
The eeh_pe_loc_get() function has been split into two functions:
eeh_pe_loc_get(struct eeh_pe *pe) which retrieves the loc for given PE. eeh_pe_loc_get_bus(struct pci_bus *bus) which retrieves the location code for given bus.
Conceptually the patch sounds OK, but given the complexity of these subsystems it's difficult to forsee all interactions. Was the patch verified not to break NVMe hotplug on PowerNV systems using actual hardware? If not, I will need to do so before sending an ack. Thanks!
It has not been specifically tested for NVMe hotplug on PowerNV hardware.
However, this change does not remove or relax any of the existing locking

Quoted 7 lines

around EEH handling, so the NVMe hotplug paths should continue to see
the same serialization as before.
If you have a convenient setup for NVMe hotplug on PowerNV, additional testing
there would definitely be helpful before merging.

Yes, I'd also suggest testing this patch against NVMe pcie error recovery code path. Please note that NVMe driver hooks into the EEH (pcie recovery) subsystem, and so, IMO, it'd be necessary to test it using NVMe.

Thanks,
--Nilay

Narayana Murty N

author

2026-01-05 09:47 UTC #6 lore raw

Quoted 52 lines · Nilay Shroff

Quoted 50 lines · Narayana Murty N
Quoted 34 lines · Timothy Pearson
----- Original Message -----
Quoted 33 lines
From: "Narayana Murty N" <[email protected]>
To: "mahesh" <[email protected]>, "Oliver" <[email protected]>, "Madhavan Srinivasan" <[email protected]>, "Michael Ellerman" <[email protected]>, "npiggin" <[email protected]>, "christophe leroy" <[email protected]>
Cc: "Bjorn Helgaas" <[email protected]>, "Timothy Pearson" <[email protected]>, "linuxppc-dev" <[email protected]>, "linux-kernel" <[email protected]>, "vaibhav" <[email protected]>, "Shivaprasad G Bhat" <[email protected]>, [email protected]
Sent: Wednesday, December 10, 2025 8:25:59 AM
Subject: [PATCH v2 1/1] powerpc/eeh: fix recursive pci_lock_rescan_remove locking in EEH event handling The recent commit 1010b4c012b0 ("powerpc/eeh: Make EEH driver device
hotplug safe") restructured the EEH driver to improve synchronization
with the PCI hotplug layer.
However, it inadvertently moved pci_lock_rescan_remove() outside its intended scope in eeh_handle_normal_event(), leading to broken PCI error reporting and improper EEH event triggering. Specifically, eeh_handle_normal_event() acquired pci_lock_rescan_remove() before calling eeh_pe_bus_get(), but eeh_pe_bus_get() itself attempts to acquire the same lock internally, causing nested locking and disrupting normal EEH event handling paths.
This patch adds a boolean parameter do_lock to _eeh_pe_bus_get(), with two public wrappers:
eeh_pe_bus_get() with locking enabled. eeh_pe_bus_get_nolock() that skips locking.
Callers that already hold pci_lock_rescan_remove() now use eeh_pe_bus_get_nolock() to avoid recursive lock acquisition.
Additionally, pci_lock_rescan_remove() calls are restored to the correct position—after eeh_pe_bus_get() and immediately before iterating affected PEs and devices. This ensures EEH-triggered PCI removes occur under proper bus rescan locking without recursive lock contention.
Quoted 5 lines
Quoted 4 lines
The eeh_pe_loc_get() function has been split into two functions:
eeh_pe_loc_get(struct eeh_pe *pe) which retrieves the loc for given PE. eeh_pe_loc_get_bus(struct pci_bus *bus) which retrieves the location code for given bus.
Conceptually the patch sounds OK, but given the complexity of these subsystems it's difficult to forsee all interactions. Was the patch verified not to break NVMe hotplug on PowerNV systems using actual hardware? If not, I will need to do so before sending an ack. Thanks!
It has not been specifically tested for NVMe hotplug on PowerNV hardware.
However, this change does not remove or relax any of the existing locking
around EEH handling, so the NVMe hotplug paths should continue to see
the same serialization as before.
If you have a convenient setup for NVMe hotplug on PowerNV, additional testing

Quoted 7 lines

Quoted 1 line
there would definitely be helpful before merging.
Yes, I'd also suggest testing this patch against NVMe pcie error recovery code path. Please note that NVMe driver hooks into the EEH (pcie recovery) subsystem, and so, IMO, it'd be necessary to test it using NVMe.
Thanks,
--Nilay

Yes Nilay, this patches tested it on pseries nvme, error injecton and recovery

are working as expected.

Regards,

Narayana.

Quoted 3 lines

Narayana Murty N

author

2026-01-07 06:23 UTC #7 lore raw

In reply to Timothy Pearson

Quoted 35 lines · Timothy Pearson

----- Original Message -----
Quoted 33 lines
From: "Narayana Murty N" <[email protected]>
To: "mahesh" <[email protected]>, "Oliver" <[email protected]>, "Madhavan Srinivasan" <[email protected]>, "Michael Ellerman" <[email protected]>, "npiggin" <[email protected]>, "christophe leroy" <[email protected]>
Cc: "Bjorn Helgaas" <[email protected]>, "Timothy Pearson" <[email protected]>, "linuxppc-dev" <[email protected]>, "linux-kernel" <[email protected]>, "vaibhav" <[email protected]>, "Shivaprasad G Bhat" <[email protected]>, [email protected]
Sent: Wednesday, December 10, 2025 8:25:59 AM
Subject: [PATCH v2 1/1] powerpc/eeh: fix recursive pci_lock_rescan_remove locking in EEH event handling The recent commit 1010b4c012b0 ("powerpc/eeh: Make EEH driver device
hotplug safe") restructured the EEH driver to improve synchronization
with the PCI hotplug layer.
However, it inadvertently moved pci_lock_rescan_remove() outside its intended scope in eeh_handle_normal_event(), leading to broken PCI error reporting and improper EEH event triggering. Specifically, eeh_handle_normal_event() acquired pci_lock_rescan_remove() before calling eeh_pe_bus_get(), but eeh_pe_bus_get() itself attempts to acquire the same lock internally, causing nested locking and disrupting normal EEH event handling paths.
This patch adds a boolean parameter do_lock to _eeh_pe_bus_get(), with two public wrappers:
eeh_pe_bus_get() with locking enabled. eeh_pe_bus_get_nolock() that skips locking.
Callers that already hold pci_lock_rescan_remove() now use eeh_pe_bus_get_nolock() to avoid recursive lock acquisition.
Additionally, pci_lock_rescan_remove() calls are restored to the correct position—after eeh_pe_bus_get() and immediately before iterating affected PEs and devices. This ensures EEH-triggered PCI removes occur under proper bus rescan locking without recursive lock contention.

Quoted 5 lines

Quoted 4 lines
The eeh_pe_loc_get() function has been split into two functions:
eeh_pe_loc_get(struct eeh_pe *pe) which retrieves the loc for given PE. eeh_pe_loc_get_bus(struct pci_bus *bus) which retrieves the location code for given bus.
Conceptually the patch sounds OK, but given the complexity of these subsystems it's difficult to forsee all interactions. Was the patch verified not to break NVMe hotplug on PowerNV systems using actual hardware? If not, I will need to do so before sending an ack. Thanks!

Hi Timothy,

Thanks for your suggestion,I have now verified the change on a PowerNV system with NVMe hotplug.

Test setup:
Platform: PowerNV (“Hardware name: 9105-22A POWER10 (raw) 0x800200 opal:v7.1-126-g9f16f2d9e PowerNV”).
Kernel: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git this patch on top of commit d358e5254674
Devices: two PCIe NVMe drives in hotpluggable slots.

Tests performed:
Basic hotplug:
Repeated NVMe add/remove cycles using the platform’s hotplug controls (slot power off/on or PCIe attention button, as applicable).
Confirmed that each add/remove cycle correctly created and removed /dev/nvme* nodes, and that nvme list/I/O (e.g. fio or dd) worked before removal and failed cleanly after removal. Confirmed there were no lockdep splats, warnings, or stack traces related to pci_lock_rescan_remove() or EEH during these tests.

Regression checks:
With these tests, NVMe hotplug and EEH behaviour on PowerNV appears unchanged except for the intended fix (no recursive pci_lock_rescan_remove() acquisition and normal EEH event handling).

Thanks,

Narayana

Madhavan Srinivasan

maintainer Applied

2026-01-14 03:04 UTC #8 lore raw

In reply to Narayana Murty N

Quoted 12 lines · Narayana Murty N

The recent commit 1010b4c012b0 ("powerpc/eeh: Make EEH driver device hotplug safe") restructured the EEH driver to improve synchronization with the PCI hotplug layer.
However, it inadvertently moved pci_lock_rescan_remove() outside its intended scope in eeh_handle_normal_event(), leading to broken PCI error reporting and improper EEH event triggering. Specifically, eeh_handle_normal_event() acquired pci_lock_rescan_remove() before calling eeh_pe_bus_get(), but eeh_pe_bus_get() itself attempts to acquire the same lock internally, causing nested locking and disrupting normal EEH event handling paths.

Quoted 1 line

[...]

Applied to powerpc/next.

[1/1] powerpc/eeh: fix recursive pci_lock_rescan_remove locking in EEH event handling

      https://git.kernel.org/powerpc/c/815a8d2feb5615ae7f0b5befd206af0b0160614c

cheers