diff mbox

[v4,03/17] be2iscsi: Fix to use atomic bit operations for tag_state

Message ID 1453279261-659-4-git-send-email-jitendra.bhivare@avagotech.com (mailing list archive)
State Accepted, archived
Headers show

Commit Message

Jitendra Bhivare Jan. 20, 2016, 8:40 a.m. UTC 
beiscsi_mccq_compl sets MCC_TAG_STATE_TIMEOUT before setting up
tag_mem_state. be_mcc_compl_process_isr checks for
MCC_TAG_STATE_TIMEOUT first then accesses tag_mem_state which might be
still getting populated in the process context.

Fix:
Set MCC_TAG_STATE_TIMEOUT after tag_mem_state is populated.
Removed MCC_TAG_STATE_COMPLETED. When posted its in running state and
the running state is cleared in be_mcc_compl_process_isr.
be_mcc_notify now takes tag argument to set it to running state.
Use bit operations for tag_state. Use barriers before setting the state.

Signed-off-by: Jitendra Bhivare <jitendra.bhivare@avagotech.com>
---
 drivers/scsi/be2iscsi/be.h      |   7 ++-
 drivers/scsi/be2iscsi/be_cmds.c | 111 ++++++++++++++++++++--------------------
 drivers/scsi/be2iscsi/be_cmds.h |   4 +-
 drivers/scsi/be2iscsi/be_mgmt.c |  39 ++++++++------
 4 files changed, 84 insertions(+), 77 deletions(-)

Comments

Hannes Reinecke Jan. 20, 2016, 10:13 a.m. UTC | #1 
On 01/20/2016 09:40 AM, Jitendra Bhivare wrote:
> beiscsi_mccq_compl sets MCC_TAG_STATE_TIMEOUT before setting up
> tag_mem_state. be_mcc_compl_process_isr checks for
> MCC_TAG_STATE_TIMEOUT first then accesses tag_mem_state which might be
> still getting populated in the process context.
> 
> Fix:
> Set MCC_TAG_STATE_TIMEOUT after tag_mem_state is populated.
> Removed MCC_TAG_STATE_COMPLETED. When posted its in running state and
> the running state is cleared in be_mcc_compl_process_isr.
> be_mcc_notify now takes tag argument to set it to running state.
> Use bit operations for tag_state. Use barriers before setting the state.
> 
> Signed-off-by: Jitendra Bhivare <jitendra.bhivare@avagotech.com>
> ---
>  drivers/scsi/be2iscsi/be.h      |   7 ++-
>  drivers/scsi/be2iscsi/be_cmds.c | 111 ++++++++++++++++++++--------------------
>  drivers/scsi/be2iscsi/be_cmds.h |   4 +-
>  drivers/scsi/be2iscsi/be_mgmt.c |  39 ++++++++------
>  4 files changed, 84 insertions(+), 77 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
diff mbox

Patch

diff --git a/drivers/scsi/be2iscsi/be.h b/drivers/scsi/be2iscsi/be.h
index cf19bce..629c53d 100644
--- a/drivers/scsi/be2iscsi/be.h
+++ b/drivers/scsi/be2iscsi/be.h
@@ -110,10 +110,9 @@  struct be_mcc_obj {
 };
 
 struct beiscsi_mcc_tag_state {
-#define MCC_TAG_STATE_COMPLETED 0x00
-#define MCC_TAG_STATE_RUNNING   0x01
-#define MCC_TAG_STATE_TIMEOUT   0x02
-	uint8_t tag_state;
+	unsigned long tag_state;
+#define MCC_TAG_STATE_RUNNING	1
+#define MCC_TAG_STATE_TIMEOUT	2
 	struct be_dma_mem tag_mem_state;
 };
 
diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
index 6fabded..1913e9e 100644
--- a/drivers/scsi/be2iscsi/be_cmds.c
+++ b/drivers/scsi/be2iscsi/be_cmds.c
@@ -104,13 +104,16 @@  int be_chk_reset_complete(struct beiscsi_hba *phba)
 	return 0;
 }
 
-void be_mcc_notify(struct beiscsi_hba *phba)
+void be_mcc_notify(struct beiscsi_hba *phba, unsigned int tag)
 {
 	struct be_queue_info *mccq = &phba->ctrl.mcc_obj.q;
 	u32 val = 0;
 
+	set_bit(MCC_TAG_STATE_RUNNING, &phba->ctrl.ptag_state[tag].tag_state);
 	val |= mccq->id & DB_MCCQ_RING_ID_MASK;
 	val |= 1 << DB_MCCQ_NUM_POSTED_SHIFT;
+	/* ring doorbell after all of request and state is written */
+	wmb();
 	iowrite32(val, phba->db_va + DB_MCCQ_OFFSET);
 }
 
@@ -122,6 +125,7 @@  unsigned int alloc_mcc_tag(struct beiscsi_hba *phba)
 		tag = phba->ctrl.mcc_tag[phba->ctrl.mcc_alloc_index];
 		phba->ctrl.mcc_tag[phba->ctrl.mcc_alloc_index] = 0;
 		phba->ctrl.mcc_numtag[tag] = 0;
+		phba->ctrl.ptag_state[tag].tag_state = 0;
 	}
 	if (tag) {
 		phba->ctrl.mcc_tag_available--;
@@ -163,26 +167,25 @@  int beiscsi_mccq_compl(struct beiscsi_hba *phba,
 		return -EPERM;
 	}
 
-	/* Set MBX Tag state to Active */
-	mutex_lock(&phba->ctrl.mbox_lock);
-	phba->ctrl.ptag_state[tag].tag_state = MCC_TAG_STATE_RUNNING;
-	mutex_unlock(&phba->ctrl.mbox_lock);
-
 	/* wait for the mccq completion */
 	rc = wait_event_interruptible_timeout(
 				phba->ctrl.mcc_wait[tag],
 				phba->ctrl.mcc_numtag[tag],
 				msecs_to_jiffies(
 				BEISCSI_HOST_MBX_TIMEOUT));
-
+	/**
+	 * If MBOX cmd timeout expired, tag and resource allocated
+	 * for cmd is not freed until FW returns completion.
+	 */
 	if (rc <= 0) {
 		struct be_dma_mem *tag_mem;
-		/* Set MBX Tag state to timeout */
-		mutex_lock(&phba->ctrl.mbox_lock);
-		phba->ctrl.ptag_state[tag].tag_state = MCC_TAG_STATE_TIMEOUT;
-		mutex_unlock(&phba->ctrl.mbox_lock);
 
-		/* Store resource addr to be freed later */
+		/**
+		 * PCI/DMA memory allocated and posted in non-embedded mode
+		 * will have mbx_cmd_mem != NULL.
+		 * Save virtual and bus addresses for the command so that it
+		 * can be freed later.
+		 **/
 		tag_mem = &phba->ctrl.ptag_state[tag].tag_mem_state;
 		if (mbx_cmd_mem) {
 			tag_mem->size = mbx_cmd_mem->size;
@@ -191,19 +194,19 @@  int beiscsi_mccq_compl(struct beiscsi_hba *phba,
 		} else
 			tag_mem->size = 0;
 
+		/* first make tag_mem_state visible to all */
+		wmb();
+		set_bit(MCC_TAG_STATE_TIMEOUT,
+				&phba->ctrl.ptag_state[tag].tag_state);
+
 		beiscsi_log(phba, KERN_ERR,
 			    BEISCSI_LOG_INIT | BEISCSI_LOG_EH |
 			    BEISCSI_LOG_CONFIG,
 			    "BC_%d : MBX Cmd Completion timed out\n");
 		return -EBUSY;
-	} else {
-		rc = 0;
-		/* Set MBX Tag state to completed */
-		mutex_lock(&phba->ctrl.mbox_lock);
-		phba->ctrl.ptag_state[tag].tag_state = MCC_TAG_STATE_COMPLETED;
-		mutex_unlock(&phba->ctrl.mbox_lock);
 	}
 
+	rc = 0;
 	mcc_tag_response = phba->ctrl.mcc_numtag[tag];
 	status = (mcc_tag_response & CQE_STATUS_MASK);
 	addl_status = ((mcc_tag_response & CQE_STATUS_ADDL_MASK) >>
@@ -231,7 +234,7 @@  int beiscsi_mccq_compl(struct beiscsi_hba *phba,
 			    mbx_hdr->subsystem,
 			    mbx_hdr->opcode,
 			    status, addl_status);
-
+		rc = -EIO;
 		if (status == MCC_STATUS_INSUFFICIENT_BUFFER) {
 			mbx_resp_hdr = (struct be_cmd_resp_hdr *) mbx_hdr;
 			beiscsi_log(phba, KERN_WARNING,
@@ -241,17 +244,11 @@  int beiscsi_mccq_compl(struct beiscsi_hba *phba,
 				    "Resp_Len : %d Actual_Resp_Len : %d\n",
 				    mbx_resp_hdr->response_length,
 				    mbx_resp_hdr->actual_resp_len);
-
 			rc = -EAGAIN;
-			goto release_mcc_tag;
 		}
-		rc = -EIO;
 	}
 
-release_mcc_tag:
-	/* Release the MCC entry */
 	free_mcc_tag(&phba->ctrl, tag);
-
 	return rc;
 }
 
@@ -354,9 +351,37 @@  int be_mcc_compl_process_isr(struct be_ctrl_info *ctrl,
 {
 	struct beiscsi_hba *phba = pci_get_drvdata(ctrl->pdev);
 	u16 compl_status, extd_status;
+	struct be_dma_mem *tag_mem;
 	unsigned short tag;
 
 	be_dws_le_to_cpu(compl, 4);
+	tag = (compl->tag0 & 0x000000FF);
+
+	if (!test_bit(MCC_TAG_STATE_RUNNING,
+		      &ctrl->ptag_state[tag].tag_state)) {
+		beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_MBOX |
+			    BEISCSI_LOG_INIT | BEISCSI_LOG_CONFIG,
+			    "BC_%d : MBX cmd completed but not posted\n");
+		return 0;
+	}
+
+	if (test_bit(MCC_TAG_STATE_TIMEOUT,
+		     &ctrl->ptag_state[tag].tag_state)) {
+		beiscsi_log(phba, KERN_WARNING,
+			    BEISCSI_LOG_MBOX | BEISCSI_LOG_INIT |
+			    BEISCSI_LOG_CONFIG,
+			    "BC_%d : MBX Completion for timeout Command from FW\n");
+		/**
+		 * Check for the size before freeing resource.
+		 * Only for non-embedded cmd, PCI resource is allocated.
+		 **/
+		tag_mem = &ctrl->ptag_state[tag].tag_mem_state;
+		if (tag_mem->size)
+			pci_free_consistent(ctrl->pdev, tag_mem->size,
+					tag_mem->va, tag_mem->dma);
+		free_mcc_tag(ctrl, tag);
+		return 0;
+	}
 
 	compl_status = (compl->status >> CQE_STATUS_COMPL_SHIFT) &
 					CQE_STATUS_COMPL_MASK;
@@ -364,40 +389,16 @@  int be_mcc_compl_process_isr(struct be_ctrl_info *ctrl,
 	 * [31] = valid, [30:24] = Rsvd, [23:16] = wrb, [15:8] = extd_status,
 	 * [7:0] = compl_status
 	 */
-	tag = (compl->tag0 & 0x000000FF);
 	extd_status = (compl->status >> CQE_STATUS_EXTD_SHIFT) &
 					CQE_STATUS_EXTD_MASK;
-
 	ctrl->mcc_numtag[tag]  = 0x80000000;
 	ctrl->mcc_numtag[tag] |= (compl->tag0 & 0x00FF0000);
 	ctrl->mcc_numtag[tag] |= (extd_status & 0x000000FF) << 8;
 	ctrl->mcc_numtag[tag] |= (compl_status & 0x000000FF);
 
-	if (ctrl->ptag_state[tag].tag_state == MCC_TAG_STATE_RUNNING) {
-		wake_up_interruptible(&ctrl->mcc_wait[tag]);
-	} else if (ctrl->ptag_state[tag].tag_state == MCC_TAG_STATE_TIMEOUT) {
-		struct be_dma_mem *tag_mem;
-		tag_mem = &ctrl->ptag_state[tag].tag_mem_state;
-
-		beiscsi_log(phba, KERN_WARNING,
-			    BEISCSI_LOG_MBOX | BEISCSI_LOG_INIT |
-			    BEISCSI_LOG_CONFIG,
-			    "BC_%d : MBX Completion for timeout Command "
-			    "from FW\n");
-		/* Check if memory needs to be freed */
-		if (tag_mem->size)
-			pci_free_consistent(ctrl->pdev, tag_mem->size,
-					    tag_mem->va, tag_mem->dma);
-
-		/* Change tag state */
-		mutex_lock(&phba->ctrl.mbox_lock);
-		ctrl->ptag_state[tag].tag_state = MCC_TAG_STATE_COMPLETED;
-		mutex_unlock(&phba->ctrl.mbox_lock);
-
-		/* Free MCC Tag */
-		free_mcc_tag(ctrl, tag);
-	}
-
+	/* write ordering implied in wake_up_interruptible */
+	clear_bit(MCC_TAG_STATE_RUNNING, &ctrl->ptag_state[tag].tag_state);
+	wake_up_interruptible(&ctrl->mcc_wait[tag]);
 	return 0;
 }
 
@@ -568,9 +569,9 @@  static int be_mcc_wait_compl(struct beiscsi_hba *phba)
  * Success: 0
  * Failure: Non-Zero
  **/
-int be_mcc_notify_wait(struct beiscsi_hba *phba)
+int be_mcc_notify_wait(struct beiscsi_hba *phba, unsigned int tag)
 {
-	be_mcc_notify(phba);
+	be_mcc_notify(phba, tag);
 	return be_mcc_wait_compl(phba);
 }
 
@@ -1439,7 +1440,7 @@  int be_cmd_set_vlan(struct beiscsi_hba *phba,
 	req->interface_hndl = phba->interface_handle;
 	req->vlan_priority = vlan_tag;
 
-	be_mcc_notify(phba);
+	be_mcc_notify(phba, tag);
 	mutex_unlock(&ctrl->mbox_lock);
 
 	return tag;
diff --git a/drivers/scsi/be2iscsi/be_cmds.h b/drivers/scsi/be2iscsi/be_cmds.h
index 4bfca35..1883d32 100644
--- a/drivers/scsi/be2iscsi/be_cmds.h
+++ b/drivers/scsi/be2iscsi/be_cmds.h
@@ -745,8 +745,8 @@  int be_cmd_fw_uninit(struct be_ctrl_info *ctrl);
 
 struct be_mcc_wrb *wrb_from_mbox(struct be_dma_mem *mbox_mem);
 struct be_mcc_wrb *wrb_from_mccq(struct beiscsi_hba *phba);
-int be_mcc_notify_wait(struct beiscsi_hba *phba);
-void be_mcc_notify(struct beiscsi_hba *phba);
+int be_mcc_notify_wait(struct beiscsi_hba *phba, unsigned int tag);
+void be_mcc_notify(struct beiscsi_hba *phba, unsigned int tag);
 unsigned int alloc_mcc_tag(struct beiscsi_hba *phba);
 void beiscsi_async_link_state_process(struct beiscsi_hba *phba,
 		struct be_async_event_link_state *evt);
diff --git a/drivers/scsi/be2iscsi/be_mgmt.c b/drivers/scsi/be2iscsi/be_mgmt.c
index a41013e..7f3f8268 100644
--- a/drivers/scsi/be2iscsi/be_mgmt.c
+++ b/drivers/scsi/be2iscsi/be_mgmt.c
@@ -187,7 +187,7 @@  int be_cmd_modify_eq_delay(struct beiscsi_hba *phba,
 				cpu_to_le32(set_eqd[i].delay_multiplier);
 	}
 
-	be_mcc_notify(phba);
+	be_mcc_notify(phba, tag);
 	mutex_unlock(&ctrl->mbox_lock);
 	return tag;
 }
@@ -234,7 +234,7 @@  unsigned int mgmt_reopen_session(struct beiscsi_hba *phba,
 	req->reopen_type = reopen_type;
 	req->session_handle = sess_handle;
 
-	be_mcc_notify(phba);
+	be_mcc_notify(phba, tag);
 	mutex_unlock(&ctrl->mbox_lock);
 	return tag;
 }
@@ -265,7 +265,7 @@  unsigned int mgmt_get_boot_target(struct beiscsi_hba *phba)
 			   OPCODE_ISCSI_INI_BOOT_GET_BOOT_TARGET,
 			   sizeof(struct be_cmd_get_boot_target_resp));
 
-	be_mcc_notify(phba);
+	be_mcc_notify(phba, tag);
 	mutex_unlock(&ctrl->mbox_lock);
 	return tag;
 }
@@ -310,7 +310,7 @@  unsigned int mgmt_get_session_info(struct beiscsi_hba *phba,
 	sge->pa_lo = cpu_to_le32(nonemb_cmd->dma & 0xFFFFFFFF);
 	sge->len = cpu_to_le32(nonemb_cmd->size);
 
-	be_mcc_notify(phba);
+	be_mcc_notify(phba, tag);
 	mutex_unlock(&ctrl->mbox_lock);
 	return tag;
 }
@@ -541,7 +541,7 @@  unsigned int mgmt_vendor_specific_fw_cmd(struct be_ctrl_info *ctrl,
 	mcc_sge->len = cpu_to_le32(nonemb_cmd->size);
 	wrb->tag0 |= tag;
 
-	be_mcc_notify(phba);
+	be_mcc_notify(phba, tag);
 
 	mutex_unlock(&ctrl->mbox_lock);
 	return tag;
@@ -561,19 +561,26 @@  int mgmt_epfw_cleanup(struct beiscsi_hba *phba, unsigned short ulp_num)
 	struct be_ctrl_info *ctrl = &phba->ctrl;
 	struct be_mcc_wrb *wrb = wrb_from_mccq(phba);
 	struct iscsi_cleanup_req *req = embedded_payload(wrb);
-	int status = 0;
+	unsigned int tag;
+	int status;
 
 	mutex_lock(&ctrl->mbox_lock);
+	tag = alloc_mcc_tag(phba);
+	if (!tag) {
+		mutex_unlock(&ctrl->mbox_lock);
+		return -EBUSY;
+	}
 
 	be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0);
 	be_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_ISCSI,
 			   OPCODE_COMMON_ISCSI_CLEANUP, sizeof(*req));
+	wrb->tag0 |= tag;
 
 	req->chute = (1 << ulp_num);
 	req->hdr_ring_id = cpu_to_le16(HWI_GET_DEF_HDRQ_ID(phba, ulp_num));
 	req->data_ring_id = cpu_to_le16(HWI_GET_DEF_BUFQ_ID(phba, ulp_num));
 
-	status =  be_mcc_notify_wait(phba);
+	status = be_mcc_notify_wait(phba, tag);
 	if (status)
 		beiscsi_log(phba, KERN_WARNING, BEISCSI_LOG_INIT,
 			    "BG_%d : mgmt_epfw_cleanup , FAILED\n");
@@ -622,7 +629,7 @@  unsigned int  mgmt_invalidate_icds(struct beiscsi_hba *phba,
 	sge->pa_lo = cpu_to_le32(nonemb_cmd->dma & 0xFFFFFFFF);
 	sge->len = cpu_to_le32(nonemb_cmd->size);
 
-	be_mcc_notify(phba);
+	be_mcc_notify(phba, tag);
 	mutex_unlock(&ctrl->mbox_lock);
 	return tag;
 }
@@ -659,7 +666,7 @@  unsigned int mgmt_invalidate_connection(struct beiscsi_hba *phba,
 	else
 		req->cleanup_type = CMD_ISCSI_CONNECTION_INVALIDATE;
 	req->save_cfg = savecfg_flag;
-	be_mcc_notify(phba);
+	be_mcc_notify(phba, tag);
 	mutex_unlock(&ctrl->mbox_lock);
 	return tag;
 }
@@ -687,7 +694,7 @@  unsigned int mgmt_upload_connection(struct beiscsi_hba *phba,
 			   OPCODE_COMMON_TCP_UPLOAD, sizeof(*req));
 	req->id = (unsigned short)cid;
 	req->upload_type = (unsigned char)upload_flag;
-	be_mcc_notify(phba);
+	be_mcc_notify(phba, tag);
 	mutex_unlock(&ctrl->mbox_lock);
 	return tag;
 }
@@ -803,7 +810,7 @@  int mgmt_open_connection(struct beiscsi_hba *phba,
 		req->tcp_window_scale_count = 2;
 	}
 
-	be_mcc_notify(phba);
+	be_mcc_notify(phba, tag);
 	mutex_unlock(&ctrl->mbox_lock);
 	return tag;
 }
@@ -833,7 +840,7 @@  unsigned int mgmt_get_all_if_id(struct beiscsi_hba *phba)
 	be_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_ISCSI,
 			   OPCODE_COMMON_ISCSI_NTWK_GET_ALL_IF_ID,
 			   sizeof(*req));
-	be_mcc_notify(phba);
+	be_mcc_notify(phba, tag);
 	mutex_unlock(&ctrl->mbox_lock);
 
 	status = beiscsi_mccq_compl(phba, tag, &wrb, NULL);
@@ -884,7 +891,7 @@  static int mgmt_exec_nonemb_cmd(struct beiscsi_hba *phba,
 	sge->pa_lo = cpu_to_le32(lower_32_bits(nonemb_cmd->dma));
 	sge->len = cpu_to_le32(nonemb_cmd->size);
 
-	be_mcc_notify(phba);
+	be_mcc_notify(phba, tag);
 	mutex_unlock(&ctrl->mbox_lock);
 
 	rc = beiscsi_mccq_compl(phba, tag, NULL, nonemb_cmd);
@@ -1281,7 +1288,7 @@  unsigned int be_cmd_get_initname(struct beiscsi_hba *phba)
 			OPCODE_ISCSI_INI_CFG_GET_HBA_NAME,
 			sizeof(*req));
 
-	be_mcc_notify(phba);
+	be_mcc_notify(phba, tag);
 	mutex_unlock(&ctrl->mbox_lock);
 	return tag;
 }
@@ -1309,7 +1316,7 @@  unsigned int be_cmd_get_port_speed(struct beiscsi_hba *phba)
 			OPCODE_COMMON_NTWK_LINK_STATUS_QUERY,
 			sizeof(*req));
 
-	be_mcc_notify(phba);
+	be_mcc_notify(phba, tag);
 	mutex_unlock(&ctrl->mbox_lock);
 	return tag;
 }
@@ -1786,7 +1793,7 @@  int beiscsi_logout_fw_sess(struct beiscsi_hba *phba,
 
 	/* Set the session handle */
 	req->session_handle = fw_sess_handle;
-	be_mcc_notify(phba);
+	be_mcc_notify(phba, tag);
 	mutex_unlock(&ctrl->mbox_lock);
 
 	rc = beiscsi_mccq_compl(phba, tag, &wrb, NULL);