Re: [PATCH 09/10] scsi: qedi: Fix failed disconnect handling.

[Lee Duncan <lduncan@suse.com>]

On 4/7/22 17:13, Mike Christie wrote:
> We set the qedi_ep state to EP_STATE_OFLDCONN_START when the ep is
> created. Then in qedi_set_path we kick off the offload work. If userspace
> times out the connection and calls ep_disconnect, qedi will only flush the
> offload work if the qedi_ep state has transitioned away from
> EP_STATE_OFLDCONN_START. If we can't connect we will not have transitioned
> state and will leave the offload work running, and we will free the
> qedi_ep from under it.
> 
> This patch just has us init the work when we create the ep, then always
> flush it.
> 
> Signed-off-by: Mike Christie <michael.christie@oracle.com>
> ---
>   drivers/scsi/qedi/qedi_iscsi.c | 69 +++++++++++++++++-----------------
>   1 file changed, 34 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/scsi/qedi/qedi_iscsi.c b/drivers/scsi/qedi/qedi_iscsi.c
> index 8196f89f404e..31ec429104e2 100644
> --- a/drivers/scsi/qedi/qedi_iscsi.c
> +++ b/drivers/scsi/qedi/qedi_iscsi.c
> @@ -860,6 +860,37 @@ static int qedi_task_xmit(struct iscsi_task *task)
>   	return qedi_iscsi_send_ioreq(task);
>   }
>   
> +static void qedi_offload_work(struct work_struct *work)
> +{
> +	struct qedi_endpoint *qedi_ep =
> +		container_of(work, struct qedi_endpoint, offload_work);
> +	struct qedi_ctx *qedi;
> +	int wait_delay = 5 * HZ;
> +	int ret;
> +
> +	qedi = qedi_ep->qedi;
> +
> +	ret = qedi_iscsi_offload_conn(qedi_ep);
> +	if (ret) {
> +		QEDI_ERR(&qedi->dbg_ctx,
> +			 "offload error: iscsi_cid=%u, qedi_ep=%p, ret=%d\n",
> +			 qedi_ep->iscsi_cid, qedi_ep, ret);
> +		qedi_ep->state = EP_STATE_OFLDCONN_FAILED;
> +		return;
> +	}
> +
> +	ret = wait_event_interruptible_timeout(qedi_ep->tcp_ofld_wait,
> +					       (qedi_ep->state ==
> +					       EP_STATE_OFLDCONN_COMPL),
> +					       wait_delay);
> +	if (ret <= 0 || qedi_ep->state != EP_STATE_OFLDCONN_COMPL) {
> +		qedi_ep->state = EP_STATE_OFLDCONN_FAILED;
> +		QEDI_ERR(&qedi->dbg_ctx,
> +			 "Offload conn TIMEOUT iscsi_cid=%u, qedi_ep=%p\n",
> +			 qedi_ep->iscsi_cid, qedi_ep);
> +	}
> +}
> +
>   static struct iscsi_endpoint *
>   qedi_ep_connect(struct Scsi_Host *shost, struct sockaddr *dst_addr,
>   		int non_blocking)
> @@ -908,6 +939,7 @@ qedi_ep_connect(struct Scsi_Host *shost, struct sockaddr *dst_addr,
>   	}
>   	qedi_ep = ep->dd_data;
>   	memset(qedi_ep, 0, sizeof(struct qedi_endpoint));
> +	INIT_WORK(&qedi_ep->offload_work, qedi_offload_work);
>   	qedi_ep->state = EP_STATE_IDLE;
>   	qedi_ep->iscsi_cid = (u32)-1;
>   	qedi_ep->qedi = qedi;
> @@ -1056,12 +1088,11 @@ static void qedi_ep_disconnect(struct iscsi_endpoint *ep)
>   	qedi_ep = ep->dd_data;
>   	qedi = qedi_ep->qedi;
>   
> +	flush_work(&qedi_ep->offload_work);
> +
>   	if (qedi_ep->state == EP_STATE_OFLDCONN_START)
>   		goto ep_exit_recover;
>   
> -	if (qedi_ep->state != EP_STATE_OFLDCONN_NONE)
> -		flush_work(&qedi_ep->offload_work);
> -
>   	if (qedi_ep->conn) {
>   		qedi_conn = qedi_ep->conn;
>   		abrt_conn = qedi_conn->abrt_conn;
> @@ -1235,37 +1266,6 @@ static int qedi_data_avail(struct qedi_ctx *qedi, u16 vlanid)
>   	return rc;
>   }
>   
> -static void qedi_offload_work(struct work_struct *work)
> -{
> -	struct qedi_endpoint *qedi_ep =
> -		container_of(work, struct qedi_endpoint, offload_work);
> -	struct qedi_ctx *qedi;
> -	int wait_delay = 5 * HZ;
> -	int ret;
> -
> -	qedi = qedi_ep->qedi;
> -
> -	ret = qedi_iscsi_offload_conn(qedi_ep);
> -	if (ret) {
> -		QEDI_ERR(&qedi->dbg_ctx,
> -			 "offload error: iscsi_cid=%u, qedi_ep=%p, ret=%d\n",
> -			 qedi_ep->iscsi_cid, qedi_ep, ret);
> -		qedi_ep->state = EP_STATE_OFLDCONN_FAILED;
> -		return;
> -	}
> -
> -	ret = wait_event_interruptible_timeout(qedi_ep->tcp_ofld_wait,
> -					       (qedi_ep->state ==
> -					       EP_STATE_OFLDCONN_COMPL),
> -					       wait_delay);
> -	if ((ret <= 0) || (qedi_ep->state != EP_STATE_OFLDCONN_COMPL)) {
> -		qedi_ep->state = EP_STATE_OFLDCONN_FAILED;
> -		QEDI_ERR(&qedi->dbg_ctx,
> -			 "Offload conn TIMEOUT iscsi_cid=%u, qedi_ep=%p\n",
> -			 qedi_ep->iscsi_cid, qedi_ep);
> -	}
> -}
> -
>   static int qedi_set_path(struct Scsi_Host *shost, struct iscsi_path *path_data)
>   {
>   	struct qedi_ctx *qedi;
> @@ -1381,7 +1381,6 @@ static int qedi_set_path(struct Scsi_Host *shost, struct iscsi_path *path_data)
>   			  qedi_ep->dst_addr, qedi_ep->dst_port);
>   	}
>   
> -	INIT_WORK(&qedi_ep->offload_work, qedi_offload_work);
>   	queue_work(qedi->offload_thread, &qedi_ep->offload_work);
>   
>   	ret = 0;

Reviewed-by: Lee Duncan <lduncan@suse.com>