Messages in this thread Patch in this message | | | Date | Thu, 04 May 2006 16:00:52 +0300 | From | Or Gerlitz <> | Subject | Re: [openib-general] [PATCH 5/6] iser RDMA CM (CMA) and IB verbs interaction |
| |
Or Gerlitz wrote: > Sean Hefty wrote: >>> +static void iser_disconnected_handler(struct rdma_cm_id *cma_id) >>> +{ >>> + struct iser_conn *ib_conn; >>> + >>> + ib_conn = (struct iser_conn *)cma_id->context; >>> + ib_conn->disc_evt_flag = 1; >>> + >>> + /* If this event is unsolicited this means that the conn is >>> being */ >>> + /* terminated asynchronously from the iSCSI layer's >>> perspective. */ >>> + if (atomic_read(&ib_conn->state) == ISER_CONN_PENDING) { >>> + atomic_set(&ib_conn->state, ISER_CONN_DOWN); >>> + wake_up_interruptible(&ib_conn->wait); >>> + } else { >>> + if (atomic_read(&ib_conn->state) == ISER_CONN_UP) { >>> + atomic_set(&ib_conn->state, ISER_CONN_TERMINATING); >>> + iscsi_conn_failure(ib_conn->iser_conn->iscsi_conn, >>> + ISCSI_ERR_CONN_FAILED); >>> + } >>> + /* Complete the termination process if no posts are pending */ >>> + if ((atomic_read(&ib_conn->post_recv_buf_count) == 0) && >>> + (atomic_read(&ib_conn->post_send_buf_count) == 0)) { >>> + atomic_set(&ib_conn->state, ISER_CONN_DOWN); >>> + wake_up_interruptible(&ib_conn->wait); >>> + } >>> + }
>> Are there races here between reading ib_conn->state and setting it? >> Could it have changed in between the atomic_read() and atomic_set()?
> It seems that indeed a race is possible here, i am rethinking now on the > implementation of the ib connection states moves, thanks for pointing this.
Following a review and the clarification i have got from you re cma callbacks serialization, i have committed this change which removes unneeded state checks from two flows (disconnect handler and connect error)
Or.
r6900 | ogerlitz | 2006-05-04 11:06:24 +0300 (Thu, 04 May 2006) | 7 lines
two fixes to iser ib conn state management:
+1 when getting DISCONNECTED cma event, iser's state can't be PENDING +2 when connect_error is called, iser's state is PENDING, no need to check it
Signed-off-by: Or Gerlitz <ogerlitz@voltaire.com
Index: iser_verbs.c =================================================================== --- iser_verbs.c (revision 6802) +++ iser_verbs.c (revision 6900) @@ -309,12 +309,8 @@ static void iser_connect_error(struct rd struct iser_conn *ib_conn; ib_conn = (struct iser_conn *)cma_id->context;
- if (atomic_read(&ib_conn->state) == ISER_CONN_PENDING) { - atomic_set(&ib_conn->state, ISER_CONN_DOWN); - wake_up_interruptible(&ib_conn->wait); - } else - iser_err("Unexpected evt for conn.state: %d\n", - atomic_read(&ib_conn->state)); + atomic_set(&ib_conn->state, ISER_CONN_DOWN); + wake_up_interruptible(&ib_conn->wait); }
static void iser_addr_handler(struct rdma_cm_id *cma_id) @@ -386,21 +382,16 @@ static void iser_disconnected_handler(st
/* If this event is unsolicited this means that the conn is being */ /* terminated asynchronously from the iSCSI layer's perspective. */ - if (atomic_read(&ib_conn->state) == ISER_CONN_PENDING) { + if (atomic_read(&ib_conn->state) == ISER_CONN_UP) { + atomic_set(&ib_conn->state, ISER_CONN_TERMINATING); + iscsi_conn_failure(ib_conn->iser_conn->iscsi_conn, + ISCSI_ERR_CONN_FAILED); + } + /* Complete the termination process if no posts are pending */ + if ((atomic_read(&ib_conn->post_recv_buf_count) == 0) && + (atomic_read(&ib_conn->post_send_buf_count) == 0)) { atomic_set(&ib_conn->state, ISER_CONN_DOWN); wake_up_interruptible(&ib_conn->wait); - } else { - if (atomic_read(&ib_conn->state) == ISER_CONN_UP) { - atomic_set(&ib_conn->state, ISER_CONN_TERMINATING); - iscsi_conn_failure(ib_conn->iser_conn->iscsi_conn, - ISCSI_ERR_CONN_FAILED); - } - /* Complete the termination process if no posts are pending */ - if ((atomic_read(&ib_conn->post_recv_buf_count) == 0) && - (atomic_read(&ib_conn->post_send_buf_count) == 0)) { - atomic_set(&ib_conn->state, ISER_CONN_DOWN); - wake_up_interruptible(&ib_conn->wait); - } } }
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |