qla4xxx: Avoid relogin on device marked missing

PATCH In review Ravi Anand 2 participants 3 messages Jan 30, 2010 – Feb 11, 2010

Subsystems (2)

SCSI QLogic QLA4XXX iSCSI driver

lore

Share & tools

lore

Ravi Anand

author

2010-01-30 06:27 UTC #0 lore raw

PATCH 7/11 qla4xxx: Avoid relogin on device marked missing

From: Vikas Chaudhary <[email protected]>

Signed-off-by Nilesh Javali <[email protected]>

Signed-off-by Vikas Chaudhary <[email protected]>

Signed-off-by Ravi Anand <[email protected]>

---
 drivers/scsi/qla4xxx/ql4_os.c |   27 ++++++++++++++++++++++++---
 1 files changed, 24 insertions(+), 3 deletions(-)

drivers/scsi/qla4xxx/ql4_os.c +24 -3

index 9057860..e5c9b85 100644
@@ -370,6 +370,7 @@ void qla4xxx_mark_device_missing(struct scsi_qla_host *ha,
 		      ddb_entry->fw_ddb_index));
 	iscsi_block_session(ddb_entry->sess);
 	iscsi_conn_error_event(ddb_entry->conn, ISCSI_ERR_CONN_FAILED);
+	set_bit(DF_NO_RELOGIN, &ddb_entry->flags);
 }
 
 /***

@@ -642,6 +643,24 @@ static void qla4xxx_timer(struct scsi_qla_host *ha)
 
 	/* Search for relogin's to time-out and port down retry. */
 	list_for_each_entry_safe(ddb_entry, dtemp, &ha->ddb_list, list) {
+		/* First check to see if the device has exhausted the
+		 * port down retry count */
+		if (atomic_read(&ddb_entry->state) == DDB_STATE_MISSING) {
+			if (atomic_read(&ddb_entry->port_down_timer) == 0)
+				continue;
+
+			if (atomic_dec_and_test(&ddb_entry->port_down_timer)) {
+				DEBUG2(printk("scsi%ld: %s: index [%d] "
+					"port down retry count of (%d) secs "
+					"exhausted.\n",
+					ha->host_no, __func__,
+					ddb_entry->fw_ddb_index,
+					ha->port_down_retry_count);)
+
+				atomic_set(&ddb_entry->state, DDB_STATE_DEAD);
+				start_dpc++;
+			}
+		}
 		/* Count down time between sending relogins */
 		if (adapter_up(ha) &&
 		    !test_bit(DF_RELOGIN, &ddb_entry->flags) &&

@@ -676,7 +695,8 @@ static void qla4xxx_timer(struct scsi_qla_host *ha)
 			if (atomic_read(&ddb_entry->state) !=
 			    DDB_STATE_ONLINE &&
 			    ddb_entry->fw_ddb_device_state ==
-			    DDB_DS_SESSION_FAILED) {
+			    DDB_DS_SESSION_FAILED &&
+			    !test_bit(DF_NO_RELOGIN, &ddb_entry->flags)) {
 				/* Reset retry relogin timer */
 				atomic_inc(&ddb_entry->relogin_retry_count);
 				DEBUG2(printk("scsi%ld: index[%d] relogin"

@@ -1111,8 +1131,9 @@ static void qla4xxx_do_dpc(struct work_struct *work)
 	    test_and_clear_bit(DPC_RELOGIN_DEVICE, &ha->dpc_flags)) {
 		list_for_each_entry_safe(ddb_entry, dtemp,
 					 &ha->ddb_list, list) {
-			if (test_and_clear_bit(DF_RELOGIN, &ddb_entry->flags) &&
-			    atomic_read(&ddb_entry->state) != DDB_STATE_ONLINE)
+			if ((test_and_clear_bit(DF_RELOGIN, &ddb_entry->flags)) &&
+			    (!test_bit(DF_NO_RELOGIN, &ddb_entry->flags)) &&
+			    (atomic_read(&ddb_entry->state) != DDB_STATE_ONLINE))
 				qla4xxx_relogin_device(ha, ddb_entry);
 
 			/*

Signature · 1 line

1.6.0.2

- If you pull a cable and we mark devices missing, then when link up is detected will qla4xxx_isr_decode_mailbox set the AF_LINK_UP bit and then the driver will start to try and relogin? Or would you get a MBOX_ASTS_DATABASE_CHANGED aen, and then that notifies you that that the firmware detected link up and logged back in (so the state would then be DDB_DS_SESSION_ACTIVE?)?

- What cases do you get a databased change aen where the new device state indicates the session is non active so qla4xxx_process_ddb_changed calls qla4xxx_mark_device_missing? Would it be where the target logs us out, or when the firmware detects a problem with the session/connection and drop the session, what about if the firmware detects a link down?

In the case qla4xxx_process_ddb_changed calls qla4xxx_mark_device_missing will we get some sort of databased change aen indicating that the state is again DDB_DS_SESSION_ACTIVE?

- What cases do we now use the relogin code now? Do we always call qla4xxx_mark_device_missing in cases where a relogin is needed, so can we just remove the relogin code? Or is the relogin code still used for the initial driver initialization or something else?

Ravi Anand

author

2010-02-11 11:07 UTC #2 lore raw

Quoted 26 lines · Mike Christie

Quoted 24 lines · Ravi Anand

From: Vikas Chaudhary<[email protected]>

Signed-off-by Nilesh Javali<[email protected]>

Signed-off-by Vikas Chaudhary<[email protected]>

Signed-off-by Ravi Anand<[email protected]>

---
  drivers/scsi/qla4xxx/ql4_os.c |   27 ++++++++++++++++++++++++---
  1 files changed, 24 insertions(+), 3 deletions(-)

drivers/scsi/qla4xxx/ql4_os.c +1 -0

index 9057860..e5c9b85 100644
@@ -370,6 +370,7 @@ void qla4xxx_mark_device_missing(struct scsi_qla_host *ha,
  		      ddb_entry->fw_ddb_index));
  	iscsi_block_session(ddb_entry->sess);
  	iscsi_conn_error_event(ddb_entry->conn, ISCSI_ERR_CONN_FAILED);
+	set_bit(DF_NO_RELOGIN,&ddb_entry->flags);
  }
 
  /***

@@ -642,6 +643,24 @@ static void qla4xxx_timer(struct scsi_qla_host *ha)

Quoted 11 lines

drivers/scsi/qla4xxx/ql4_os.c static void qla4xxx_timer(struct scsi_qla_host *ha) · line 651

Quoted 8 lines

drivers/scsi/qla4xxx/ql4_os.c +6 -0

  	/* Search for relogin's to time-out and port down retry. */
  	list_for_each_entry_safe(ddb_entry, dtemp,&ha->ddb_list, list) {
+		/* First check to see if the device has exhausted the
+		 * port down retry count */
+		if (atomic_read(&ddb_entry->state) == DDB_STATE_MISSING) {
+			if (atomic_read(&ddb_entry->port_down_timer) == 0)
+				continue;
+

How does the relogin work in this case?

Actually it does not. We have fixed it and will resubmit an updated patch along with other patches to address the review comments.

Quoted 15 lines

- If you pull a cable and we mark devices missing, then when link up is detected will qla4xxx_isr_decode_mailbox set the AF_LINK_UP bit and then the driver will start to try and relogin? Or would you get a MBOX_ASTS_DATABASE_CHANGED aen, and then that notifies you that that the firmware detected link up and logged back in (so the state would then be DDB_DS_SESSION_ACTIVE?)?
- What cases do you get a databased change aen where the new device state indicates the session is non active so qla4xxx_process_ddb_changed calls qla4xxx_mark_device_missing? Would it be where the target logs us out, or when the firmware detects a problem with the session/connection and drop the session, what about if the firmware detects a link down?

Quoted 8 lines

In the case qla4xxx_process_ddb_changed calls qla4xxx_mark_device_missing will we get some sort of databased change aen indicating that the state is again DDB_DS_SESSION_ACTIVE?
- What cases do we now use the relogin code now? Do we always call qla4xxx_mark_device_missing in cases where a relogin is needed, so can we just remove the relogin code? Or is the relogin code still used for the initial driver initialization or something else?

F/W does the auto login only once during the init time once the link is up. After that any time if there is any connection failure or I/O timeout resulting in tear down of connection etc, its driver responsibility to relogin.

For cable pull scenario's if there are no I/O oustanding and KATO expires, in that case driver will receive 8014 indicating that the target was logged out. Again in that case its driver responsibility to log back in.

So doing a set_ddb() will result in re-establising the session and F/W will report _SESSION_ACTIVE via 8014.

Thanks for the feedback. We will resubmit all patches as *V2* which should address your concern.

THanks
Ravi