On Thu, Jun 02, 2005 at 12:55:34PM -0700, Sean Hefty wrote:
> The ucm code in ib_ucm_event_handler() reads the cm_id->state information in
> response to an event occurrence.  The cm_id state can change dynamically in
> response to an event, and in general should not be accessed.  (I.e. the
> current cm_id state may not match up with the event being reported.)
> 
> Access to the state requires coordination with the CM code itself, so trying
> to rely on it to drive external states will eventually lead to problems.
> Clients of the CM should either maintain their own state information or
> perform operations based on the occurrence of the reported event.
> 
> I had originally exposed the state only for debugging purposes.  A comment
> was added that it should only be used for internal/debug use, but I will
> make a note to remove the state from the externally visible cm_id structure.

  Makes sense to me. This patch removes the state variable from both
kernel and userspace UCM code. I'll also work on a patch for SDP when
I get a chance, but it should be fairly straigh forward.

-Libor

Index: linux-kernel/infiniband/include/ib_user_cm.h
===================================================================
--- linux-kernel/infiniband/include/ib_user_cm.h        (revision 2564)
+++ linux-kernel/infiniband/include/ib_user_cm.h        (working copy)
@@ -90,8 +90,6 @@
 struct ib_ucm_attr_id_resp {
        __u64 service_id;
        __u64 service_mask;
-       __u32 state;
-       __u32 lap_state;
        __u32 local_id;
        __u32 remote_id;
 };
@@ -310,7 +308,6 @@
 
 struct ib_ucm_event_resp {
        __u32 id;
-       __u32 state;
        __u32 event;
        __u32 present;
        union {
Index: linux-kernel/infiniband/core/ucm.c
===================================================================
--- linux-kernel/infiniband/core/ucm.c  (revision 2564)
+++ linux-kernel/infiniband/core/ucm.c  (working copy)
@@ -58,7 +58,7 @@
 
 static struct semaphore ctx_id_mutex;
 static struct idr       ctx_id_table;
-static int           ctx_id_rover = 0;
+static int              ctx_id_rover = 0;
 
 static struct ib_ucm_context *ib_ucm_ctx_get(int id)
 {
@@ -428,7 +428,6 @@
 
        uevent->resp.id    = id;
        uevent->resp.event = event->event;
-       uevent->resp.state = cm_id->state;
 
        result = ib_ucm_event_process(event, uevent);
        if (result)
@@ -652,8 +651,6 @@
 
        resp.service_id   = ctx->cm_id->service_id;
        resp.service_mask = ctx->cm_id->service_mask;
-       resp.state        = ctx->cm_id->state;
-       resp.lap_state    = ctx->cm_id->lap_state;
        resp.local_id     = ctx->cm_id->local_id;
        resp.remote_id    = ctx->cm_id->remote_id;
 
Index: userspace/libibcm/include/infiniband/cm.h
===================================================================
--- userspace/libibcm/include/infiniband/cm.h   (revision 2521)
+++ userspace/libibcm/include/infiniband/cm.h   (working copy)
@@ -39,33 +39,6 @@
 #include <infiniband/verbs.h>
 #include <infiniband/sa.h>
 
-enum ib_cm_state {
-       IB_CM_IDLE,
-       IB_CM_LISTEN,
-       IB_CM_REQ_SENT,
-       IB_CM_REQ_RCVD,
-       IB_CM_MRA_REQ_SENT,
-       IB_CM_MRA_REQ_RCVD,
-       IB_CM_REP_SENT,
-       IB_CM_REP_RCVD,
-       IB_CM_MRA_REP_SENT,
-       IB_CM_MRA_REP_RCVD,
-       IB_CM_ESTABLISHED,
-       IB_CM_DREQ_SENT,
-       IB_CM_DREQ_RCVD,
-       IB_CM_TIMEWAIT,
-       IB_CM_SIDR_REQ_SENT,
-       IB_CM_SIDR_REQ_RCVD
-};
-
-enum ib_cm_lap_state {
-       IB_CM_LAP_IDLE,
-       IB_CM_LAP_SENT,
-       IB_CM_LAP_RCVD,
-       IB_CM_MRA_LAP_SENT,
-       IB_CM_MRA_LAP_RCVD,
-};
-
 enum ib_cm_event_type {
        IB_CM_REQ_ERROR,
        IB_CM_REQ_RECEIVED,
@@ -240,7 +213,6 @@
 struct ib_cm_event {
        uint32_t              cm_id;
        enum ib_cm_event_type event;
-       enum ib_cm_state      state;
        union {
                struct ib_cm_req_event_param    req_rcvd;
                struct ib_cm_rep_event_param    rep_rcvd;
@@ -313,8 +285,6 @@
 struct ib_cm_attr_param {
        uint64_t                service_id;
        uint64_t                service_mask;
-       enum ib_cm_state        state;
-       enum ib_cm_lap_state    lap_state;
        uint32_t                local_id;
        uint32_t                remote_id;
 };
Index: userspace/libibcm/include/infiniband/cm_abi.h
===================================================================
--- userspace/libibcm/include/infiniband/cm_abi.h       (revision 2521)
+++ userspace/libibcm/include/infiniband/cm_abi.h       (working copy)
@@ -94,8 +94,6 @@
 struct cm_abi_attr_id_resp {
        __u64 service_id;
        __u64 service_mask;
-       __u32 state;
-       __u32 lap_state;
        __u32 local_id;
        __u32 remote_id;
 };
@@ -314,7 +312,6 @@
 
 struct cm_abi_event_resp {
        __u32 id;
-       __u32 state;
        __u32 event;
        __u32 present;
        union {
Index: userspace/libibcm/src/cm.c
===================================================================
--- userspace/libibcm/src/cm.c  (revision 2521)
+++ userspace/libibcm/src/cm.c  (working copy)
@@ -188,8 +188,6 @@
 
        param->service_id   = resp->service_id;
        param->service_mask = resp->service_mask;
-       param->state        = resp->state;
-       param->lap_state    = resp->lap_state;
        param->local_id     = resp->local_id;
        param->remote_id    = resp->remote_id;
 
@@ -773,7 +771,6 @@
 
        evt->cm_id = resp->id;
        evt->event = resp->event;
-       evt->state = resp->state;
 
        if (resp->present & CM_ABI_PRES_PRIMARY) {
 
Index: userspace/libibcm/examples/simple.c
===================================================================
--- userspace/libibcm/examples/simple.c (revision 2521)
+++ userspace/libibcm/examples/simple.c (working copy)
@@ -168,11 +168,10 @@
                        goto done;
                }
 
-               printf("CM ID <%d> Event <%d> State <%d>\n", 
-                      event->cm_id, event->event, event->state);
+               printf("CM ID <%d> Event <%d>\n", event->cm_id, event->event);
 
-               switch (event->state) {
-               case IB_CM_REQ_RCVD:
+               switch (event->event) {
+               case IB_CM_REQ_RECEIVED:
 
                        result = ib_cm_destroy_id(cm_id);
                        if (result < 0) {
@@ -205,7 +204,7 @@
                        }
                
                        break;
-               case IB_CM_REP_RCVD:
+               case IB_CM_REP_RECEIVED:
 
                        result = ib_cm_send_rtu(cm_id, NULL, 0);
                        if (result < 0) {
@@ -215,7 +214,7 @@
                        }
 
                        break;
-               case IB_CM_ESTABLISHED:
+               case IB_CM_RTU_RECEIVED:
 
                        result = ib_cm_send_dreq(cm_id, NULL, 0);
                        if (result < 0) {
@@ -225,7 +224,7 @@
                        }
 
                        break;
-               case IB_CM_DREQ_RCVD:
+               case IB_CM_DREQ_RECEIVED:
 
                        result = ib_cm_send_drep(cm_id, NULL, 0);
                        if (result < 0) {
@@ -235,15 +234,14 @@
                        }
 
                        break;
-               case IB_CM_TIMEWAIT:
+               case IB_CM_DREP_RECEIVED:
                        break;
-               case IB_CM_IDLE:
+               case IB_CM_TIMEWAIT_EXIT:
                        status = 1;
                        break;
                default:
                        status = EINVAL;
-                       printf("Unhandled state <%d:%d>\n", 
-                              event->state, event->event);
+                       printf("Unhandled event <%d>\n", event->event);
                        break;
                }
 
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general

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

Reply via email to