Skip to contentSkip to conversation

iscsi bugfixes: fix r2t handling

PATCH In review Mike Christie 1 participant 1 message Jul 24, 2006

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 2/11 iscsi bugfixes: fix r2t handling

The iscsi tcp code can pluck multiple rt2s from the tasks's r2tqueue in the xmit code. This can result in the task being queued on the xmit queue but gettting completed at the same time.

This patch fixes the above bug by making the fifo a list so we always remove the entry on the list del.

drivers/scsi/iscsi_tcp.c +21 -24
index 88dafdf..ab324d9 100644
@@ -185,11 +185,19 @@ iscsi_hdr_extract(struct iscsi_tcp_conn 
  * must be called with session lock
  */
 static void
-__iscsi_ctask_cleanup(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
+iscsi_tcp_cleanup_ctask(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
 {
 	struct iscsi_tcp_cmd_task *tcp_ctask = ctask->dd_data;
+	struct iscsi_r2t_info *r2t;
 	struct scsi_cmnd *sc;
 
+	/* flush ctask's r2t queues */
+	while (__kfifo_get(tcp_ctask->r2tqueue, (void*)&r2t, sizeof(void*))) {
+		__kfifo_put(tcp_ctask->r2tpool.queue, (void*)&r2t,
+			    sizeof(void*));
+		debug_scsi("iscsi_tcp_cleanup_ctask pending r2t dropped\n");
+	}
+
 	sc = ctask->sc;
 	if (unlikely(!sc))
 		return;
@@ -374,6 +382,7 @@ iscsi_r2t_rsp(struct iscsi_conn *conn, s
 		spin_unlock(&session->lock);
 		return 0;
 	}
+
 	rc = __kfifo_get(tcp_ctask->r2tpool.queue, (void*)&r2t, sizeof(void*));
 	BUG_ON(!rc);
 
@@ -399,7 +408,7 @@ iscsi_r2t_rsp(struct iscsi_conn *conn, s
 	tcp_ctask->exp_r2tsn = r2tsn + 1;
 	tcp_ctask->xmstate |= XMSTATE_SOL_HDR;
 	__kfifo_put(tcp_ctask->r2tqueue, (void*)&r2t, sizeof(void*));
-	__kfifo_put(conn->xmitqueue, (void*)&ctask, sizeof(void*));
+	list_move_tail(&ctask->running, &conn->xmitqueue);
 
 	scsi_queue_work(session->host, &conn->xmitwork);
 	conn->r2t_pdus_cnt++;
@@ -484,7 +493,7 @@ iscsi_tcp_hdr_recv(struct iscsi_conn *co
 			goto copy_hdr;
 
 		spin_lock(&session->lock);
-		__iscsi_ctask_cleanup(conn, tcp_conn->in.ctask);
+		iscsi_tcp_cleanup_ctask(conn, tcp_conn->in.ctask);
 		rc = __iscsi_complete_pdu(conn, hdr, NULL, 0);
 		spin_unlock(&session->lock);
 		break;
@@ -745,10 +754,11 @@ static int iscsi_scsi_data_in(struct isc
 done:
 	/* check for non-exceptional status */
 	if (tcp_conn->in.hdr->flags & ISCSI_FLAG_DATA_STATUS) {
-		debug_scsi("done [sc %lx res %d itt 0x%x]\n",
-			   (long)sc, sc->result, ctask->itt);
+		debug_scsi("done [sc %lx res %d itt 0x%x flags 0x%x]\n",
+			   (long)sc, sc->result, ctask->itt,
+			   tcp_conn->in.hdr->flags);
 		spin_lock(&conn->session->lock);
-		__iscsi_ctask_cleanup(conn, ctask);
+		iscsi_tcp_cleanup_ctask(conn, ctask);
 		__iscsi_complete_pdu(conn, tcp_conn->in.hdr, NULL, 0);
 		spin_unlock(&conn->session->lock);
 	}
@@ -769,7 +779,7 @@ iscsi_data_recv(struct iscsi_conn *conn)
 		break;
 	case ISCSI_OP_SCSI_CMD_RSP:
 		spin_lock(&conn->session->lock);
-		__iscsi_ctask_cleanup(conn, tcp_conn->in.ctask);
+		iscsi_tcp_cleanup_ctask(conn, tcp_conn->in.ctask);
 		spin_unlock(&conn->session->lock);
 	case ISCSI_OP_TEXT_RSP:
 	case ISCSI_OP_LOGIN_RSP:
@@ -1308,7 +1318,7 @@ iscsi_tcp_cmd_init(struct iscsi_cmd_task
 				    ctask->imm_count -
 				    ctask->unsol_count;
 
-		debug_scsi("cmd [itt %x total %d imm %d imm_data %d "
+		debug_scsi("cmd [itt 0x%x total %d imm %d imm_data %d "
 			   "r2t_data %d]\n",
 			   ctask->itt, ctask->total_length, ctask->imm_count,
 			   ctask->unsol_count, tcp_ctask->r2t_data_count);
@@ -1636,7 +1646,7 @@ handle_xmstate_sol_data(struct iscsi_con
 	}
 solicit_again:
 	/*
-	 * send Data-Out whitnin this R2T sequence.
+	 * send Data-Out within this R2T sequence.
 	 */
 	if (!r2t->data_count)
 		goto data_out_done;
@@ -1731,7 +1741,7 @@ handle_xmstate_w_pad(struct iscsi_conn *
 	struct iscsi_tcp_cmd_task *tcp_ctask = ctask->dd_data;
 	struct iscsi_tcp_conn *tcp_conn = conn->dd_data;
 	struct iscsi_data_task *dtask = tcp_ctask->dtask;
-	int sent, rc;
+	int sent = 0, rc;
 
 	tcp_ctask->xmstate &= ~XMSTATE_W_PAD;
 	iscsi_buf_init_iov(&tcp_ctask->sendbuf, (char*)&tcp_ctask->pad,
@@ -2002,20 +2012,6 @@ iscsi_tcp_conn_bind(struct iscsi_cls_ses
 }
 
 static void
-iscsi_tcp_cleanup_ctask(struct iscsi_conn *conn, struct iscsi_cmd_task *ctask)
-{
-	struct iscsi_tcp_cmd_task *tcp_ctask = ctask->dd_data;
-	struct iscsi_r2t_info *r2t;
-
-	/* flush ctask's r2t queues */
-	while (__kfifo_get(tcp_ctask->r2tqueue, (void*)&r2t, sizeof(void*)))
-		__kfifo_put(tcp_ctask->r2tpool.queue, (void*)&r2t,
-			    sizeof(void*));
-
-	__iscsi_ctask_cleanup(conn, ctask);
-}
-
-static void
 iscsi_tcp_suspend_conn_rx(struct iscsi_conn *conn)
 {
 	struct iscsi_tcp_conn *tcp_conn = conn->dd_data;
@@ -2057,6 +2053,7 @@ iscsi_tcp_mgmt_init(struct iscsi_conn *c
 	iscsi_buf_init_iov(&tcp_mtask->headbuf, (char*)mtask->hdr,
 			   sizeof(struct iscsi_hdr));
 	tcp_mtask->xmstate = XMSTATE_IMM_HDR;
+	tcp_mtask->sent = 0;
 
 	if (mtask->data_count)
 		iscsi_buf_init_iov(&tcp_mtask->sendbuf, (char*)mtask->data,
drivers/scsi/libiscsi.c +38 -52

178 lines not on this page · raw message

include/scsi/libiscsi.h +9 -1
index ba27608..e71d6e9 100644
@@ -83,6 +83,12 @@ struct iscsi_mgmt_task {
 	struct list_head	running;
 };
 
+enum {
+	ISCSI_TASK_COMPLETED,
+	ISCSI_TASK_PENDING,
+	ISCSI_TASK_RUNNING,
+};
+
 struct iscsi_cmd_task {
 	/*
 	 * Becuae LLDs allocate their hdr differently, this is a pointer to
@@ -101,6 +107,8 @@ struct iscsi_cmd_task {
 	struct iscsi_conn	*conn;		/* used connection    */
 	struct iscsi_mgmt_task	*mtask;		/* tmf mtask in progr */
 
+	/* state set/tested under session->lock */
+	int			state;
 	struct list_head	running;	/* running cmd list */
 	void			*dd_data;	/* driver/transport data */
 };
@@ -134,7 +142,7 @@ struct iscsi_conn {
 	struct kfifo		*immqueue;	/* immediate xmit queue */
 	struct kfifo		*mgmtqueue;	/* mgmt (control) xmit queue */
 	struct list_head	mgmt_run_list;	/* list of control tasks */
-	struct kfifo		*xmitqueue;	/* data-path cmd queue */
+	struct list_head	xmitqueue;	/* data-path cmd queue */
 	struct list_head	run_list;	/* list of cmds in progress */
 	struct work_struct	xmitwork;	/* per-conn. xmit workqueue */
 	/*