[PATCH 0/5] qla2xxx: Patches for 3.7-rc.

[PATCH 0/5] qla2xxx: Patches for 3.7-rc.

[Saurav Kashyap]

Hi James,

Please apply the following patches for 3.7-rc.

Thanks,
~Saurav
 


Armen Baloyan (1):
  qla2xxx: Properly set result field of bsg_job reply structure for
    success and failure.

David Jeffery (1):
  qla2xxx: Test and clear FCPORT_UPDATE_NEEDED atomically.

Giridhar Malavali (1):
  qla2xxx: Change in setting UNLOADING flag and FC vports logout
    sequence while unloading qla2xxx driver.

Saurav Kashyap (1):
  qla2xxx: Update the driver version to 8.04.00.08-k.

Steve Hodgson (1):
  qla2xxx: Free rsp_data even on error in qla2x00_process_loopback()

 drivers/scsi/qla2xxx/qla_attr.c    |    3 +-
 drivers/scsi/qla2xxx/qla_bsg.c     |   72 ++++++++++++++----------------------
 drivers/scsi/qla2xxx/qla_os.c      |    7 +--
 drivers/scsi/qla2xxx/qla_version.h |    2 +-
 4 files changed, 33 insertions(+), 51 deletions(-)

-- 
1.7.7

[PATCH 1/5] qla2xxx: Properly set result field of bsg_job reply structure for success and failure.

[Saurav Kashyap]

From: Armen Baloyan <armen.baloyan@qlogic.com>

FC transport on receiving bsg_job submission failure, calls bsg_job->job_done()
and sets the bsg_job->reply->result the returned value. In contrast, when the
success code (0) is returned fc transport doesn't call bsg_job->job_done() and
doesn't populate bsg_job->reply->result.

Signed-off-by: Steve Hodgson <steve@purestorage.com>
Signed-off-by: Armen Baloyan <armen.baloyan@qlogic.com>
Signed-off-by: Saurav Kashyap <saurav.kashyap@qlogic.com>
---
 drivers/scsi/qla2xxx/qla_bsg.c |   65 +++++++++++++++-------------------------
 1 files changed, 24 insertions(+), 41 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
index 2f9bddd..05520be 100644
--- a/drivers/scsi/qla2xxx/qla_bsg.c
+++ b/drivers/scsi/qla2xxx/qla_bsg.c
@@ -219,7 +219,8 @@ qla24xx_proc_fcp_prio_cfg_cmd(struct fc_bsg_job *bsg_job)
 		break;
 	}
 exit_fcp_prio_cfg:
-	bsg_job->job_done(bsg_job);
+	if (!ret)
+		bsg_job->job_done(bsg_job);
 	return ret;
 }
 
@@ -741,7 +742,6 @@ qla2x00_process_loopback(struct fc_bsg_job *bsg_job)
 			if (qla81xx_get_port_config(vha, config)) {
 				ql_log(ql_log_warn, vha, 0x701f,
 				    "Get port config failed.\n");
-				bsg_job->reply->result = (DID_ERROR << 16);
 				rval = -EPERM;
 				goto done_free_dma_req;
 			}
@@ -761,7 +761,6 @@ qla2x00_process_loopback(struct fc_bsg_job *bsg_job)
 				    new_config, elreq.options);
 
 			if (rval) {
-				bsg_job->reply->result = (DID_ERROR << 16);
 				rval = -EPERM;
 				goto done_free_dma_req;
 			}
@@ -795,7 +794,6 @@ qla2x00_process_loopback(struct fc_bsg_job *bsg_job)
 					    "MPI reset failed.\n");
 				}
 
-				bsg_job->reply->result = (DID_ERROR << 16);
 				rval = -EIO;
 				goto done_free_dma_req;
 			}
@@ -812,33 +810,25 @@ qla2x00_process_loopback(struct fc_bsg_job *bsg_job)
 		ql_log(ql_log_warn, vha, 0x702c,
 		    "Vendor request %s failed.\n", type);
 
-		fw_sts_ptr = ((uint8_t *)bsg_job->req->sense) +
-		    sizeof(struct fc_bsg_reply);
-
-		memcpy(fw_sts_ptr, response, sizeof(response));
-		fw_sts_ptr += sizeof(response);
-		*fw_sts_ptr = command_sent;
 		rval = 0;
 		bsg_job->reply->result = (DID_ERROR << 16);
+		bsg_job->reply->reply_payload_rcv_len = 0;
 	} else {
 		ql_dbg(ql_dbg_user, vha, 0x702d,
 		    "Vendor request %s completed.\n", type);
-
-		bsg_job->reply_len = sizeof(struct fc_bsg_reply) +
-			sizeof(response) + sizeof(uint8_t);
-		bsg_job->reply->reply_payload_rcv_len =
-			bsg_job->reply_payload.payload_len;
-		fw_sts_ptr = ((uint8_t *)bsg_job->req->sense) +
-			sizeof(struct fc_bsg_reply);
-		memcpy(fw_sts_ptr, response, sizeof(response));
-		fw_sts_ptr += sizeof(response);
-		*fw_sts_ptr = command_sent;
-		bsg_job->reply->result = DID_OK;
+		bsg_job->reply->result = (DID_OK << 16);
 		sg_copy_from_buffer(bsg_job->reply_payload.sg_list,
 			bsg_job->reply_payload.sg_cnt, rsp_data,
 			rsp_data_len);
 	}
-	bsg_job->job_done(bsg_job);
+
+	bsg_job->reply_len = sizeof(struct fc_bsg_reply) +
+	    sizeof(response) + sizeof(uint8_t);
+	fw_sts_ptr = ((uint8_t *)bsg_job->req->sense) +
+	    sizeof(struct fc_bsg_reply);
+	memcpy(fw_sts_ptr, response, sizeof(response));
+	fw_sts_ptr += sizeof(response);
+	*fw_sts_ptr = command_sent;
 
 	dma_free_coherent(&ha->pdev->dev, rsp_data_len,
 		rsp_data, rsp_data_dma);
@@ -853,6 +843,8 @@ done_unmap_req_sg:
 	dma_unmap_sg(&ha->pdev->dev,
 	    bsg_job->request_payload.sg_list,
 	    bsg_job->request_payload.sg_cnt, DMA_TO_DEVICE);
+	if (!rval)
+		bsg_job->job_done(bsg_job);
 	return rval;
 }
 
@@ -877,16 +869,15 @@ qla84xx_reset(struct fc_bsg_job *bsg_job)
 	if (rval) {
 		ql_log(ql_log_warn, vha, 0x7030,
 		    "Vendor request 84xx reset failed.\n");
-		rval = 0;
-		bsg_job->reply->result = (DID_ERROR << 16);
+		rval = (DID_ERROR << 16);
 
 	} else {
 		ql_dbg(ql_dbg_user, vha, 0x7031,
 		    "Vendor request 84xx reset completed.\n");
 		bsg_job->reply->result = DID_OK;
+		bsg_job->job_done(bsg_job);
 	}
 
-	bsg_job->job_done(bsg_job);
 	return rval;
 }
 
@@ -976,8 +967,7 @@ qla84xx_updatefw(struct fc_bsg_job *bsg_job)
 		ql_log(ql_log_warn, vha, 0x7037,
 		    "Vendor request 84xx updatefw failed.\n");
 
-		rval = 0;
-		bsg_job->reply->result = (DID_ERROR << 16);
+		rval = (DID_ERROR << 16);
 	} else {
 		ql_dbg(ql_dbg_user, vha, 0x7038,
 		    "Vendor request 84xx updatefw completed.\n");
@@ -986,7 +976,6 @@ qla84xx_updatefw(struct fc_bsg_job *bsg_job)
 		bsg_job->reply->result = DID_OK;
 	}
 
-	bsg_job->job_done(bsg_job);
 	dma_pool_free(ha->s_dma_pool, mn, mn_dma);
 
 done_free_fw_buf:
@@ -996,6 +985,8 @@ done_unmap_sg:
 	dma_unmap_sg(&ha->pdev->dev, bsg_job->request_payload.sg_list,
 		bsg_job->request_payload.sg_cnt, DMA_TO_DEVICE);
 
+	if (!rval)
+		bsg_job->job_done(bsg_job);
 	return rval;
 }
 
@@ -1163,8 +1154,7 @@ qla84xx_mgmt_cmd(struct fc_bsg_job *bsg_job)
 		ql_log(ql_log_warn, vha, 0x7043,
 		    "Vendor request 84xx mgmt failed.\n");
 
-		rval = 0;
-		bsg_job->reply->result = (DID_ERROR << 16);
+		rval = (DID_ERROR << 16);
 
 	} else {
 		ql_dbg(ql_dbg_user, vha, 0x7044,
@@ -1184,8 +1174,6 @@ qla84xx_mgmt_cmd(struct fc_bsg_job *bsg_job)
 		}
 	}
 
-	bsg_job->job_done(bsg_job);
-
 done_unmap_sg:
 	if (mgmt_b)
 		dma_free_coherent(&ha->pdev->dev, data_len, mgmt_b, mgmt_dma);
@@ -1200,6 +1188,8 @@ done_unmap_sg:
 exit_mgmt:
 	dma_pool_free(ha->s_dma_pool, mn, mn_dma);
 
+	if (!rval)
+		bsg_job->job_done(bsg_job);
 	return rval;
 }
 
@@ -1276,9 +1266,7 @@ qla24xx_iidma(struct fc_bsg_job *bsg_job)
 		    fcport->port_name[3], fcport->port_name[4],
 		    fcport->port_name[5], fcport->port_name[6],
 		    fcport->port_name[7], rval, fcport->fp_speed, mb[0], mb[1]);
-		rval = 0;
-		bsg_job->reply->result = (DID_ERROR << 16);
-
+		rval = (DID_ERROR << 16);
 	} else {
 		if (!port_param->mode) {
 			bsg_job->reply_len = sizeof(struct fc_bsg_reply) +
@@ -1292,9 +1280,9 @@ qla24xx_iidma(struct fc_bsg_job *bsg_job)
 		}
 
 		bsg_job->reply->result = DID_OK;
+		bsg_job->job_done(bsg_job);
 	}
 
-	bsg_job->job_done(bsg_job);
 	return rval;
 }
 
@@ -1887,8 +1875,6 @@ qla2x00_process_vendor_specific(struct fc_bsg_job *bsg_job)
 		return qla24xx_process_bidir_cmd(bsg_job);
 
 	default:
-		bsg_job->reply->result = (DID_ERROR << 16);
-		bsg_job->job_done(bsg_job);
 		return -ENOSYS;
 	}
 }
@@ -1919,8 +1905,6 @@ qla24xx_bsg_request(struct fc_bsg_job *bsg_job)
 		ql_dbg(ql_dbg_user, vha, 0x709f,
 		    "BSG: ISP abort active/needed -- cmd=%d.\n",
 		    bsg_job->request->msgcode);
-		bsg_job->reply->result = (DID_ERROR << 16);
-		bsg_job->job_done(bsg_job);
 		return -EBUSY;
 	}
 
@@ -1943,7 +1927,6 @@ qla24xx_bsg_request(struct fc_bsg_job *bsg_job)
 	case FC_BSG_RPT_CT:
 	default:
 		ql_log(ql_log_warn, vha, 0x705a, "Unsupported BSG request.\n");
-		bsg_job->reply->result = ret;
 		break;
 	}
 	return ret;
-- 
1.7.7

[PATCH 2/5] qla2xxx: Test and clear FCPORT_UPDATE_NEEDED atomically.

[Saurav Kashyap]

From: David Jeffery <djeffery@redhat.com>

When the qla2xxx driver loses access to multiple, remote ports, there is a race
condition which can occur which will keep the request stuck on a scsi request
queue indefinitely.

This bad state occurred do to a race condition with how the FCPORT_UPDATE_NEEDED
bit is set in qla2x00_schedule_rport_del(), and how it is cleared in
qla2x00_do_dpc().  The problem port has its drport pointer set, but it has never
been processed by the driver to inform the fc transport that the port has been
lost.  qla2x00_schedule_rport_del() sets drport, and then sets the
FCPORT_UPDATE_NEEDED bit.  In qla2x00_do_dpc(), the port lists are walked and
any drport pointer is handled and the fc transport informed of the port loss,
then the FCPORT_UPDATE_NEEDED bit is cleared.  This leaves a race where the
dpc thread is processing one port removal, another port removal is marked
with a call to qla2x00_schedule_rport_del(), and the dpc thread clears the
bit for both removals, even though only the first removal was actually
handled.  Until another event occurs to set FCPORT_UPDATE_NEEDED, the later
port removal is never finished and qla2xxx stays in a bad state which causes
requests to become stuck on request queues.

This patch updates the driver to test and clear FCPORT_UPDATE_NEEDED
atomically.  This ensures the port state changes are processed and not lost.

Signed-off-by: David Jeffery <djeffery@redhat.com>
Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com>
Cc: stable@vger.kernel.org
Signed-off-by: Saurav Kashyap <saurav.kashyap@qlogic.com>
---
 drivers/scsi/qla2xxx/qla_os.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index d501bf5..a603967 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -4505,9 +4505,9 @@ qla2x00_do_dpc(void *data)
 			    "ISP abort end.\n");
 		}
 
-		if (test_bit(FCPORT_UPDATE_NEEDED, &base_vha->dpc_flags)) {
+		if (test_and_clear_bit(FCPORT_UPDATE_NEEDED,
+		    &base_vha->dpc_flags)) {
 			qla2x00_update_fcports(base_vha);
-			clear_bit(FCPORT_UPDATE_NEEDED, &base_vha->dpc_flags);
 		}
 
 		if (test_bit(SCR_PENDING, &base_vha->dpc_flags)) {
-- 
1.7.7

Re: [PATCH 2/5] qla2xxx: Test and clear FCPORT_UPDATE_NEEDED atomically.

[Saurav Kashyap]

Hi,
This needs to be copied to stable, added in cc.

Thanks,
~Saurav



>From: David Jeffery <djeffery@redhat.com>
>
>When the qla2xxx driver loses access to multiple, remote ports, there is
>a race
>condition which can occur which will keep the request stuck on a scsi
>request
>queue indefinitely.
>
>This bad state occurred do to a race condition with how the
>FCPORT_UPDATE_NEEDED
>bit is set in qla2x00_schedule_rport_del(), and how it is cleared in
>qla2x00_do_dpc().  The problem port has its drport pointer set, but it
>has never
>been processed by the driver to inform the fc transport that the port has
>been
>lost.  qla2x00_schedule_rport_del() sets drport, and then sets the
>FCPORT_UPDATE_NEEDED bit.  In qla2x00_do_dpc(), the port lists are walked
>and
>any drport pointer is handled and the fc transport informed of the port
>loss,
>then the FCPORT_UPDATE_NEEDED bit is cleared.  This leaves a race where
>the
>dpc thread is processing one port removal, another port removal is marked
>with a call to qla2x00_schedule_rport_del(), and the dpc thread clears the
>bit for both removals, even though only the first removal was actually
>handled.  Until another event occurs to set FCPORT_UPDATE_NEEDED, the
>later
>port removal is never finished and qla2xxx stays in a bad state which
>causes
>requests to become stuck on request queues.
>
>This patch updates the driver to test and clear FCPORT_UPDATE_NEEDED
>atomically.  This ensures the port state changes are processed and not
>lost.
>
>Signed-off-by: David Jeffery <djeffery@redhat.com>
>Signed-off-by: Chad Dupuis <chad.dupuis@qlogic.com>
>Cc: stable@vger.kernel.org
>Signed-off-by: Saurav Kashyap <saurav.kashyap@qlogic.com>
>---
> drivers/scsi/qla2xxx/qla_os.c |    4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
>index d501bf5..a603967 100644
>--- a/drivers/scsi/qla2xxx/qla_os.c
>+++ b/drivers/scsi/qla2xxx/qla_os.c
>@@ -4505,9 +4505,9 @@ qla2x00_do_dpc(void *data)
>                           "ISP abort end.\n");
>               }
>
>-              if (test_bit(FCPORT_UPDATE_NEEDED, &base_vha->dpc_flags)) {
>+              if (test_and_clear_bit(FCPORT_UPDATE_NEEDED,
>+                  &base_vha->dpc_flags)) {
>                       qla2x00_update_fcports(base_vha);
>-                      clear_bit(FCPORT_UPDATE_NEEDED, &base_vha->dpc_flags);
>               }
>
>               if (test_bit(SCR_PENDING, &base_vha->dpc_flags)) {
>--
>1.7.7
>


________________________________

This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.

[PATCH 3/5] qla2xxx: Change in setting UNLOADING flag and FC vports logout sequence while unloading qla2xxx driver.

[Saurav Kashyap]

From: Giridhar Malavali <giridhar.malavali@qlogic.com>

Signed-off-by: Giridhar Malavali <giridhar.malavali@qlogic.com>
Signed-off-by: Saurav Kashyap <saurav.kashyap@qlogic.com>
---
 drivers/scsi/qla2xxx/qla_attr.c |    3 +--
 drivers/scsi/qla2xxx/qla_os.c   |    3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index 1c28215..83d7984 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -1615,8 +1615,7 @@ qla2x00_terminate_rport_io(struct fc_rport *rport)
 	 * At this point all fcport's software-states are cleared.  Perform any
 	 * final cleanup of firmware resources (PCBs and XCBs).
 	 */
-	if (fcport->loop_id != FC_NO_LOOP_ID &&
-	    !test_bit(UNLOADING, &fcport->vha->dpc_flags)) {
+	if (fcport->loop_id != FC_NO_LOOP_ID) {
 		if (IS_FWI2_CAPABLE(fcport->vha->hw))
 			fcport->vha->hw->isp_ops->fabric_logout(fcport->vha,
 			    fcport->loop_id, fcport->d_id.b.domain,
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index a603967..f4b1fc8 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -2755,6 +2755,7 @@ qla2x00_remove_one(struct pci_dev *pdev)
 
 	ha->flags.host_shutting_down = 1;
 
+	set_bit(UNLOADING, &base_vha->dpc_flags);
 	mutex_lock(&ha->vport_lock);
 	while (ha->cur_vport_count) {
 		struct Scsi_Host *scsi_host;
@@ -2784,8 +2785,6 @@ qla2x00_remove_one(struct pci_dev *pdev)
 			    "Error while clearing DRV-Presence.\n");
 	}
 
-	set_bit(UNLOADING, &base_vha->dpc_flags);
-
 	qla2x00_abort_all_cmds(base_vha, DID_NO_CONNECT << 16);
 
 	qla2x00_dfs_remove(base_vha);
-- 
1.7.7

[PATCH 4/5] qla2xxx: Free rsp_data even on error in qla2x00_process_loopback()

[Saurav Kashyap]

From: Steve Hodgson <steve@purestorage.com>

Signed-off-by: Steve Hodgson <steve@purestorage.com>
Signed-off-by: Armen Baloyan <armen.baloyan@qlogic.com>
Signed-off-by: Saurav Kashyap <saurav.kashyap@qlogic.com>
---
 drivers/scsi/qla2xxx/qla_bsg.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
index 05520be..9f34ded 100644
--- a/drivers/scsi/qla2xxx/qla_bsg.c
+++ b/drivers/scsi/qla2xxx/qla_bsg.c
@@ -743,7 +743,7 @@ qla2x00_process_loopback(struct fc_bsg_job *bsg_job)
 				ql_log(ql_log_warn, vha, 0x701f,
 				    "Get port config failed.\n");
 				rval = -EPERM;
-				goto done_free_dma_req;
+				goto done_free_dma_rsp;
 			}
 
 			ql_dbg(ql_dbg_user, vha, 0x70c0,
@@ -762,7 +762,7 @@ qla2x00_process_loopback(struct fc_bsg_job *bsg_job)
 
 			if (rval) {
 				rval = -EPERM;
-				goto done_free_dma_req;
+				goto done_free_dma_rsp;
 			}
 
 			type = "FC_BSG_HST_VENDOR_LOOPBACK";
@@ -795,7 +795,7 @@ qla2x00_process_loopback(struct fc_bsg_job *bsg_job)
 				}
 
 				rval = -EIO;
-				goto done_free_dma_req;
+				goto done_free_dma_rsp;
 			}
 		} else {
 			type = "FC_BSG_HST_VENDOR_LOOPBACK";
@@ -830,6 +830,7 @@ qla2x00_process_loopback(struct fc_bsg_job *bsg_job)
 	fw_sts_ptr += sizeof(response);
 	*fw_sts_ptr = command_sent;
 
+done_free_dma_rsp:
 	dma_free_coherent(&ha->pdev->dev, rsp_data_len,
 		rsp_data, rsp_data_dma);
 done_free_dma_req:
-- 
1.7.7

[PATCH 5/5] qla2xxx: Update the driver version to 8.04.00.08-k.

[Saurav Kashyap]

Signed-off-by: Giridhar Malavali <giridhar.malavali@qlogic.com>
Signed-off-by: Saurav Kashyap <saurav.kashyap@qlogic.com>
---
 drivers/scsi/qla2xxx/qla_version.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_version.h b/drivers/scsi/qla2xxx/qla_version.h
index cfe934e..49697ca 100644
--- a/drivers/scsi/qla2xxx/qla_version.h
+++ b/drivers/scsi/qla2xxx/qla_version.h
@@ -7,7 +7,7 @@
 /*
  * Driver version
  */
-#define QLA2XXX_VERSION      "8.04.00.07-k"
+#define QLA2XXX_VERSION      "8.04.00.08-k"
 
 #define QLA_DRIVER_MAJOR_VER	8
 #define QLA_DRIVER_MINOR_VER	4
-- 
1.7.7