[PATCH 07/11] qla4xxx: Avoid relogin on device marked missing

[PATCH 07/11] qla4xxx: Avoid relogin on device marked missing

[Ravi Anand]

From: Vikas Chaudhary <vikas.chaudhary@qlogic.com>

Signed-off-by: Nilesh Javali <nilesh.javali@qlogic.com>
Signed-off-by: Vikas Chaudhary <vikas.chaudhary@qlogic.com>
Signed-off-by: Ravi Anand <ravi.anand@qlogic.com>
---
 drivers/scsi/qla4xxx/ql4_os.c |   27 ++++++++++++++++++++++++---
 1 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index 9057860..e5c9b85 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -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);
 
 			/*
-- 
1.6.0.2

Re: [PATCH 07/11] qla4xxx: Avoid relogin on device marked missing

[Mike Christie]

On 01/30/2010 12:29 AM, Ravi Anand wrote:
>
> From: Vikas Chaudhary<vikas.chaudhary@qlogic.com>
>
> Signed-off-by: Nilesh Javali<nilesh.javali@qlogic.com>
> Signed-off-by: Vikas Chaudhary<vikas.chaudhary@qlogic.com>
> Signed-off-by: Ravi Anand<ravi.anand@qlogic.com>
> ---
>   drivers/scsi/qla4xxx/ql4_os.c |   27 ++++++++++++++++++++++++---
>   1 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
> index 9057860..e5c9b85 100644
> --- a/drivers/scsi/qla4xxx/ql4_os.c
> +++ b/drivers/scsi/qla4xxx/ql4_os.c
> @@ -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;
> +


How does the relogin work in this case?

- 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?

Re: [PATCH 07/11] qla4xxx: Avoid relogin on device marked missing

[Ravi Anand]

On Mon, 01 Feb 2010, Mike Christie wrote:

> 
> On 01/30/2010 12:29 AM, Ravi Anand wrote:
> >
> > From: Vikas Chaudhary<vikas.chaudhary@qlogic.com>
> >
> > Signed-off-by: Nilesh Javali<nilesh.javali@qlogic.com>
> > Signed-off-by: Vikas Chaudhary<vikas.chaudhary@qlogic.com>
> > Signed-off-by: Ravi Anand<ravi.anand@qlogic.com>
> > ---
> >   drivers/scsi/qla4xxx/ql4_os.c |   27 ++++++++++++++++++++++++---
> >   1 files changed, 24 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
> > index 9057860..e5c9b85 100644
> > --- a/drivers/scsi/qla4xxx/ql4_os.c
> > +++ b/drivers/scsi/qla4xxx/ql4_os.c
> > @@ -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;
> > +
> 
> 
> 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.

> 
> - 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?

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