fc class: terminate callback fixes

fc class: terminate callback fixes

[michaelc]

This fixes the problems discussed in this thread:
http://marc.info/?l=linux-scsi&m=121573444523292&w=2
where we need to call scsi_target_unblock in the code
path where the terminate call back is used. The unblock calls is
need so that IO does not get stuck when removing the rport. And for
the fast IO fail case, I merged the patch to fast fail
IO so it does not bounce around between the queue
and driver and is returned to the upper layer quickly.
These patch then solve all the issues and fix the problem
for drivers that were not implementing the callback.

The patches were made over scsi-misc and the qlogic update
in that thread.

[PATCH 1/2] fc class: unblock target after calling terminate callback

[michaelc]

From: Mike Christie <michaelc@cs.wisc.edu>

When we block a rport and the driver implements the terminate
callback we will fail IO that was running quickly. However
IO that was in the scsi_device/block queue sits there until
the dev_loss_tmo fires, and this can make it look like IO is
lost because new IO will get executed but that IO stuck in
the blocked queue sits there for some time longer.

With this patch when the fast io fail tmo fires, we will
fail the blocked IO and any new IO. This patch also allows
all drivers to partially support the fast io fail tmo. If the
terminate io callback is not implemented, we will still fail blocked
IO and any new IO, so multipath can handle that.

This patch also allows the fc and iscsi classes to implement the
same behavior. The timers are just unfornately named differently.

This patch also fixes the problem where drivers were unblocking
the target in their terminate callback, which was needed for
rport removal, but for fast io fail timeout it would cause
IO to bounce arround the scsi/block layer and the LLD queuecommand.
And it for drivers that could have IO stuck but did not have
a terminate callback the unblock calls in the class will fix
them.


Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/scsi_transport_fc.c |   43 +++++++++++++++++++++++---------------
 include/scsi/scsi_transport_fc.h |    6 ++++-
 2 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 5fd64e7..ba2ad97 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -2156,8 +2156,7 @@ fc_attach_transport(struct fc_function_template *ft)
 	SETUP_PRIVATE_RPORT_ATTRIBUTE_RD(roles);
 	SETUP_PRIVATE_RPORT_ATTRIBUTE_RD(port_state);
 	SETUP_PRIVATE_RPORT_ATTRIBUTE_RD(scsi_target_id);
-	if (ft->terminate_rport_io)
-		SETUP_PRIVATE_RPORT_ATTRIBUTE_RW(fast_io_fail_tmo);
+	SETUP_PRIVATE_RPORT_ATTRIBUTE_RW(fast_io_fail_tmo);
 
 	BUG_ON(count > FC_RPORT_NUM_ATTRS);
 
@@ -2351,6 +2350,22 @@ fc_remove_host(struct Scsi_Host *shost)
 }
 EXPORT_SYMBOL(fc_remove_host);
 
+static void fc_terminate_rport_io(struct fc_rport *rport)
+{
+	struct Scsi_Host *shost = rport_to_shost(rport);
+	struct fc_internal *i = to_fc_internal(shost->transportt);
+
+	/* Involve the LLDD if possible to terminate all io on the rport. */
+	if (i->f->terminate_rport_io)
+		i->f->terminate_rport_io(rport);
+
+	/*
+	 * must unblock to flush queued IO. The caller will have set
+	 * the port_state or flags, so that fc_remote_port_chkready will
+	 * fail IO.
+	 */
+	scsi_target_unblock(&rport->dev);
+}
 
 /**
  * fc_starget_delete - called to delete the scsi decendents of an rport
@@ -2363,13 +2378,8 @@ fc_starget_delete(struct work_struct *work)
 {
 	struct fc_rport *rport =
 		container_of(work, struct fc_rport, stgt_delete_work);
-	struct Scsi_Host *shost = rport_to_shost(rport);
-	struct fc_internal *i = to_fc_internal(shost->transportt);
-
-	/* Involve the LLDD if possible to terminate all io on the rport. */
-	if (i->f->terminate_rport_io)
-		i->f->terminate_rport_io(rport);
 
+	fc_terminate_rport_io(rport);
 	scsi_remove_target(&rport->dev);
 }
 
@@ -2395,10 +2405,7 @@ fc_rport_final_delete(struct work_struct *work)
 	if (rport->flags & FC_RPORT_SCAN_PENDING)
 		scsi_flush_work(shost);
 
-	/* involve the LLDD to terminate all pending i/o */
-	if (i->f->terminate_rport_io)
-		i->f->terminate_rport_io(rport);
-
+	fc_terminate_rport_io(rport);
 	/*
 	 * Cancel any outstanding timers. These should really exist
 	 * only when rmmod'ing the LLDD and we're asking for
@@ -2662,6 +2669,7 @@ fc_remote_port_add(struct Scsi_Host *shost, int channel,
 
 				spin_lock_irqsave(shost->host_lock, flags);
 
+				rport->flags &= ~FC_RPORT_FAST_FAIL_TIMEDOUT;
 				rport->flags &= ~FC_RPORT_DEVLOSS_PENDING;
 
 				/* if target, initiate a scan */
@@ -2725,6 +2733,7 @@ fc_remote_port_add(struct Scsi_Host *shost, int channel,
 			rport->port_id = ids->port_id;
 			rport->roles = ids->roles;
 			rport->port_state = FC_PORTSTATE_ONLINE;
+			rport->flags &= ~FC_RPORT_FAST_FAIL_TIMEDOUT;
 
 			if (fci->f->dd_fcrport_size)
 				memset(rport->dd_data, 0,
@@ -2807,7 +2816,6 @@ void
 fc_remote_port_delete(struct fc_rport  *rport)
 {
 	struct Scsi_Host *shost = rport_to_shost(rport);
-	struct fc_internal *i = to_fc_internal(shost->transportt);
 	int timeout = rport->dev_loss_tmo;
 	unsigned long flags;
 
@@ -2853,7 +2861,7 @@ fc_remote_port_delete(struct fc_rport  *rport)
 
 	/* see if we need to kill io faster than waiting for device loss */
 	if ((rport->fast_io_fail_tmo != -1) &&
-	    (rport->fast_io_fail_tmo < timeout) && (i->f->terminate_rport_io))
+	    (rport->fast_io_fail_tmo < timeout))
 		fc_queue_devloss_work(shost, &rport->fail_io_work,
 					rport->fast_io_fail_tmo * HZ);
 
@@ -2930,6 +2938,7 @@ fc_remote_port_rolechg(struct fc_rport  *rport, u32 roles)
 
 		spin_lock_irqsave(shost->host_lock, flags);
 		rport->flags &= ~FC_RPORT_DEVLOSS_PENDING;
+		rport->flags &= ~FC_RPORT_FAST_FAIL_TIMEDOUT;
 		spin_unlock_irqrestore(shost->host_lock, flags);
 
 		/* ensure any stgt delete functions are done */
@@ -3024,6 +3033,7 @@ fc_timeout_deleted_rport(struct work_struct *work)
 	rport->supported_classes = FC_COS_UNSPECIFIED;
 	rport->roles = FC_PORT_ROLE_UNKNOWN;
 	rport->port_state = FC_PORTSTATE_NOTPRESENT;
+	rport->flags &= ~FC_RPORT_FAST_FAIL_TIMEDOUT;
 
 	/* remove the identifiers that aren't used in the consisting binding */
 	switch (fc_host->tgtid_bind_type) {
@@ -3066,13 +3076,12 @@ fc_timeout_fail_rport_io(struct work_struct *work)
 {
 	struct fc_rport *rport =
 		container_of(work, struct fc_rport, fail_io_work.work);
-	struct Scsi_Host *shost = rport_to_shost(rport);
-	struct fc_internal *i = to_fc_internal(shost->transportt);
 
 	if (rport->port_state != FC_PORTSTATE_BLOCKED)
 		return;
 
-	i->f->terminate_rport_io(rport);
+	rport->flags |= FC_RPORT_FAST_FAIL_TIMEDOUT;
+	fc_terminate_rport_io(rport);
 }
 
 /**
diff --git a/include/scsi/scsi_transport_fc.h b/include/scsi/scsi_transport_fc.h
index 06f72ba..72fc9af 100644
--- a/include/scsi/scsi_transport_fc.h
+++ b/include/scsi/scsi_transport_fc.h
@@ -338,6 +338,7 @@ struct fc_rport {	/* aka fc_starget_attrs */
 /* bit field values for struct fc_rport "flags" field: */
 #define FC_RPORT_DEVLOSS_PENDING	0x01
 #define FC_RPORT_SCAN_PENDING		0x02
+#define FC_RPORT_FAST_FAIL_TIMEDOUT	0x03
 
 #define	dev_to_rport(d)				\
 	container_of(d, struct fc_rport, dev)
@@ -664,7 +665,10 @@ fc_remote_port_chkready(struct fc_rport *rport)
 			result = DID_NO_CONNECT << 16;
 		break;
 	case FC_PORTSTATE_BLOCKED:
-		result = DID_IMM_RETRY << 16;
+		if (rport->flags & FC_RPORT_FAST_FAIL_TIMEDOUT)
+			result = DID_NO_CONNECT << 16;
+		else
+			result = DID_IMM_RETRY << 16;
 		break;
 	default:
 		result = DID_NO_CONNECT << 16;
-- 
1.5.4.1

[PATCH 2/2] fc drivers: remove scsi_target_unblock calls in terminate callbacks

[michaelc]

From: Mike Christie <michaelc@cs.wisc.edu>

The fc class now calls scsi_target_unblock after calling the
terminate callback, so this patch removes the calls from the
drivers.

This patch was made over scsi-misc which has the ibmvfc driver.
It is also made over the latest qlogic patchset, which added
a terminate callback
patchset:
http://marc.info/?l=linux-scsi&m=121573412422984&w=2
patch with callabck addition
http://marc.info/?l=linux-scsi&m=121573444523292&w=2

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/ibmvscsi/ibmvfc.c   |    2 --
 drivers/scsi/lpfc/lpfc_hbadisc.c |    8 --------
 drivers/scsi/qla2xxx/qla_attr.c  |    1 -
 3 files changed, 0 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index eb702b9..f5a6982 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -2001,8 +2001,6 @@ static void ibmvfc_terminate_rport_io(struct fc_rport *rport)
 		spin_unlock_irqrestore(shost->host_lock, flags);
 	} else
 		ibmvfc_issue_fc_host_lip(shost);
-
-	scsi_target_unblock(&rport->dev);
 	LEAVE;
 }
 
diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index a98d11b..aaf398e 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -88,14 +88,6 @@ lpfc_terminate_rport_io(struct fc_rport *rport)
 			&phba->sli.ring[phba->sli.fcp_ring],
 			ndlp->nlp_sid, 0, LPFC_CTX_TGT);
 	}
-
-	/*
-	 * A device is normally blocked for rediscovery and unblocked when
-	 * devloss timeout happens.  In case a vport is removed or driver
-	 * unloaded before devloss timeout happens, we need to unblock here.
-	 */
-	scsi_target_unblock(&rport->dev);
-	return;
 }
 
 /*
diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index 612e3d0..95fc562 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -1019,7 +1019,6 @@ qla2x00_terminate_rport_io(struct fc_rport *rport)
 	fc_port_t *fcport = *(fc_port_t **)rport->dd_data;
 
 	qla2x00_abort_fcport_cmds(fcport);
-	scsi_target_unblock(&rport->dev);
 }
 
 static int
-- 
1.5.4.1

Re: fc class: terminate callback fixes

[Mike Christie]

michaelc@cs.wisc.edu wrote:
> This fixes the problems discussed in this thread:
> http://marc.info/?l=linux-scsi&m=121573444523292&w=2
> where we need to call scsi_target_unblock in the code
> path where the terminate call back is used. The unblock calls is
> need so that IO does not get stuck when removing the rport. And for
> the fast IO fail case, I merged the patch to fast fail
> IO so it does not bounce around between the queue
> and driver and is returned to the upper layer quickly.
> These patch then solve all the issues and fix the problem
> for drivers that were not implementing the callback.
> 
> The patches were made over scsi-misc and the qlogic update
> in that thread.
> 

Hey JamesS,

Are these patches ok?

Re: fc class: terminate callback fixes

[James Smart]

Yep - I just sent the ack...

Thanks

-- james s

Mike Christie wrote:
> michaelc@cs.wisc.edu wrote:
>> This fixes the problems discussed in this thread:
>> http://marc.info/?l=linux-scsi&m=121573444523292&w=2
>> where we need to call scsi_target_unblock in the code
>> path where the terminate call back is used. The unblock calls is
>> need so that IO does not get stuck when removing the rport. And for
>> the fast IO fail case, I merged the patch to fast fail
>> IO so it does not bounce around between the queue
>> and driver and is returned to the upper layer quickly.
>> These patch then solve all the issues and fix the problem
>> for drivers that were not implementing the callback.
>>
>> The patches were made over scsi-misc and the qlogic update
>> in that thread.
>>
> 
> Hey JamesS,
> 
> Are these patches ok?
> 

Re: fc class: terminate callback fixes

[Mike Christie]

James Smart wrote:
> Yep - I just sent the ack...
> 

Ok. I will rediff for the current scsi-misc and fix up the bit setting 
and resend.

Thanks.

> Thanks
> 
> -- james s
> 
> Mike Christie wrote:
>> michaelc@cs.wisc.edu wrote:
>>> This fixes the problems discussed in this thread:
>>> http://marc.info/?l=linux-scsi&m=121573444523292&w=2
>>> where we need to call scsi_target_unblock in the code
>>> path where the terminate call back is used. The unblock calls is
>>> need so that IO does not get stuck when removing the rport. And for
>>> the fast IO fail case, I merged the patch to fast fail
>>> IO so it does not bounce around between the queue
>>> and driver and is returned to the upper layer quickly.
>>> These patch then solve all the issues and fix the problem
>>> for drivers that were not implementing the callback.
>>>
>>> The patches were made over scsi-misc and the qlogic update
>>> in that thread.
>>>
>>
>> Hey JamesS,
>>
>> Are these patches ok?
>>
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html