[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(®d->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(®d->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