[1/1] powerpc/eeh: fix deadlock handling dead PHB
diff mbox series

Message ID 0547e82dbf90ee0729a2979a8cac5c91665c621f.1581051445.git.sbobroff@linux.ibm.com
State Accepted
Commit d4f194ed9eb9841a8f978710e4d24296f791a85b
Headers show
Series
  • [1/1] powerpc/eeh: fix deadlock handling dead PHB
Related show

Checks

Context Check Description
snowpatch_ozlabs/needsstable success Patch has no Fixes tags
snowpatch_ozlabs/checkpatch warning total: 0 errors, 0 warnings, 1 checks, 33 lines checked
snowpatch_ozlabs/build-pmac32 success Build succeeded
snowpatch_ozlabs/build-ppc64e success Build succeeded
snowpatch_ozlabs/build-ppc64be success Build succeeded
snowpatch_ozlabs/build-ppc64le success Build succeeded
snowpatch_ozlabs/apply_patch success Successfully applied on branch powerpc/merge (530a1cfd52af0aba1af4b1c9a7bc66a202a459b1)

Commit Message

Sam Bobroff Feb. 7, 2020, 4:57 a.m. UTC
Recovering a dead PHB can currently cause a deadlock as the PCI
rescan/remove lock is taken twice.

This is caused as part of an existing bug in
eeh_handle_special_event(). The pe is processed while traversing the
PHBs even though the pe is unrelated to the loop. This causes the pe
to be, incorrectly, processed more than once.

Untangling this section can move the pe processing out of the loop and
also outside the locked section, correcting both problems.

Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
I have only compile tested this fix, Frederic Barrat (who discovered it) has
offered to test it (thanks!).

 arch/powerpc/kernel/eeh_driver.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

Comments

Frederic Barrat Feb. 7, 2020, 8:56 a.m. UTC | #1
Le 07/02/2020 à 05:57, Sam Bobroff a écrit :
> Recovering a dead PHB can currently cause a deadlock as the PCI
> rescan/remove lock is taken twice.
> 
> This is caused as part of an existing bug in
> eeh_handle_special_event(). The pe is processed while traversing the
> PHBs even though the pe is unrelated to the loop. This causes the pe
> to be, incorrectly, processed more than once.
> 
> Untangling this section can move the pe processing out of the loop and
> also outside the locked section, correcting both problems.
> 
> Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> ---


Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>
Tested-by: Frederic Barrat <fbarrat@linux.ibm.com>

I think it also needs:
Fixes: 2e25505147b8 ("powerpc/eeh: Fix crash when edev->pdev changes")
Cc: stable@vger.kernel.org # 5.4+


   Fred


> I have only compile tested this fix, Frederic Barrat (who discovered it) has
> offered to test it (thanks!).
> 
>   arch/powerpc/kernel/eeh_driver.c | 21 +++++++++++----------
>   1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
> index 3dd1a422fc29..d6e75a8a14ce 100644
> --- a/arch/powerpc/kernel/eeh_driver.c
> +++ b/arch/powerpc/kernel/eeh_driver.c
> @@ -1190,6 +1190,17 @@ void eeh_handle_special_event(void)
>   			eeh_pe_state_mark(pe, EEH_PE_RECOVERING);
>   			eeh_handle_normal_event(pe);
>   		} else {
> +			eeh_for_each_pe(pe, tmp_pe)
> +				eeh_pe_for_each_dev(tmp_pe, edev, tmp_edev)
> +					edev->mode &= ~EEH_DEV_NO_HANDLER;
> +
> +			/* Notify all devices to be down */
> +			eeh_pe_state_clear(pe, EEH_PE_PRI_BUS, true);
> +			eeh_set_channel_state(pe, pci_channel_io_perm_failure);
> +			eeh_pe_report(
> +				"error_detected(permanent failure)", pe,
> +				eeh_report_failure, NULL);
> +
>   			pci_lock_rescan_remove();
>   			list_for_each_entry(hose, &hose_list, list_node) {
>   				phb_pe = eeh_phb_pe_get(hose);
> @@ -1198,16 +1209,6 @@ void eeh_handle_special_event(void)
>   				    (phb_pe->state & EEH_PE_RECOVERING))
>   					continue;
>   
> -				eeh_for_each_pe(pe, tmp_pe)
> -					eeh_pe_for_each_dev(tmp_pe, edev, tmp_edev)
> -						edev->mode &= ~EEH_DEV_NO_HANDLER;
> -
> -				/* Notify all devices to be down */
> -				eeh_pe_state_clear(pe, EEH_PE_PRI_BUS, true);
> -				eeh_set_channel_state(pe, pci_channel_io_perm_failure);
> -				eeh_pe_report(
> -					"error_detected(permanent failure)", pe,
> -					eeh_report_failure, NULL);
>   				bus = eeh_pe_bus_get(phb_pe);
>   				if (!bus) {
>   					pr_err("%s: Cannot find PCI bus for "
>
Michael Ellerman Feb. 19, 2020, 12:39 p.m. UTC | #2
On Fri, 2020-02-07 at 04:57:31 UTC, Sam Bobroff wrote:
> Recovering a dead PHB can currently cause a deadlock as the PCI
> rescan/remove lock is taken twice.
> 
> This is caused as part of an existing bug in
> eeh_handle_special_event(). The pe is processed while traversing the
> PHBs even though the pe is unrelated to the loop. This causes the pe
> to be, incorrectly, processed more than once.
> 
> Untangling this section can move the pe processing out of the loop and
> also outside the locked section, correcting both problems.
> 
> Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>

Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/d4f194ed9eb9841a8f978710e4d24296f791a85b

cheers

Patch
diff mbox series

diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 3dd1a422fc29..d6e75a8a14ce 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -1190,6 +1190,17 @@  void eeh_handle_special_event(void)
 			eeh_pe_state_mark(pe, EEH_PE_RECOVERING);
 			eeh_handle_normal_event(pe);
 		} else {
+			eeh_for_each_pe(pe, tmp_pe)
+				eeh_pe_for_each_dev(tmp_pe, edev, tmp_edev)
+					edev->mode &= ~EEH_DEV_NO_HANDLER;
+
+			/* Notify all devices to be down */
+			eeh_pe_state_clear(pe, EEH_PE_PRI_BUS, true);
+			eeh_set_channel_state(pe, pci_channel_io_perm_failure);
+			eeh_pe_report(
+				"error_detected(permanent failure)", pe,
+				eeh_report_failure, NULL);
+
 			pci_lock_rescan_remove();
 			list_for_each_entry(hose, &hose_list, list_node) {
 				phb_pe = eeh_phb_pe_get(hose);
@@ -1198,16 +1209,6 @@  void eeh_handle_special_event(void)
 				    (phb_pe->state & EEH_PE_RECOVERING))
 					continue;
 
-				eeh_for_each_pe(pe, tmp_pe)
-					eeh_pe_for_each_dev(tmp_pe, edev, tmp_edev)
-						edev->mode &= ~EEH_DEV_NO_HANDLER;
-
-				/* Notify all devices to be down */
-				eeh_pe_state_clear(pe, EEH_PE_PRI_BUS, true);
-				eeh_set_channel_state(pe, pci_channel_io_perm_failure);
-				eeh_pe_report(
-					"error_detected(permanent failure)", pe,
-					eeh_report_failure, NULL);
 				bus = eeh_pe_bus_get(phb_pe);
 				if (!bus) {
 					pr_err("%s: Cannot find PCI bus for "