[ewg] [PATCH ofed-1.2]: 2 bug fixes for iSER

Erez Zilber erezz at voltaire.com
Tue Apr 10 00:58:56 PDT 2007


Vlad,

Please add the following 2 fixes (already in Roland's tree) to kernel_patches/fixes.

do not assume that a task may be aborted only after the qp times out

scsi-ml may abort a command that was already sent. If the initiator is
still trying to send the command (or data-out PDUs for that command), the
qp may time out after scsi-ml times out. Therefore, when aborting the command,
iSER may still have references for the command's buffers. When sending
these PDUs will complete with an error, their resources will be released.

Signed-off-by: Erez Zilber <erezz at voltaire.com>
---
 drivers/infiniband/ulp/iser/iser_initiator.c |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c b/drivers/infiniband/ulp/iser/iser_initiator.c
index 89e3728..278fcbc 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -658,6 +658,7 @@ void iser_ctask_rdma_finalize(struct isc
 {
 	int deferred;
 	int is_rdma_aligned = 1;
+	struct iser_regd_buf *regd;
 
 	/* if we were reading, copy back to unaligned sglist,
 	 * anyway dma_unmap and free the copy
@@ -672,20 +673,20 @@ void iser_ctask_rdma_finalize(struct isc
 	}
 
 	if (iser_ctask->dir[ISER_DIR_IN]) {
-		deferred = iser_regd_buff_release
-			(&iser_ctask->rdma_regd[ISER_DIR_IN]);
+		regd = &iser_ctask->rdma_regd[ISER_DIR_IN];
+		deferred = iser_regd_buff_release(regd);
 		if (deferred) {
-			iser_err("References remain for BUF-IN rdma reg\n");
-			BUG();
+			iser_err("%d references remain for BUF-IN rdma reg\n",
+				 atomic_read(&regd->ref_count));
 		}
 	}
 
 	if (iser_ctask->dir[ISER_DIR_OUT]) {
-		deferred = iser_regd_buff_release
-			(&iser_ctask->rdma_regd[ISER_DIR_OUT]);
+		regd = &iser_ctask->rdma_regd[ISER_DIR_OUT];
+		deferred = iser_regd_buff_release(regd);
 		if (deferred) {
-			iser_err("References remain for BUF-OUT rdma reg\n");
-			BUG();
+			iser_err("%d references remain for BUF-OUT rdma reg\n",
+				 atomic_read(&regd->ref_count));
 		}
 	}
 
-- 
1.4.2

do not switch context when notifying the iSCSI layer on a connection failure

When a connection is terminated asynchronously from the iSCSI
layer's perspective, iSER needs to notify the iSCSI layer that the
connection has failed. This is done using a workqueue (switched to
from the iSER tasklet context). Meanwhile, the connection object (that
holds the work struct) is released. If the workqueue function wasn't
called yet, it will be called later with a NULL pointer that will
crash the kernel.

The context switch (tasklet to workqueue) is not required, and
everything can be done from the iSER tasklet. This eliminates the NULL
work struct bug (and simplifies the code).

Signed-off-by: Erez Zilber <erezz at voltaire.com>
---
 drivers/infiniband/ulp/iser/iscsi_iser.h |    1 -
 drivers/infiniband/ulp/iser/iser_verbs.c |   40 ++++++++++++------------------
 2 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index cae8c96..8960196 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -245,7 +245,6 @@ struct iser_conn {
 	wait_queue_head_t	     wait;          /* waitq for conn/disconn  */
 	atomic_t                     post_recv_buf_count; /* posted rx count   */
 	atomic_t                     post_send_buf_count; /* posted tx count   */
-	struct work_struct           comperror_work; /* conn term sleepable ctx*/
 	char 			     name[ISER_OBJECT_NAME_SIZE];
 	struct iser_page_vec         *page_vec;     /* represents SG to fmr maps*
 						     * maps serialized as tx is*/
diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index 693b770..1fc9674 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -48,7 +48,6 @@ #define ISER_MAX_CQ_LEN		((ISER_QP_MAX_R
 
 static void iser_cq_tasklet_fn(unsigned long data);
 static void iser_cq_callback(struct ib_cq *cq, void *cq_context);
-static void iser_comp_error_worker(struct work_struct *work);
 
 static void iser_cq_event_callback(struct ib_event *cause, void *context)
 {
@@ -480,7 +479,6 @@ int iser_conn_init(struct iser_conn **ib
 	init_waitqueue_head(&ib_conn->wait);
 	atomic_set(&ib_conn->post_recv_buf_count, 0);
 	atomic_set(&ib_conn->post_send_buf_count, 0);
-	INIT_WORK(&ib_conn->comperror_work, iser_comp_error_worker);
 	INIT_LIST_HEAD(&ib_conn->conn_list);
 	spin_lock_init(&ib_conn->lock);
 
@@ -753,26 +751,6 @@ int iser_post_send(struct iser_desc *tx_
 	return ret_val;
 }
 
-static void iser_comp_error_worker(struct work_struct *work)
-{
-	struct iser_conn *ib_conn =
-		container_of(work, struct iser_conn, comperror_work);
-
-	/* getting here when the state is UP means that the conn is being *
-	 * terminated asynchronously from the iSCSI layer's perspective.  */
-	if (iser_conn_state_comp_exch(ib_conn, ISER_CONN_UP,
-				      ISER_CONN_TERMINATING))
-		iscsi_conn_failure(ib_conn->iser_conn->iscsi_conn,
-					ISCSI_ERR_CONN_FAILED);
-
-	/* complete the termination process if disconnect event was delivered *
-	 * note there are no more non completed posts to the QP               */
-	if (ib_conn->disc_evt_flag) {
-		ib_conn->state = ISER_CONN_DOWN;
-		wake_up_interruptible(&ib_conn->wait);
-	}
-}
-
 static void iser_handle_comp_error(struct iser_desc *desc)
 {
 	struct iser_dto  *dto     = &desc->dto;
@@ -791,8 +769,22 @@ static void iser_handle_comp_error(struc
 	}
 
 	if (atomic_read(&ib_conn->post_recv_buf_count) == 0 &&
-	    atomic_read(&ib_conn->post_send_buf_count) == 0)
-		schedule_work(&ib_conn->comperror_work);
+	    atomic_read(&ib_conn->post_send_buf_count) == 0) {
+		/* getting here when the state is UP means that the conn is *
+		 * being terminated asynchronously from the iSCSI layer's   *
+		 * perspective.                                             */
+		if (iser_conn_state_comp_exch(ib_conn, ISER_CONN_UP,
+		    ISER_CONN_TERMINATING))
+			iscsi_conn_failure(ib_conn->iser_conn->iscsi_conn,
+					   ISCSI_ERR_CONN_FAILED);
+
+		/* complete the termination process if disconnect event was delivered *
+		 * note there are no more non completed posts to the QP               */
+		if (ib_conn->disc_evt_flag) {
+			ib_conn->state = ISER_CONN_DOWN;
+			wake_up_interruptible(&ib_conn->wait);
+		}
+	}
 }
 
 static void iser_cq_tasklet_fn(unsigned long data)
-- 
1.4.2

-- 
____________________________________________________________

Erez Zilber   |  972-9-971-7689

Software Engineer, Storage Team

Voltaire – _The Grid Backbone_

 __

 www.voltaire.com <http://www.voltaire.com/>

<mailto:erezz at voltaire.com>

  





More information about the ewg mailing list