[ofa-general] cma: add support for TIMEWAIT_EXIT event (second try)

[Amir Vadai]

Sean Hi,

Acquiring the handler_mutex explicitly sounds like a good idea.

When will it be commited? Will we have it in 2.6.27?

Thanks,
- Amir
 

> -----Original Message-----
> From: Sean Hefty [mailto:sean.hefty at intel.com] 
> Sent: Wednesday, July 02, 2008 9:48 PM
> To: Amir Vadai
> Cc: general at lists.openfabrics.org; Oren Duer
> Subject: RE: [ofa-general] cma: add support for TIMEWAIT_EXIT 
> event (second try)
> 
> >SDP needs to be notified when CM exits the TIMEWAIT state.
> >CM does notify the CMA but CMA doesn't pass it to the upper layer.
> >
> >Below is a patch for the CMA code. I wasn't sure if it is ok to set
> >CMA_DISCONNECT in the IB_CM_TIMEWAIT_EXIT instead of doing it in the
> >IB_CM_DREQ_RECEIVED/IB_CM_DREP_RECEIVED as it is done today. Instead
> >I changed the check at the beginning of cma_ib_handler() as you can
> >see in the patch.
> >
> >Signed-off-by: Amir Vadai <amirv at mellanox.co.il>
> 
> Sorry I was slow to get to this.  The general approach looks 
> fine to me.
> 
> >--- include/rdma/rdma_cm.h.orig    2008-06-18 
> 15:04:22.560904000 +0300
> >+++ include/rdma/rdma_cm.h    2008-06-11 11:55:26.758053000 +0300
> >@@ -53,7 +53,8 @@ enum rdma_cm_event_type {
> >     RDMA_CM_EVENT_DISCONNECTED,
> >     RDMA_CM_EVENT_DEVICE_REMOVAL,
> >     RDMA_CM_EVENT_MULTICAST_JOIN,
> >-    RDMA_CM_EVENT_MULTICAST_ERROR
> >+    RDMA_CM_EVENT_MULTICAST_ERROR,
> >+    RDMA_CM_EVENT_TIMWAIT_EXIT
> > };
> 
> This will need to be updated based on the address change 
> event patch that went
> in.  I'll update the librdmacm myself separately.
> 
> > enum rdma_port_space {
> >--- drivers/infiniband/core/cma.c.orig    2008-06-11 
> 11:24:38.021543000
> >+0300
> >+++ drivers/infiniband/core/cma.c    2008-06-18 
> 13:27:08.098747000 +0300
> >@@ -931,7 +931,8 @@ static int cma_ib_handler(struct ib_cm_i
> >     struct rdma_cm_event event;
> >     int ret = 0;
> >
> >-    if (cma_disable_remove(id_priv, CMA_CONNECT))
> >+    if ((ib_event->event != IB_CM_TIMEWAIT_EXIT &&
> >cma_disable_remove(id_priv, CMA_CONNECT)) ||
> >+        (ib_event->event == IB_CM_TIMEWAIT_EXIT &&
> >cma_disable_remove(id_priv, CMA_DISCONNECT)))
> >         return 0;
> 
> I wonder if it would be better to acquire the handler_mutex 
> explicitly, like
> this:
> 
> 	mutex_lock(&id_priv->handler_mutex);
> 	if ((id_priv->state == CMA_CONNECT &&
> 	     ib_event->event != IB_CM_TIMEWAIT_EXIT) ||
> 	    (id_priv->state == CMA_DISCONNECT &&
> 	     ib_event->event == IB_CM_TIMEWAIT_EXIT)) {
> 		mutex_unlock(&id_priv->handler_mutex);
> 		return 0;
> 	}
> 
> so that ensuring the mutex is acquired once isn't so subtle.
> 
> - Sean
> 
>