| Message ID | 20210410184016.21603-14-michael.christie@oracle.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | : qedi tmf fixes | expand |
> -----Original Message----- > From: Mike Christie <michael.christie@oracle.com> > Sent: Sunday, April 11, 2021 12:10 AM > To: lduncan@suse.com; martin.petersen@oracle.com; Manish Rangankar > <mrangankar@marvell.com>; Santosh Vernekar <svernekar@marvell.com>; > linux-scsi@vger.kernel.org; jejb@linux.ibm.com > Cc: Mike Christie <michael.christie@oracle.com> > Subject: [EXT] [PATCH 13/13] scsi: qedi: always wake up if cmd_cleanup_req is > set > > External Email > > ---------------------------------------------------------------------- > If we got a response then we should always wake up the conn. For both the > cmd_cleanup_req == 0 or cmd_cleanup_req > 0, we shouldn't dig into > iscsi_itt_to_task because we don't know what the upper layers are doing. > > We can also remove the qedi_clear_task_idx call here because once we signal > success libiscsi will loop over the affected commands and end up calling the > cleanup_task callout which will release it. > > Signed-off-by: Mike Christie <michael.christie@oracle.com> > --- > drivers/scsi/qedi/qedi_fw.c | 31 ++++--------------------------- > 1 file changed, 4 insertions(+), 27 deletions(-) > > diff --git a/drivers/scsi/qedi/qedi_fw.c b/drivers/scsi/qedi/qedi_fw.c index > 13dd06915d74..13d1250951a6 100644 > --- a/drivers/scsi/qedi/qedi_fw.c > +++ b/drivers/scsi/qedi/qedi_fw.c > @@ -739,7 +739,6 @@ static void qedi_process_cmd_cleanup_resp(struct > qedi_ctx *qedi, { > struct qedi_work_map *work, *work_tmp; > u32 proto_itt = cqe->itid; > - u32 ptmp_itt = 0; > itt_t protoitt = 0; > int found = 0; > struct qedi_cmd *qedi_cmd = NULL; > @@ -823,37 +822,15 @@ static void qedi_process_cmd_cleanup_resp(struct > qedi_ctx *qedi, > > check_cleanup_reqs: > if (qedi_conn->cmd_cleanup_req > 0) { > - spin_lock_bh(&conn->session->back_lock); > - qedi_get_proto_itt(qedi, cqe->itid, &ptmp_itt); > - protoitt = build_itt(ptmp_itt, conn->session->age); > - task = iscsi_itt_to_task(conn, protoitt); > - QEDI_INFO(&qedi->dbg_ctx, QEDI_LOG_SCSI_TM, > - "cleanup io itid=0x%x, protoitt=0x%x, > cmd_cleanup_cmpl=%d, cid=0x%x\n", > - cqe->itid, protoitt, qedi_conn->cmd_cleanup_cmpl, > - qedi_conn->iscsi_conn_id); > - > - spin_unlock_bh(&conn->session->back_lock); > - if (!task) { > - QEDI_NOTICE(&qedi->dbg_ctx, > - "task is null, itid=0x%x, cid=0x%x\n", > - cqe->itid, qedi_conn->iscsi_conn_id); > - return; > - } > - qedi_conn->cmd_cleanup_cmpl++; > - wake_up(&qedi_conn->wait_queue); > - > QEDI_INFO(&qedi->dbg_ctx, QEDI_LOG_TID, > "Freeing tid=0x%x for cid=0x%x\n", > cqe->itid, qedi_conn->iscsi_conn_id); > - qedi_clear_task_idx(qedi_conn->qedi, cqe->itid); > - > + qedi_conn->cmd_cleanup_cmpl++; > + wake_up(&qedi_conn->wait_queue); > } else { > - qedi_get_proto_itt(qedi, cqe->itid, &ptmp_itt); > - protoitt = build_itt(ptmp_itt, conn->session->age); > - task = iscsi_itt_to_task(conn, protoitt); > QEDI_ERR(&qedi->dbg_ctx, > - "Delayed or untracked cleanup response, itt=0x%x, > tid=0x%x, cid=0x%x, task=%p\n", > - protoitt, cqe->itid, qedi_conn->iscsi_conn_id, task); > + "Delayed or untracked cleanup response, itt=0x%x, > tid=0x%x, cid=0x%x\n", > + protoitt, cqe->itid, qedi_conn->iscsi_conn_id); > } > } > > -- > 2.25.1 Thanks, Reviewed-by: Manish Rangankar <mrangankar@marvell.com>
diff --git a/drivers/scsi/qedi/qedi_fw.c b/drivers/scsi/qedi/qedi_fw.c index 13dd06915d74..13d1250951a6 100644 --- a/drivers/scsi/qedi/qedi_fw.c +++ b/drivers/scsi/qedi/qedi_fw.c @@ -739,7 +739,6 @@ static void qedi_process_cmd_cleanup_resp(struct qedi_ctx *qedi, { struct qedi_work_map *work, *work_tmp; u32 proto_itt = cqe->itid; - u32 ptmp_itt = 0; itt_t protoitt = 0; int found = 0; struct qedi_cmd *qedi_cmd = NULL; @@ -823,37 +822,15 @@ static void qedi_process_cmd_cleanup_resp(struct qedi_ctx *qedi, check_cleanup_reqs: if (qedi_conn->cmd_cleanup_req > 0) { - spin_lock_bh(&conn->session->back_lock); - qedi_get_proto_itt(qedi, cqe->itid, &ptmp_itt); - protoitt = build_itt(ptmp_itt, conn->session->age); - task = iscsi_itt_to_task(conn, protoitt); - QEDI_INFO(&qedi->dbg_ctx, QEDI_LOG_SCSI_TM, - "cleanup io itid=0x%x, protoitt=0x%x, cmd_cleanup_cmpl=%d, cid=0x%x\n", - cqe->itid, protoitt, qedi_conn->cmd_cleanup_cmpl, - qedi_conn->iscsi_conn_id); - - spin_unlock_bh(&conn->session->back_lock); - if (!task) { - QEDI_NOTICE(&qedi->dbg_ctx, - "task is null, itid=0x%x, cid=0x%x\n", - cqe->itid, qedi_conn->iscsi_conn_id); - return; - } - qedi_conn->cmd_cleanup_cmpl++; - wake_up(&qedi_conn->wait_queue); - QEDI_INFO(&qedi->dbg_ctx, QEDI_LOG_TID, "Freeing tid=0x%x for cid=0x%x\n", cqe->itid, qedi_conn->iscsi_conn_id); - qedi_clear_task_idx(qedi_conn->qedi, cqe->itid); - + qedi_conn->cmd_cleanup_cmpl++; + wake_up(&qedi_conn->wait_queue); } else { - qedi_get_proto_itt(qedi, cqe->itid, &ptmp_itt); - protoitt = build_itt(ptmp_itt, conn->session->age); - task = iscsi_itt_to_task(conn, protoitt); QEDI_ERR(&qedi->dbg_ctx, - "Delayed or untracked cleanup response, itt=0x%x, tid=0x%x, cid=0x%x, task=%p\n", - protoitt, cqe->itid, qedi_conn->iscsi_conn_id, task); + "Delayed or untracked cleanup response, itt=0x%x, tid=0x%x, cid=0x%x\n", + protoitt, cqe->itid, qedi_conn->iscsi_conn_id); } }
If we got a response then we should always wake up the conn. For both the cmd_cleanup_req == 0 or cmd_cleanup_req > 0, we shouldn't dig into iscsi_itt_to_task because we don't know what the upper layers are doing. We can also remove the qedi_clear_task_idx call here because once we signal success libiscsi will loop over the affected commands and end up calling the cleanup_task callout which will release it. Signed-off-by: Mike Christie <michael.christie@oracle.com> --- drivers/scsi/qedi/qedi_fw.c | 31 ++++--------------------------- 1 file changed, 4 insertions(+), 27 deletions(-)