Re: [PATCH v3 8/9] powerpc/eeh: Factor out common code eeh_reset_device()

[Alexey Kardashevskiy]

On 21/3/18 1:06 pm, Sam Bobroff wrote:
> The caller will always pass NULL for 'rmv_data' when
> 'eeh_aware_driver' is true, so the first two calls to
> eeh_pe_dev_traverse() can be combined without changing behaviour as
> can the two arms of the final 'if' block.
> 
> This should not change behaviour.
> 
> Signed-off-by: Sam Bobroff 


Reviewed-by: Alexey Kardashevskiy 


> ---
> == v2 -> v3: ==
> Corrected two missed cases of eeh_aware_driver -> driver_eeh_aware.
> 
>  arch/powerpc/kernel/eeh_driver.c | 32 ++--
>  1 file changed, 10 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/eeh_driver.c 
> b/arch/powerpc/kernel/eeh_driver.c
> index 93fc22e791fa..43ceb6263cd8 100644
> --- a/arch/powerpc/kernel/eeh_driver.c
> +++ b/arch/powerpc/kernel/eeh_driver.c
> @@ -647,16 +647,12 @@ static int eeh_reset_device(struct eeh_pe *pe, struct 
> pci_bus *bus,
>* into pci_hp_add_devices().
>*/
>   eeh_pe_state_mark(pe, EEH_PE_KEEP);
> - if (!driver_eeh_aware) {
> - if (pe->type & EEH_PE_VF) {
> - eeh_pe_dev_traverse(pe, eeh_rmv_device, NULL);
> - } else {
> - pci_lock_rescan_remove();
> - pci_hp_remove_devices(bus);
> - pci_unlock_rescan_remove();
> - }
> - } else {
> + if (driver_eeh_aware || (pe->type & EEH_PE_VF)) {
>   eeh_pe_dev_traverse(pe, eeh_rmv_device, rmv_data);
> + } else {
> + pci_lock_rescan_remove();
> + pci_hp_remove_devices(bus);
> + pci_unlock_rescan_remove();
>   }
>  
>   /*
> @@ -691,8 +687,9 @@ static int eeh_reset_device(struct eeh_pe *pe, struct 
> pci_bus *bus,
>* the device up before the scripts have taken it down,
>* potentially weird things happen.
>*/
> - if (!driver_eeh_aware) {
> - pr_info("EEH: Sleep 5s ahead of complete hotplug\n");
> + if (!driver_eeh_aware || rmv_data->removed) {
> + pr_info("EEH: Sleep 5s ahead of %s hotplug\n",
> + (driver_eeh_aware ? "partial" : "complete"));
>   ssleep(5);
>  
>   /*
> @@ -705,19 +702,10 @@ static int eeh_reset_device(struct eeh_pe *pe, struct 
> pci_bus *bus,
>   if (pe->type & EEH_PE_VF) {
>   eeh_add_virt_device(edev, NULL);
>   } else {
> - eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
> + if (!driver_eeh_aware)
> + eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
>   pci_hp_add_devices(bus);
>   }
> - } else if (rmv_data->removed) {
> - pr_info("EEH: Sleep 5s ahead of partial hotplug\n");
> - ssleep(5);
> -
> - edev = list_first_entry(>edevs, struct eeh_dev, list);
> - eeh_pe_traverse(pe, eeh_pe_detach_dev, NULL);
> - if (pe->type & EEH_PE_VF)
> - eeh_add_virt_device(edev, NULL);
> - else
> - pci_hp_add_devices(bus);
>   }
>   eeh_pe_state_clear(pe, EEH_PE_KEEP);
>  
> 


-- 
Alexey

[PATCH v3 8/9] powerpc/eeh: Factor out common code eeh_reset_device()

[Sam Bobroff]

The caller will always pass NULL for 'rmv_data' when
'eeh_aware_driver' is true, so the first two calls to
eeh_pe_dev_traverse() can be combined without changing behaviour as
can the two arms of the final 'if' block.

This should not change behaviour.

Signed-off-by: Sam Bobroff 
---
== v2 -> v3: ==
Corrected two missed cases of eeh_aware_driver -> driver_eeh_aware.

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

diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 93fc22e791fa..43ceb6263cd8 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -647,16 +647,12 @@ static int eeh_reset_device(struct eeh_pe *pe, struct 
pci_bus *bus,
 * into pci_hp_add_devices().
 */
eeh_pe_state_mark(pe, EEH_PE_KEEP);
-   if (!driver_eeh_aware) {
-   if (pe->type & EEH_PE_VF) {
-   eeh_pe_dev_traverse(pe, eeh_rmv_device, NULL);
-   } else {
-   pci_lock_rescan_remove();
-   pci_hp_remove_devices(bus);
-   pci_unlock_rescan_remove();
-   }
-   } else {
+   if (driver_eeh_aware || (pe->type & EEH_PE_VF)) {
eeh_pe_dev_traverse(pe, eeh_rmv_device, rmv_data);
+   } else {
+   pci_lock_rescan_remove();
+   pci_hp_remove_devices(bus);
+   pci_unlock_rescan_remove();
}
 
/*
@@ -691,8 +687,9 @@ static int eeh_reset_device(struct eeh_pe *pe, struct 
pci_bus *bus,
 * the device up before the scripts have taken it down,
 * potentially weird things happen.
 */
-   if (!driver_eeh_aware) {
-   pr_info("EEH: Sleep 5s ahead of complete hotplug\n");
+   if (!driver_eeh_aware || rmv_data->removed) {
+   pr_info("EEH: Sleep 5s ahead of %s hotplug\n",
+   (driver_eeh_aware ? "partial" : "complete"));
ssleep(5);
 
/*
@@ -705,19 +702,10 @@ static int eeh_reset_device(struct eeh_pe *pe, struct 
pci_bus *bus,
if (pe->type & EEH_PE_VF) {
eeh_add_virt_device(edev, NULL);
} else {
-   eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
+   if (!driver_eeh_aware)
+   eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
pci_hp_add_devices(bus);
}
-   } else if (rmv_data->removed) {
-   pr_info("EEH: Sleep 5s ahead of partial hotplug\n");
-   ssleep(5);
-
-   edev = list_first_entry(>edevs, struct eeh_dev, list);
-   eeh_pe_traverse(pe, eeh_pe_detach_dev, NULL);
-   if (pe->type & EEH_PE_VF)
-   eeh_add_virt_device(edev, NULL);
-   else
-   pci_hp_add_devices(bus);
}
eeh_pe_state_clear(pe, EEH_PE_KEEP);
 
-- 
2.16.1.74.g9b0b1f47b