qla2xxx: prevent board_disable from running during EEH

PATCH New Thadeu Lima de Souza Cascardo 4 participants 4 messages Apr 30, 2015 – Jun 26, 2015

Subsystems (2)

SCSI QLogic QLA2XXX FC-SCSI driver

lore

Share & tools

lore

Thadeu Lima de Souza Cascardo

author

2015-04-30 12:13 UTC #0 lore raw

PATCH 1/1 qla2xxx: prevent board_disable from running during EEH

Commit f3ddac1918fe963bcbf8d407a3a3c0881b47248b ("[SCSI] qla2xxx:
Disable adapter when we encounter a PCI disconnect.") has introduced a code that disables the board, releasing some resources, when reading 0xffffffff.

In case this happens when there is an EEH, this read will trigger EEH detection and set PCI channel offline. EEH will be able to recover the card from this state by doing a reset, so it's a better option than simply disabling the card.

Since eeh_check_failure will mark the channel as offline before returning the read value, in case there really was an EEH, we can simply check for pci_channel_offline, preventing the board_disable code from running if it's true.

Without this patch, EEH code will try to access those same resources that board_disable will try to free. This race can cause EEH recovery to fail.

[  504.370577] EEH: Notify device driver to resume
[  504.370580] qla2xxx [0001:07:00.0]-9002:2: The device failed to resume I/O from slot/link_reset.

Signed-off-by Thadeu Lima de Souza Cascardo <[email protected]>

Cc <[email protected]>

---
 drivers/scsi/qla2xxx/qla_isr.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/scsi/qla2xxx/qla_isr.c +1 -1

index a04a1b1..8132926 100644
@@ -116,7 +116,7 @@ bool
 qla2x00_check_reg32_for_disconnect(scsi_qla_host_t *vha, uint32_t reg)
 {
 	/* Check for PCI disconnection */
-	if (reg == 0xffffffff) {
+	if (reg == 0xffffffff && !pci_channel_offline(vha->hw->pdev)) {
 		if (!test_and_set_bit(PFLG_DISCONNECTED, &vha->pci_flags) &&
 		    !test_bit(PFLG_DRIVER_REMOVING, &vha->pci_flags) &&
 		    !test_bit(PFLG_DRIVER_PROBING, &vha->pci_flags)) {

Signature · 1 line

1.7.10.4

Mauricio Faria de Oliveira

2015-06-26 16:34 UTC #1 lore raw

PATCH RESEND 1/1 qla2xxx: prevent board_disable from running during EEH

Without this patch, EEH code will try to access those same resources that board_disable will try to free. This race can cause EEH recovery to fail.

[  504.370577] EEH: Notify device driver to resume
[  504.370580] qla2xxx [0001:07:00.0]-9002:2: The device failed to resume I/O from slot/link_reset.

Signed-off-by Thadeu Lima de Souza Cascardo <[email protected]>

Cc <[email protected]>

---
 drivers/scsi/qla2xxx/qla_isr.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/scsi/qla2xxx/qla_isr.c +1 -1

index a04a1b1..8132926 100644
@@ -116,7 +116,7 @@ bool
 qla2x00_check_reg32_for_disconnect(scsi_qla_host_t *vha, uint32_t reg)
 {
 	/* Check for PCI disconnection */
-	if (reg == 0xffffffff) {
+	if (reg == 0xffffffff && !pci_channel_offline(vha->hw->pdev)) {
 		if (!test_and_set_bit(PFLG_DISCONNECTED, &vha->pci_flags) &&
 		    !test_bit(PFLG_DRIVER_REMOVING, &vha->pci_flags) &&
 		    !test_bit(PFLG_DRIVER_PROBING, &vha->pci_flags)) {

Signature · 1 line

1.7.10.4

Himanshu Madhani

Acked-by on v1 1/1

2015-06-26 17:06 UTC #2 lore raw

Quoted 48 lines · Mauricio Faria de Oliveira

Commit f3ddac1918fe963bcbf8d407a3a3c0881b47248b ("[SCSI] qla2xxx:
Disable adapter when we encounter a PCI disconnect.") has introduced a code that disables the board, releasing some resources, when reading 0xffffffff.
In case this happens when there is an EEH, this read will trigger EEH detection and set PCI channel offline. EEH will be able to recover the card from this state by doing a reset, so it's a better option than simply disabling the card.
Since eeh_check_failure will mark the channel as offline before returning the read value, in case there really was an EEH, we can simply check for pci_channel_offline, preventing the board_disable code from running if it's true.
Without this patch, EEH code will try to access those same resources that board_disable will try to free. This race can cause EEH recovery to fail.
[  504.370577] EEH: Notify device driver to resume
[  504.370580] qla2xxx [0001:07:00.0]-9002:2: The device failed to
resume I/O from slot/link_reset.
Signed-off-by Thadeu Lima de Souza Cascardo <[email protected]>
Cc <[email protected]>
Cc <[email protected]>
---
 drivers/scsi/qla2xxx/qla_isr.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/scsi/qla2xxx/qla_isr.c +1 -1
index a04a1b1..8132926 100644
@@ -116,7 +116,7 @@ bool
 qla2x00_check_reg32_for_disconnect(scsi_qla_host_t *vha, uint32_t reg)
 {
 	/* Check for PCI disconnection */
-	if (reg == 0xffffffff) {
+	if (reg == 0xffffffff && !pci_channel_offline(vha->hw->pdev)) {
 		if (!test_and_set_bit(PFLG_DISCONNECTED, &vha->pci_flags) &&
 		    !test_bit(PFLG_DRIVER_REMOVING, &vha->pci_flags) &&
 		    !test_bit(PFLG_DRIVER_PROBING, &vha->pci_flags)) {
Signature · 1 line
1.7.10.4

Looks good.

Acked-by Himanshu Madhani <[email protected]>

Quoted 5 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Joe Lawrence

2015-06-26 21:24 UTC #3 lore raw

In reply to Mauricio Faria de Oliveira

Quoted 36 lines · Mauricio Faria de Oliveira

Commit f3ddac1918fe963bcbf8d407a3a3c0881b47248b ("[SCSI] qla2xxx:
Disable adapter when we encounter a PCI disconnect.") has introduced a code that disables the board, releasing some resources, when reading 0xffffffff.
In case this happens when there is an EEH, this read will trigger EEH detection and set PCI channel offline. EEH will be able to recover the card from this state by doing a reset, so it's a better option than simply disabling the card.
Since eeh_check_failure will mark the channel as offline before returning the read value, in case there really was an EEH, we can simply check for pci_channel_offline, preventing the board_disable code from running if it's true.
Without this patch, EEH code will try to access those same resources that board_disable will try to free. This race can cause EEH recovery to fail.
[  504.370577] EEH: Notify device driver to resume
[  504.370580] qla2xxx [0001:07:00.0]-9002:2: The device failed to
resume I/O from slot/link_reset.
Signed-off-by Thadeu Lima de Souza Cascardo <[email protected]>
Cc <[email protected]>
Cc <[email protected]>
---
 drivers/scsi/qla2xxx/qla_isr.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/scsi/qla2xxx/qla_isr.c +0 -0
index a04a1b1..8132926 100644

drivers/scsi/qla2xxx/qla_isr.c bool · line 122

Quoted 8 lines

drivers/scsi/qla2xxx/qla_isr.c +1 -1

@@ -116,7 +116,7 @@ bool
 qla2x00_check_reg32_for_disconnect(scsi_qla_host_t *vha, uint32_t reg)
 {
     /* Check for PCI disconnection */
-    if (reg == 0xffffffff) {
+    if (reg == 0xffffffff && !pci_channel_offline(vha->hw->pdev)) {
         if (!test_and_set_bit(PFLG_DISCONNECTED, &vha->pci_flags) &&
             !test_bit(PFLG_DRIVER_REMOVING, &vha->pci_flags) &&
             !test_bit(PFLG_DRIVER_PROBING, &vha->pci_flags)) {

Hi Mauricio,

Re: signed-off-by chain -- I believe if you are (re)sending another person's patch, you will need a: "From: Thadeu Lima de Souza Cascardo <[email protected]>" tag at the top of the message body to retain original authorship and then your own: "Signed-off-by: Mauricio Faria de Oliveira <[email protected]>" tag below Thadeu's since the patch has passed through you.

Re: the patch -- I did some work last year to harden board_disable against various races, but without having EEH hardware available, I was uncertain about EEH behavior. For example:

pci_read
EEH -> marks channel offline
pci_read returns ~0

When do the EEH error handler callbacks run?

Thanks,

-- Joe