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

Manish Rangankar
2022-04-08T16:49:37Z

> -----Original Message-----
> From: Mike Christie <michael.christie@oracle.com>
> Sent: Friday, April 8, 2022 5:43 AM
> To: Saurav Kashyap <skashyap@marvell.com>; lduncan@suse.com;
> cleech@redhat.com; Nilesh Javali <njavali@marvell.com>; Manish Rangankar
> <mrangankar@marvell.com>; GR-QLogic-Storage-Upstream <GR-QLogic-
> Storage-Upstream@marvell.com>; martin.petersen@oracle.com; linux-
> scsi@vger.kernel.org; jejb@linux.ibm.com
> Cc: Mike Christie <michael.christie@oracle.com>
> Subject: [EXT] [PATCH 09/10] scsi: qedi: Fix failed disconnect handling.
>
> External Email
>
> ----------------------------------------------------------------------
> 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;
> --
> 2.25.1
Thanks,

Acked-by: Manish Rangankar <mrangankar@marvell.com>