RE: [EXT] [PATCH 13/13] scsi: qedi: always wake up if cmd_cleanup_req is set

[Manish Rangankar <mrangankar@marvell.com>]

> -----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>