Re: [PATCH 1/1] powerpc/eeh: fix deadlock handling dead PHB

[Frederic Barrat <fbarrat@linux.ibm.com>]

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 "
>