lkml.org 
[lkml]   [2006]   [May]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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/

\
 
 \ /
  Last update: 2006-05-04 15:03    [W:0.094 / U:0.068 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site