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

[kbuild test robot]

Hi Sam,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.16-rc4]
[also build test ERROR on next-20180319]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Sam-Bobroff/EEH-refactoring-1/20180320-024729
config: powerpc-ppc64_defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc 

Note: the linux-review/Sam-Bobroff/EEH-refactoring-1/20180320-024729 HEAD 
28e7cc1fa029b84221b827a6ea2908dc33b43d10 builds fine.
  It only hurts bisectibility.

All errors (new ones prefixed by >>):

   In file included from include/linux/kernel.h:14:0,
from include/linux/delay.h:22,
from arch/powerpc/kernel/eeh_driver.c:25:
   arch/powerpc/kernel/eeh_driver.c: In function 'eeh_reset_device':
>> arch/powerpc/kernel/eeh_driver.c:692:5: error: 'eeh_aware_driver' undeclared 
>> (first use in this function); did you mean 'device_driver'?
   (eeh_aware_driver ? "partial" : "complete"));
^
   include/linux/printk.h:308:34: note: in definition of macro 'pr_info'
 printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
 ^~~
   arch/powerpc/kernel/eeh_driver.c:692:5: note: each undeclared identifier is 
reported only once for each function it appears in
   (eeh_aware_driver ? "partial" : "complete"));
^
   include/linux/printk.h:308:34: note: in definition of macro 'pr_info'
 printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
 ^~~

vim +692 arch/powerpc/kernel/eeh_driver.c

   619  
   620  /**
   621   * eeh_reset_device - Perform actual reset of a pci slot
   622   * @driver_eeh_aware: Does the device's driver provide EEH support?
   623   * @pe: EEH PE
   624   * @bus: PCI bus corresponding to the isolcated slot
   625   * @rmv_data: Optional, list to record removed devices
   626   *
   627   * This routine must be called to do reset on the indicated PE.
   628   * During the reset, udev might be invoked because those affected
   629   * PCI devices will be removed and then added.
   630   */
   631  static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
   632  struct eeh_rmv_data *rmv_data,
   633  bool driver_eeh_aware)
   634  {
   635  time64_t tstamp;
   636  int cnt, rc;
   637  struct eeh_dev *edev;
   638  
   639  /* pcibios will clear the counter; save the value */
   640  cnt = pe->freeze_count;
   641  tstamp = pe->tstamp;
   642  
   643  /*
   644   * We don't remove the corresponding PE instances because
   645   * we need the information afterwords. The attached EEH
   646   * devices are expected to be attached soon when calling
   647   * into pci_hp_add_devices().
   648   */
   649  eeh_pe_state_mark(pe, EEH_PE_KEEP);
   650  if (driver_eeh_aware || (pe->type & EEH_PE_VF)) {
   651  eeh_pe_dev_traverse(pe, eeh_rmv_device, rmv_data);
   652  } else {
   653  pci_lock_rescan_remove();
   654  pci_hp_remove_devices(bus);
   655  pci_unlock_rescan_remove();
   656  }
   657  
   658  /*
   659   * Reset the pci controller. (Asserts RST#; resets config 
space).
   660   * Reconfigure bridges and devices. Don't try to bring the 
system
   661   * up if the reset failed for some reason.
   662   *
   663   * During the reset, it's very dangerous to have uncontrolled 
PCI
   664   * config accesses. So we prefer to block them. However, 
controlled
   665   * PCI config accesses initiated from EEH itself are allowed.
   666   */
   667  rc = eeh_pe_reset_full(pe);
   668  if (rc)
   669  return rc;
   670  
   671  pci_lock_rescan_remove();
   672  
   673  /* Restore PE */
   674  eeh_ops->configure_bridge(pe);
   675  eeh_pe_restore_bars(pe);
   676  
   677  /* Clear frozen state */
   678  rc = eeh_clear_pe_frozen_state(pe, false);
   679  if (rc) {
   680  pci_unlock_rescan_remove();
   681  return rc;
   682  }
   683  
   684  /* Give the system 5 seconds to finish running the user-space
   685   * hotplug shutdown scripts, e.g. ifdown for ethernet.  Yes,
   686   * this is a hack, but if we don't do this, and try to bring
   687   * the device up before the scripts have taken it down,
 

[PATCH v2 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 
---
== v1 -> v2: ==

 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..aa3a2b08aec9 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",
+   (eeh_aware_driver ? "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 (!eeh_aware_driver)
+   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