Skip to contentSkip to conversation

ipr: fix EEH recovery

PATCH In review Kleber Sacilotto de Souza 2 participants 3 messages Nov 25, 2009 – Nov 27, 2009

Subsystems (2)
SCSI IBM Power Linux RAID adapter
lore
Share & tools
lore

Kleber Sacilotto de Souza

author
#0 lore raw

PATCH 1/1 ipr: fix EEH recovery

Hi,

After commits c82f63e411f1b58427c103bd95af2863b1c96dd1 (PCI: check saved state before restore) and 4b77b0a2ba27d64f58f16d8d4d48d8319dda36ff (PCI:
Clear saved_state after the state has been restored) PCI drivers are prevented from restoring the device standard configuration registers twice in a row. These changes introduced a regression on ipr EEH recovery.

The ipr device driver saves the PCI state only during the device probe and restores it on ipr_reset_restore_cfg_space() during IOA resets. This behavior is causing the EEH recovery to fail after the second error detected, since the registers are not being restored.

One possible solution would be saving the registers after restoring them. The problem with this approach is that while recovering from an EEH error if pci_save_state() results in an EEH error, the adapter/slot will be reset, and end up back in ipr_reset_restore_cfg_space(), but it won't have a valid saved state to restore, so pci_restore_state() will fail.

The following patch introduces a workaround for this problem, hacking around the PCI API by setting pdev->state_saved = true before we do the restore. It fixes the EEH regression and prevents that we hit another EEH error during EEH recovery.

Thanks,
Kleber

Signed-off-by Kleber Sacilotto de Souza <[email protected]>
---
 drivers/scsi/ipr.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/scsi/ipr.c +1 -0
index 76d294f..c3ff9a6 100644
@@ -6516,6 +6516,7 @@ static int ipr_reset_restore_cfg_space(struct ipr_cmnd *ipr_cmd)
 	int rc;
 
 	ENTER;
+	ioa_cfg->pdev->state_saved = true;
 	rc = pci_restore_state(ioa_cfg->pdev);
 
 	if (rc != PCIBIOS_SUCCESSFUL) {
Signature · 3 lines
-- 
Kleber S. Souza
IBM Linux Technology Center
Acked-by on v1 1/1
#1 lore raw
Acked-by Brian King <[email protected]>
Quoted 49 lines · Kleber Sacilotto de Souza

Hi,

After commits c82f63e411f1b58427c103bd95af2863b1c96dd1 (PCI: check saved state before restore) and 4b77b0a2ba27d64f58f16d8d4d48d8319dda36ff (PCI:
Clear saved_state after the state has been restored) PCI drivers are prevented from restoring the device standard configuration registers twice in a row. These changes introduced a regression on ipr EEH recovery.

The ipr device driver saves the PCI state only during the device probe and restores it on ipr_reset_restore_cfg_space() during IOA resets. This behavior is causing the EEH recovery to fail after the second error detected, since the registers are not being restored.

One possible solution would be saving the registers after restoring them. The problem with this approach is that while recovering from an EEH error if pci_save_state() results in an EEH error, the adapter/slot will be reset, and end up back in ipr_reset_restore_cfg_space(), but it won't have a valid saved state to restore, so pci_restore_state() will fail.

The following patch introduces a workaround for this problem, hacking around the PCI API by setting pdev->state_saved = true before we do the restore. It fixes the EEH regression and prevents that we hit another EEH error during EEH recovery.

Thanks,
Kleber

Signed-off-by Kleber Sacilotto de Souza <[email protected]>
---
 drivers/scsi/ipr.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/scsi/ipr.c +1 -0
index 76d294f..c3ff9a6 100644
@@ -6516,6 +6516,7 @@ static int ipr_reset_restore_cfg_space(struct ipr_cmnd *ipr_cmd)
 	int rc;
 
 	ENTER;
+	ioa_cfg->pdev->state_saved = true;
 	rc = pci_restore_state(ioa_cfg->pdev);
 
 	if (rc != PCIBIOS_SUCCESSFUL) {
Signature · 3 lines
Brian King
Linux on Power Virtualization
IBM Linux Technology Center
author
#2 lore raw
Quoted 15 lines · Brian King
Acked-by Brian King <[email protected]>
Quoted 13 lines
Signed-off-by Kleber Sacilotto de Souza <[email protected]>
---
 drivers/scsi/ipr.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/scsi/ipr.c +0 -0
index 76d294f..c3ff9a6 100644
@@ -6516,6 +6516,7 @@ static int ipr_reset_restore_cfg_space(struct ipr_cmnd *ipr_cmd)
 	int rc;
Quoted 7 lines

drivers/scsi/ipr.c static int ipr_reset_restore_cfg_space(struct ipr_cmnd *ipr_cmd) · line 6522

Quoted 5 lines
drivers/scsi/ipr.c +1 -0
 	ENTER;
+	ioa_cfg->pdev->state_saved = true;
 	rc = pci_restore_state(ioa_cfg->pdev);
 
 	if (rc != PCIBIOS_SUCCESSFUL) {

Since this is a regression, is there any chance to have this patch applied and merged for 2.6.32?

Thanks,

Signature · 2 lines
Kleber S. Souza
IBM Linux Technology Center