From d3305f3407fa3e9452079ec6cc8379067456e4aa Mon Sep 17 00:00:00 2001 From: Mike Christie Date: Thu, 20 Aug 2009 15:10:58 -0500 Subject: [SCSI] libiscsi: don't increment cmdsn if cmd is not sent We increment session->cmdsn at the top of iscsi_prep_scsi_cmd_pdu, but if the prep ecb or prep bidi or init_task calls fails then we leave the session->cmdsn incremented. This moves the cmdsn manipulation to the end of the function when we know it has succeeded. It also adds a session->cmdsn--; in queuecommand for if a driver like bnx2i tries to send a a task from that context but it fails. We do not have to do this in the xmit thread context because that code will retry the same task if the initial call fails. Signed-off-by: Mike Christie Signed-off-by: James Bottomley --- drivers/scsi/libiscsi.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/scsi/libiscsi.c') diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c index a751f6230c2..c68cb53a984 100644 --- a/drivers/scsi/libiscsi.c +++ b/drivers/scsi/libiscsi.c @@ -301,8 +301,6 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_task *task) hdr->flags = ISCSI_ATTR_SIMPLE; int_to_scsilun(sc->device->lun, (struct scsi_lun *)hdr->lun); memcpy(task->lun, hdr->lun, sizeof(task->lun)); - hdr->cmdsn = task->cmdsn = cpu_to_be32(session->cmdsn); - session->cmdsn++; hdr->exp_statsn = cpu_to_be32(conn->exp_statsn); cmd_len = sc->cmd_len; if (cmd_len < ISCSI_CDB_SIZE) @@ -388,6 +386,8 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_task *task) return -EIO; task->state = ISCSI_TASK_RUNNING; + hdr->cmdsn = task->cmdsn = cpu_to_be32(session->cmdsn); + session->cmdsn++; conn->scsicmd_pdus_cnt++; ISCSI_DBG_SESSION(session, "iscsi prep [%s cid %d sc %p cdb 0x%x " @@ -1497,6 +1497,7 @@ int iscsi_queuecommand(struct scsi_cmnd *sc, void (*done)(struct scsi_cmnd *)) } } if (session->tt->xmit_task(task)) { + session->cmdsn--; reason = FAILURE_SESSION_NOT_READY; goto prepd_reject; } -- cgit v1.2.3-70-g09d2 From 8afa1439fcff58da8f28c1d083046f229f6ab3de Mon Sep 17 00:00:00 2001 From: Mike Christie Date: Thu, 20 Aug 2009 15:10:59 -0500 Subject: [SCSI] libiscsi: handle immediate command rejections If we sent multiple pdus as immediate the target could be rejecting some and we have just been dropping the rejection notification. This adds code to handle nop-out rejections, so if a nop-out was sent as a ping and rejected we do not mark the connection bad. Instead we just clean up the timers since we have pdu making a rount trip we know the connection is good. Signed-off-by: Mike Christie Signed-off-by: James Bottomley --- drivers/scsi/libiscsi.c | 106 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 87 insertions(+), 19 deletions(-) (limited to 'drivers/scsi/libiscsi.c') diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c index c68cb53a984..1d7a8b7e8a7 100644 --- a/drivers/scsi/libiscsi.c +++ b/drivers/scsi/libiscsi.c @@ -857,27 +857,102 @@ static void iscsi_send_nopout(struct iscsi_conn *conn, struct iscsi_nopin *rhdr) } } +static int iscsi_nop_out_rsp(struct iscsi_task *task, + struct iscsi_nopin *nop, char *data, int datalen) +{ + struct iscsi_conn *conn = task->conn; + int rc = 0; + + if (conn->ping_task != task) { + /* + * If this is not in response to one of our + * nops then it must be from userspace. + */ + if (iscsi_recv_pdu(conn->cls_conn, (struct iscsi_hdr *)nop, + data, datalen)) + rc = ISCSI_ERR_CONN_FAILED; + } else + mod_timer(&conn->transport_timer, jiffies + conn->recv_timeout); + iscsi_complete_task(task, ISCSI_TASK_COMPLETED); + return rc; +} + static int iscsi_handle_reject(struct iscsi_conn *conn, struct iscsi_hdr *hdr, char *data, int datalen) { struct iscsi_reject *reject = (struct iscsi_reject *)hdr; struct iscsi_hdr rejected_pdu; + int opcode, rc = 0; conn->exp_statsn = be32_to_cpu(reject->statsn) + 1; - if (reject->reason == ISCSI_REASON_DATA_DIGEST_ERROR) { - if (ntoh24(reject->dlength) > datalen) - return ISCSI_ERR_PROTO; + if (ntoh24(reject->dlength) > datalen || + ntoh24(reject->dlength) < sizeof(struct iscsi_hdr)) { + iscsi_conn_printk(KERN_ERR, conn, "Cannot handle rejected " + "pdu. Invalid data length (pdu dlength " + "%u, datalen %d\n", ntoh24(reject->dlength), + datalen); + return ISCSI_ERR_PROTO; + } + memcpy(&rejected_pdu, data, sizeof(struct iscsi_hdr)); + opcode = rejected_pdu.opcode & ISCSI_OPCODE_MASK; - if (ntoh24(reject->dlength) >= sizeof(struct iscsi_hdr)) { - memcpy(&rejected_pdu, data, sizeof(struct iscsi_hdr)); - iscsi_conn_printk(KERN_ERR, conn, - "pdu (op 0x%x) rejected " - "due to DataDigest error.\n", - rejected_pdu.opcode); + switch (reject->reason) { + case ISCSI_REASON_DATA_DIGEST_ERROR: + iscsi_conn_printk(KERN_ERR, conn, + "pdu (op 0x%x itt 0x%x) rejected " + "due to DataDigest error.\n", + rejected_pdu.itt, opcode); + break; + case ISCSI_REASON_IMM_CMD_REJECT: + iscsi_conn_printk(KERN_ERR, conn, + "pdu (op 0x%x itt 0x%x) rejected. Too many " + "immediate commands.\n", + rejected_pdu.itt, opcode); + /* + * We only send one TMF at a time so if the target could not + * handle it, then it should get fixed (RFC mandates that + * a target can handle one immediate TMF per conn). + * + * For nops-outs, we could have sent more than one if + * the target is sending us lots of nop-ins + */ + if (opcode != ISCSI_OP_NOOP_OUT) + return 0; + + if (rejected_pdu.itt == cpu_to_be32(ISCSI_RESERVED_TAG)) + /* + * nop-out in response to target's nop-out rejected. + * Just resend. + */ + iscsi_send_nopout(conn, + (struct iscsi_nopin*)&rejected_pdu); + else { + struct iscsi_task *task; + /* + * Our nop as ping got dropped. We know the target + * and transport are ok so just clean up + */ + task = iscsi_itt_to_task(conn, rejected_pdu.itt); + if (!task) { + iscsi_conn_printk(KERN_ERR, conn, + "Invalid pdu reject. Could " + "not lookup rejected task.\n"); + rc = ISCSI_ERR_BAD_ITT; + } else + rc = iscsi_nop_out_rsp(task, + (struct iscsi_nopin*)&rejected_pdu, + NULL, 0); } + break; + default: + iscsi_conn_printk(KERN_ERR, conn, + "pdu (op 0x%x itt 0x%x) rejected. Reason " + "code 0x%x\n", rejected_pdu.itt, + rejected_pdu.opcode, reject->reason); + break; } - return 0; + return rc; } /** @@ -1038,15 +1113,8 @@ int __iscsi_complete_pdu(struct iscsi_conn *conn, struct iscsi_hdr *hdr, } conn->exp_statsn = be32_to_cpu(hdr->statsn) + 1; - if (conn->ping_task != task) - /* - * If this is not in response to one of our - * nops then it must be from userspace. - */ - goto recv_pdu; - - mod_timer(&conn->transport_timer, jiffies + conn->recv_timeout); - iscsi_complete_task(task, ISCSI_TASK_COMPLETED); + rc = iscsi_nop_out_rsp(task, (struct iscsi_nopin*)hdr, + data, datalen); break; default: rc = ISCSI_ERR_BAD_OPCODE; -- cgit v1.2.3-70-g09d2 From 70b31c152dc49ef70bd2b34ad53ccbd9bb4116d4 Mon Sep 17 00:00:00 2001 From: Mike Christie Date: Thu, 20 Aug 2009 15:11:03 -0500 Subject: [SCSI] libiscsi, iscsi_tcp: check suspend bit before each call to xmit_task If we had multiple tasks on the cmd or requeue lists, and iscsi_tcp returns a error, the write_space function can still run and queue iscsi_data_xmit. If it was a legetimate problem and iscsi_conn_failure was run but we raced and iscsi_data_xmit was run first it could miss the suspend bit checks, and start trying to send data again and hit another timeout. A similar problem is present when using cxgb3i. This has libiscsi check the suspend bit before calling the xmit task callout, so we at least do not try sending multiple tasks (one could be sent). Signed-off-by: Mike Christie Signed-off-by: James Bottomley --- drivers/scsi/libiscsi.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) (limited to 'drivers/scsi/libiscsi.c') diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c index 1d7a8b7e8a7..a7ee4bb4070 100644 --- a/drivers/scsi/libiscsi.c +++ b/drivers/scsi/libiscsi.c @@ -1280,6 +1280,9 @@ static int iscsi_xmit_task(struct iscsi_conn *conn) struct iscsi_task *task = conn->task; int rc; + if (test_bit(ISCSI_SUSPEND_BIT, &conn->suspend_tx)) + return -ENODATA; + __iscsi_get_task(task); spin_unlock_bh(&conn->session->lock); rc = conn->session->tt->xmit_task(task); @@ -1329,7 +1332,7 @@ static int iscsi_data_xmit(struct iscsi_conn *conn) int rc = 0; spin_lock_bh(&conn->session->lock); - if (unlikely(conn->suspend_tx)) { + if (test_bit(ISCSI_SUSPEND_BIT, &conn->suspend_tx)) { ISCSI_DBG_SESSION(conn->session, "Tx suspended!\n"); spin_unlock_bh(&conn->session->lock); return -ENODATA; @@ -1338,7 +1341,7 @@ static int iscsi_data_xmit(struct iscsi_conn *conn) if (conn->task) { rc = iscsi_xmit_task(conn); if (rc) - goto again; + goto done; } /* @@ -1358,7 +1361,7 @@ check_mgmt: } rc = iscsi_xmit_task(conn); if (rc) - goto again; + goto done; } /* process pending command queue */ @@ -1379,14 +1382,14 @@ check_mgmt: list_add_tail(&conn->task->running, &conn->cmdqueue); conn->task = NULL; - goto again; + goto done; } else fail_scsi_task(conn->task, DID_ABORT); continue; } rc = iscsi_xmit_task(conn); if (rc) - goto again; + goto done; /* * we could continuously get new task requests so * we need to check the mgmt queue for nops that need to @@ -1412,16 +1415,14 @@ check_mgmt: conn->task->state = ISCSI_TASK_RUNNING; rc = iscsi_xmit_task(conn); if (rc) - goto again; + goto done; if (!list_empty(&conn->mgmtqueue)) goto check_mgmt; } spin_unlock_bh(&conn->session->lock); return -ENODATA; -again: - if (unlikely(conn->suspend_tx)) - rc = -ENODATA; +done: spin_unlock_bh(&conn->session->lock); return rc; } -- cgit v1.2.3-70-g09d2 From 4c0ba5d2593b5156327263f3ef6d7399dc0717b8 Mon Sep 17 00:00:00 2001 From: Mike Christie Date: Sat, 5 Sep 2009 07:34:23 +0530 Subject: [SCSI] libiscsi: add completion function for drivers that do not need pdu processing beiscsi does not need the iscsi scsi cmd processing. It does not even get this info on the completion path. This adds a function to just update the sequencing numbers and complete a task. Signed-off-by: Mike Christie Signed-off-by: Jayamohan Kallickal Signed-off-by: James Bottomley --- drivers/scsi/libiscsi.c | 38 +++++++++++++++++++++++++++++++++----- include/scsi/libiscsi.h | 2 ++ 2 files changed, 35 insertions(+), 5 deletions(-) (limited to 'drivers/scsi/libiscsi.c') diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c index a7ee4bb4070..67b2a2b0028 100644 --- a/drivers/scsi/libiscsi.c +++ b/drivers/scsi/libiscsi.c @@ -109,12 +109,9 @@ inline void iscsi_conn_queue_work(struct iscsi_conn *conn) } EXPORT_SYMBOL_GPL(iscsi_conn_queue_work); -void -iscsi_update_cmdsn(struct iscsi_session *session, struct iscsi_nopin *hdr) +static void __iscsi_update_cmdsn(struct iscsi_session *session, + uint32_t exp_cmdsn, uint32_t max_cmdsn) { - uint32_t max_cmdsn = be32_to_cpu(hdr->max_cmdsn); - uint32_t exp_cmdsn = be32_to_cpu(hdr->exp_cmdsn); - /* * standard specifies this check for when to update expected and * max sequence numbers @@ -138,6 +135,12 @@ iscsi_update_cmdsn(struct iscsi_session *session, struct iscsi_nopin *hdr) iscsi_conn_queue_work(session->leadconn); } } + +void iscsi_update_cmdsn(struct iscsi_session *session, struct iscsi_nopin *hdr) +{ + __iscsi_update_cmdsn(session, be32_to_cpu(hdr->exp_cmdsn), + be32_to_cpu(hdr->max_cmdsn)); +} EXPORT_SYMBOL_GPL(iscsi_update_cmdsn); /** @@ -499,6 +502,31 @@ static void iscsi_complete_task(struct iscsi_task *task, int state) __iscsi_put_task(task); } +/** + * iscsi_complete_scsi_task - finish scsi task normally + * @task: iscsi task for scsi cmd + * @exp_cmdsn: expected cmd sn in cpu format + * @max_cmdsn: max cmd sn in cpu format + * + * This is used when drivers do not need or cannot perform + * lower level pdu processing. + * + * Called with session lock + */ +void iscsi_complete_scsi_task(struct iscsi_task *task, + uint32_t exp_cmdsn, uint32_t max_cmdsn) +{ + struct iscsi_conn *conn = task->conn; + + ISCSI_DBG_SESSION(conn->session, "[itt 0x%x]\n", task->itt); + + conn->last_recv = jiffies; + __iscsi_update_cmdsn(conn->session, exp_cmdsn, max_cmdsn); + iscsi_complete_task(task, ISCSI_TASK_COMPLETED); +} +EXPORT_SYMBOL_GPL(iscsi_complete_scsi_task); + + /* * session lock must be held and if not called for a task that is * still pending or from the xmit thread, then xmit thread must diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h index 61afeb59a83..439c8b75cb6 100644 --- a/include/scsi/libiscsi.h +++ b/include/scsi/libiscsi.h @@ -415,6 +415,8 @@ extern struct iscsi_task *iscsi_itt_to_task(struct iscsi_conn *, itt_t); extern void iscsi_requeue_task(struct iscsi_task *task); extern void iscsi_put_task(struct iscsi_task *task); extern void __iscsi_get_task(struct iscsi_task *task); +extern void iscsi_complete_scsi_task(struct iscsi_task *task, + uint32_t exp_cmdsn, uint32_t max_cmdsn); /* * generic helpers -- cgit v1.2.3-70-g09d2 From 661134ad3765348ecd6150a92e736bf28ba40f80 Mon Sep 17 00:00:00 2001 From: Mike Christie Date: Sat, 5 Sep 2009 07:35:33 +0530 Subject: [SCSI] libiscsi, bnx2i: make bound ep check common bnx2i currently has a check for if a ep is properly bound, so if iscsi_queuecommand/xmit_task is called while there is no ep we will not queue IO. be2iscsi sends IO from queuecommand/xmit_task like how bnx2i does and needs a similar test. This patch has us just use the suspend_bit test for this. When ep_poll has succeeed iscsid will call conn_bind, the LLD will then call iscsi_conn_bind which will clear the suspend bit. When ep_disconnect is called (or if there is a conn error) we set the suspend bit. For the ep_disconnect case I am adding a helper in this patch that will take the session lock to make sure iscsi_queuecommand/xmit_task is not running and it will set the suspend bit. Signed-off-by: Mike Christie Signed-off-by: Jayamohan Kallickal Signed-off-by: James Bottomley --- drivers/scsi/libiscsi.c | 33 +++++++++++++++++++++++++++++++++ include/scsi/libiscsi.h | 1 + 2 files changed, 34 insertions(+) (limited to 'drivers/scsi/libiscsi.c') diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c index 67b2a2b0028..8dc73c489a1 100644 --- a/drivers/scsi/libiscsi.c +++ b/drivers/scsi/libiscsi.c @@ -1571,6 +1571,12 @@ int iscsi_queuecommand(struct scsi_cmnd *sc, void (*done)(struct scsi_cmnd *)) goto fault; } + if (test_bit(ISCSI_SUSPEND_BIT, &conn->suspend_tx)) { + reason = FAILURE_SESSION_IN_RECOVERY; + sc->result = DID_REQUEUE; + goto fault; + } + if (iscsi_check_cmdsn_window_closed(conn)) { reason = FAILURE_WINDOW_CLOSED; goto reject; @@ -1810,6 +1816,33 @@ static void fail_scsi_tasks(struct iscsi_conn *conn, unsigned lun, } } +/** + * iscsi_suspend_queue - suspend iscsi_queuecommand + * @conn: iscsi conn to stop queueing IO on + * + * This grabs the session lock to make sure no one is in + * xmit_task/queuecommand, and then sets suspend to prevent + * new commands from being queued. This only needs to be called + * by offload drivers that need to sync a path like ep disconnect + * with the iscsi_queuecommand/xmit_task. To start IO again libiscsi + * will call iscsi_start_tx and iscsi_unblock_session when in FFP. + */ +void iscsi_suspend_queue(struct iscsi_conn *conn) +{ + spin_lock_bh(&conn->session->lock); + set_bit(ISCSI_SUSPEND_BIT, &conn->suspend_tx); + spin_unlock_bh(&conn->session->lock); +} +EXPORT_SYMBOL_GPL(iscsi_suspend_queue); + +/** + * iscsi_suspend_tx - suspend iscsi_data_xmit + * @conn: iscsi conn tp stop processing IO on. + * + * This function sets the suspend bit to prevent iscsi_data_xmit + * from sending new IO, and if work is queued on the xmit thread + * it will wait for it to be completed. + */ void iscsi_suspend_tx(struct iscsi_conn *conn) { struct Scsi_Host *shost = conn->session->host; diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h index 439c8b75cb6..887e57e3e22 100644 --- a/include/scsi/libiscsi.h +++ b/include/scsi/libiscsi.h @@ -390,6 +390,7 @@ extern void iscsi_session_failure(struct iscsi_session *session, extern int iscsi_conn_get_param(struct iscsi_cls_conn *cls_conn, enum iscsi_param param, char *buf); extern void iscsi_suspend_tx(struct iscsi_conn *conn); +extern void iscsi_suspend_queue(struct iscsi_conn *conn); extern void iscsi_conn_queue_work(struct iscsi_conn *conn); #define iscsi_conn_printk(prefix, _c, fmt, a...) \ -- cgit v1.2.3-70-g09d2