On 12/20/15 3:01 AM, Mike Christie wrote:
On 12/20/2015 01:44 AM, Mike Christie wrote:
diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c
index 6fabded..21c806f 100644
--- a/drivers/scsi/be2iscsi/be_cmds.c
+++ b/drivers/scsi/be2iscsi/be_cmds.c
@@ -164,9 +164,8 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
}
/* 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);
+ atomic_set(&phba->ctrl.ptag_state[tag].tag_state,
+ MCC_TAG_STATE_RUNNING);
Is it possible for be_mcc_compl_process_isr to run before we have set
the state to MCC_TAG_STATE_RUNNING, so the
wait_event_interruptible_timeout call timesout?
/* wait for the mccq completion */
rc = wait_event_interruptible_timeout(
@@ -178,9 +177,8 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
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);
+ atomic_set(&phba->ctrl.ptag_state[tag].tag_state,
+ MCC_TAG_STATE_TIMEOUT);
/* Store resource addr to be freed later */
tag_mem = &phba->ctrl.ptag_state[tag].tag_mem_state;
@@ -199,9 +197,8 @@ int beiscsi_mccq_compl(struct beiscsi_hba *phba,
} 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);
+ atomic_set(&phba->ctrl.ptag_state[tag].tag_state,
+ MCC_TAG_STATE_COMPLETED);
}
mcc_tag_response = phba->ctrl.mcc_numtag[tag];
@@ -373,9 +370,11 @@ int be_mcc_compl_process_isr(struct be_ctrl_info *ctrl,
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) {
+ if (atomic_read(&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) {
+ } else if (atomic_read(&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;
@@ -390,9 +389,8 @@ int be_mcc_compl_process_isr(struct be_ctrl_info *ctrl,
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);
+ atomic_set(&ctrl->ptag_state[tag].tag_state,
+ MCC_TAG_STATE_COMPLETED);
/* Free MCC Tag */
free_mcc_tag(ctrl, tag);
I think if you only need to get and set a u8 then you can just use a u8
since the operation will be atomic. No need for a atomic_t.
I think you can ignore this. I think you would need some barriers in
there too and it might be more complicated for no gain.
Ughhh. Sorry.
The atomic_ops.txt doc says atomic_read/atomic_set use still needs
barriers, so I guess they do not do anything for you and a u8 is ok.
The memory-barrier.txt doc says wake_up/wait calls have barriers if the
wake_up path is hit, so you are ok there.
However, besides the timeout issue in the previous mail, can
beiscsi_mccq_compl set the tag_mem_state to MCC_TAG_STATE_TIMEOUT and
be_mcc_compl_process_isr does not see the tag_mem values updated?
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
