Commit b40f3894 authored by Mike Christie's avatar Mike Christie Committed by Martin K. Petersen

scsi: qedi: Complete TMF works before disconnect

We need to make sure that abort and reset completion work has completed
before ep_disconnect returns. After ep_disconnect we can't manipulate
cmds because libiscsi will call conn_stop and take onwership.

We are trying to make sure abort work and reset completion work has
completed before we do the cmd clean up in ep_disconnect. The problem is
that:

 1. the work function sets the QEDI_CONN_FW_CLEANUP bit, so if the work was
    still pending we would not see the bit set. We need to do this before
    the work is queued.

 2. If we had multiple works queued then we could break from the loop in
    qedi_ep_disconnect early because when abort work 1 completes it could
    clear QEDI_CONN_FW_CLEANUP. qedi_ep_disconnect could then see that
    before work 2 has run.

 3. A TMF reset completion work could run after ep_disconnect starts
    cleaning up cmds via qedi_clearsq. ep_disconnect's call to qedi_clearsq
    -> qedi_cleanup_all_io would might think it's done cleaning up cmds,
    but the reset completion work could still be running. We then return
    from ep_disconnect while still doing cleanup.

This replaces the bit with a counter to track the number of queued TMF
works, and adds a bool to prevent new works from starting from the
completion path once a ep_disconnect starts.

Link: https://lore.kernel.org/r/20210525181821.7617-28-michael.christie@oracle.comReviewed-by: default avatarManish Rangankar <mrangankar@marvell.com>
Signed-off-by: default avatarMike Christie <michael.christie@oracle.com>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
parent 60a0d379
...@@ -156,7 +156,6 @@ static void qedi_tmf_resp_work(struct work_struct *work) ...@@ -156,7 +156,6 @@ static void qedi_tmf_resp_work(struct work_struct *work)
struct iscsi_tm_rsp *resp_hdr_ptr; struct iscsi_tm_rsp *resp_hdr_ptr;
int rval = 0; int rval = 0;
set_bit(QEDI_CONN_FW_CLEANUP, &qedi_conn->flags);
resp_hdr_ptr = (struct iscsi_tm_rsp *)qedi_cmd->tmf_resp_buf; resp_hdr_ptr = (struct iscsi_tm_rsp *)qedi_cmd->tmf_resp_buf;
rval = qedi_cleanup_all_io(qedi, qedi_conn, qedi_cmd->task, true); rval = qedi_cleanup_all_io(qedi, qedi_conn, qedi_cmd->task, true);
...@@ -169,7 +168,10 @@ static void qedi_tmf_resp_work(struct work_struct *work) ...@@ -169,7 +168,10 @@ static void qedi_tmf_resp_work(struct work_struct *work)
exit_tmf_resp: exit_tmf_resp:
kfree(resp_hdr_ptr); kfree(resp_hdr_ptr);
clear_bit(QEDI_CONN_FW_CLEANUP, &qedi_conn->flags);
spin_lock(&qedi_conn->tmf_work_lock);
qedi_conn->fw_cleanup_works--;
spin_unlock(&qedi_conn->tmf_work_lock);
} }
static void qedi_process_tmf_resp(struct qedi_ctx *qedi, static void qedi_process_tmf_resp(struct qedi_ctx *qedi,
...@@ -225,16 +227,25 @@ static void qedi_process_tmf_resp(struct qedi_ctx *qedi, ...@@ -225,16 +227,25 @@ static void qedi_process_tmf_resp(struct qedi_ctx *qedi,
} }
spin_unlock(&qedi_conn->list_lock); spin_unlock(&qedi_conn->list_lock);
if (((tmf_hdr->flags & ISCSI_FLAG_TM_FUNC_MASK) == spin_lock(&qedi_conn->tmf_work_lock);
ISCSI_TM_FUNC_LOGICAL_UNIT_RESET) || switch (tmf_hdr->flags & ISCSI_FLAG_TM_FUNC_MASK) {
((tmf_hdr->flags & ISCSI_FLAG_TM_FUNC_MASK) == case ISCSI_TM_FUNC_LOGICAL_UNIT_RESET:
ISCSI_TM_FUNC_TARGET_WARM_RESET) || case ISCSI_TM_FUNC_TARGET_WARM_RESET:
((tmf_hdr->flags & ISCSI_FLAG_TM_FUNC_MASK) == case ISCSI_TM_FUNC_TARGET_COLD_RESET:
ISCSI_TM_FUNC_TARGET_COLD_RESET)) { if (qedi_conn->ep_disconnect_starting) {
/* Session is down so ep_disconnect will clean up */
spin_unlock(&qedi_conn->tmf_work_lock);
goto unblock_sess;
}
qedi_conn->fw_cleanup_works++;
spin_unlock(&qedi_conn->tmf_work_lock);
INIT_WORK(&qedi_cmd->tmf_work, qedi_tmf_resp_work); INIT_WORK(&qedi_cmd->tmf_work, qedi_tmf_resp_work);
queue_work(qedi->tmf_thread, &qedi_cmd->tmf_work); queue_work(qedi->tmf_thread, &qedi_cmd->tmf_work);
goto unblock_sess; goto unblock_sess;
} }
spin_unlock(&qedi_conn->tmf_work_lock);
__iscsi_complete_pdu(conn, (struct iscsi_hdr *)resp_hdr_ptr, NULL, 0); __iscsi_complete_pdu(conn, (struct iscsi_hdr *)resp_hdr_ptr, NULL, 0);
kfree(resp_hdr_ptr); kfree(resp_hdr_ptr);
...@@ -1359,7 +1370,6 @@ static void qedi_abort_work(struct work_struct *work) ...@@ -1359,7 +1370,6 @@ static void qedi_abort_work(struct work_struct *work)
mtask = qedi_cmd->task; mtask = qedi_cmd->task;
tmf_hdr = (struct iscsi_tm *)mtask->hdr; tmf_hdr = (struct iscsi_tm *)mtask->hdr;
set_bit(QEDI_CONN_FW_CLEANUP, &qedi_conn->flags);
spin_lock_bh(&conn->session->back_lock); spin_lock_bh(&conn->session->back_lock);
ctask = iscsi_itt_to_ctask(conn, tmf_hdr->rtt); ctask = iscsi_itt_to_ctask(conn, tmf_hdr->rtt);
...@@ -1429,10 +1439,7 @@ static void qedi_abort_work(struct work_struct *work) ...@@ -1429,10 +1439,7 @@ static void qedi_abort_work(struct work_struct *work)
send_tmf: send_tmf:
send_iscsi_tmf(qedi_conn, qedi_cmd->task, ctask); send_iscsi_tmf(qedi_conn, qedi_cmd->task, ctask);
goto clear_cleanup;
clear_cleanup:
clear_bit(QEDI_CONN_FW_CLEANUP, &qedi_conn->flags);
return;
ldel_exit: ldel_exit:
spin_lock_bh(&qedi_conn->tmf_work_lock); spin_lock_bh(&qedi_conn->tmf_work_lock);
...@@ -1451,7 +1458,10 @@ static void qedi_abort_work(struct work_struct *work) ...@@ -1451,7 +1458,10 @@ static void qedi_abort_work(struct work_struct *work)
} }
spin_unlock(&qedi_conn->list_lock); spin_unlock(&qedi_conn->list_lock);
clear_bit(QEDI_CONN_FW_CLEANUP, &qedi_conn->flags); clear_cleanup:
spin_lock(&qedi_conn->tmf_work_lock);
qedi_conn->fw_cleanup_works--;
spin_unlock(&qedi_conn->tmf_work_lock);
} }
static int send_iscsi_tmf(struct qedi_conn *qedi_conn, struct iscsi_task *mtask, static int send_iscsi_tmf(struct qedi_conn *qedi_conn, struct iscsi_task *mtask,
...@@ -1546,6 +1556,10 @@ int qedi_send_iscsi_tmf(struct qedi_conn *qedi_conn, struct iscsi_task *mtask) ...@@ -1546,6 +1556,10 @@ int qedi_send_iscsi_tmf(struct qedi_conn *qedi_conn, struct iscsi_task *mtask)
switch (tmf_hdr->flags & ISCSI_FLAG_TM_FUNC_MASK) { switch (tmf_hdr->flags & ISCSI_FLAG_TM_FUNC_MASK) {
case ISCSI_TM_FUNC_ABORT_TASK: case ISCSI_TM_FUNC_ABORT_TASK:
spin_lock(&qedi_conn->tmf_work_lock);
qedi_conn->fw_cleanup_works++;
spin_unlock(&qedi_conn->tmf_work_lock);
INIT_WORK(&qedi_cmd->tmf_work, qedi_abort_work); INIT_WORK(&qedi_cmd->tmf_work, qedi_abort_work);
queue_work(qedi->tmf_thread, &qedi_cmd->tmf_work); queue_work(qedi->tmf_thread, &qedi_cmd->tmf_work);
break; break;
......
...@@ -603,7 +603,11 @@ static int qedi_conn_start(struct iscsi_cls_conn *cls_conn) ...@@ -603,7 +603,11 @@ static int qedi_conn_start(struct iscsi_cls_conn *cls_conn)
goto start_err; goto start_err;
} }
clear_bit(QEDI_CONN_FW_CLEANUP, &qedi_conn->flags); spin_lock(&qedi_conn->tmf_work_lock);
qedi_conn->fw_cleanup_works = 0;
qedi_conn->ep_disconnect_starting = false;
spin_unlock(&qedi_conn->tmf_work_lock);
qedi_conn->abrt_conn = 0; qedi_conn->abrt_conn = 0;
rval = iscsi_conn_start(cls_conn); rval = iscsi_conn_start(cls_conn);
...@@ -1019,7 +1023,6 @@ static void qedi_ep_disconnect(struct iscsi_endpoint *ep) ...@@ -1019,7 +1023,6 @@ static void qedi_ep_disconnect(struct iscsi_endpoint *ep)
int ret = 0; int ret = 0;
int wait_delay; int wait_delay;
int abrt_conn = 0; int abrt_conn = 0;
int count = 10;
wait_delay = 60 * HZ + DEF_MAX_RT_TIME; wait_delay = 60 * HZ + DEF_MAX_RT_TIME;
qedi_ep = ep->dd_data; qedi_ep = ep->dd_data;
...@@ -1035,13 +1038,19 @@ static void qedi_ep_disconnect(struct iscsi_endpoint *ep) ...@@ -1035,13 +1038,19 @@ static void qedi_ep_disconnect(struct iscsi_endpoint *ep)
qedi_conn = qedi_ep->conn; qedi_conn = qedi_ep->conn;
abrt_conn = qedi_conn->abrt_conn; abrt_conn = qedi_conn->abrt_conn;
while (count--) { QEDI_INFO(&qedi->dbg_ctx, QEDI_LOG_INFO,
if (!test_bit(QEDI_CONN_FW_CLEANUP, "cid=0x%x qedi_ep=%p waiting for %d tmfs\n",
&qedi_conn->flags)) { qedi_ep->iscsi_cid, qedi_ep,
break; qedi_conn->fw_cleanup_works);
}
spin_lock(&qedi_conn->tmf_work_lock);
qedi_conn->ep_disconnect_starting = true;
while (qedi_conn->fw_cleanup_works > 0) {
spin_unlock(&qedi_conn->tmf_work_lock);
msleep(1000); msleep(1000);
spin_lock(&qedi_conn->tmf_work_lock);
} }
spin_unlock(&qedi_conn->tmf_work_lock);
if (test_bit(QEDI_IN_RECOVERY, &qedi->flags)) { if (test_bit(QEDI_IN_RECOVERY, &qedi->flags)) {
if (qedi_do_not_recover) { if (qedi_do_not_recover) {
......
...@@ -169,8 +169,8 @@ struct qedi_conn { ...@@ -169,8 +169,8 @@ struct qedi_conn {
struct list_head tmf_work_list; struct list_head tmf_work_list;
wait_queue_head_t wait_queue; wait_queue_head_t wait_queue;
spinlock_t tmf_work_lock; /* tmf work lock */ spinlock_t tmf_work_lock; /* tmf work lock */
unsigned long flags; bool ep_disconnect_starting;
#define QEDI_CONN_FW_CLEANUP 1 int fw_cleanup_works;
}; };
struct qedi_cmd { struct qedi_cmd {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment