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 <[EMAIL PROTECTED]

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);
-               }
        }
 }


_______________________________________________
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Reply via email to