[ofa-general] [RFC V4 PATCH 3/5] rdma/cma: simply locking needed for serialization of callbacks
Or Gerlitz
ogerlitz at voltaire.com
Wed May 28 04:34:31 PDT 2008
The rdma-cm has some logic in place to make sure that callbacks on an ID are
delivered to the consumer in serialized manner, specifically it has code to protect
against the device removal racing with a callback now being delivered to the user.
This patch simplifies this logic by using a mutex per ID instead of the wait queue
and atomic variable. I have left the disable/enable_remove notation such that the patch
would be easier to read, but if this approach is accepted, I think we want to change
it to disable/enable_callback
Signed-off-by: Or Gerlitz <ogerlitz at voltaire.com>
cma.c | 96 ++++++++++++++++++++++++++++++++----------------------------------
1 files changed, 47 insertions(+), 49 deletions(-)
changes from v2 (I named this v4 to comply with the next patch)
- cma_disable_remove --> cma_disable_callback, acquire the mutex before the spinlock
- removed cma_enable_remove and just call mutex_unlock(id->handler_mutex) instead
Sean, basically you asked that cma_disable_remove be removed from the code, but this
would spread taking the spin lock and doing state checks on all the places which call
it, so I think it can be nice to still have it. As for the spin lock usage, I preferred
not to touch it, since the code of cma_comp, cma_exch, cma_comp_exch etc use it.
Index: linux-2.6.26-rc3/drivers/infiniband/core/cma.c
===================================================================
--- linux-2.6.26-rc3.orig/drivers/infiniband/core/cma.c 2008-05-26 15:11:17.000000000 +0300
+++ linux-2.6.26-rc3/drivers/infiniband/core/cma.c 2008-05-28 11:08:24.000000000 +0300
@@ -126,8 +126,7 @@ struct rdma_id_private {
struct completion comp;
atomic_t refcount;
- wait_queue_head_t wait_remove;
- atomic_t dev_remove;
+ struct mutex handler_mutex;
int backlog;
int timeout_ms;
@@ -351,28 +350,24 @@ static void cma_deref_id(struct rdma_id_
complete(&id_priv->comp);
}
-static int cma_disable_remove(struct rdma_id_private *id_priv,
+static int cma_disable_callback(struct rdma_id_private *id_priv,
enum cma_state state)
{
unsigned long flags;
int ret;
+ mutex_lock(&id_priv->handler_mutex);
spin_lock_irqsave(&id_priv->lock, flags);
- if (id_priv->state == state) {
- atomic_inc(&id_priv->dev_remove);
+ if (id_priv->state == state)
ret = 0;
- } else
+ else {
+ mutex_unlock(&id_priv->handler_mutex);
ret = -EINVAL;
+ }
spin_unlock_irqrestore(&id_priv->lock, flags);
return ret;
}
-static void cma_enable_remove(struct rdma_id_private *id_priv)
-{
- if (atomic_dec_and_test(&id_priv->dev_remove))
- wake_up(&id_priv->wait_remove);
-}
-
static int cma_has_cm_dev(struct rdma_id_private *id_priv)
{
return (id_priv->id.device && id_priv->cm_id.ib);
@@ -395,8 +390,7 @@ struct rdma_cm_id *rdma_create_id(rdma_c
mutex_init(&id_priv->qp_mutex);
init_completion(&id_priv->comp);
atomic_set(&id_priv->refcount, 1);
- init_waitqueue_head(&id_priv->wait_remove);
- atomic_set(&id_priv->dev_remove, 0);
+ mutex_init(&id_priv->handler_mutex);
INIT_LIST_HEAD(&id_priv->listen_list);
INIT_LIST_HEAD(&id_priv->mc_list);
get_random_bytes(&id_priv->seq_num, sizeof id_priv->seq_num);
@@ -923,7 +917,7 @@ 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 (cma_disable_callback(id_priv, CMA_CONNECT))
return 0;
memset(&event, 0, sizeof event);
@@ -980,12 +974,12 @@ static int cma_ib_handler(struct ib_cm_i
/* Destroy the CM ID by returning a non-zero value. */
id_priv->cm_id.ib = NULL;
cma_exch(id_priv, CMA_DESTROYING);
- cma_enable_remove(id_priv);
+ mutex_unlock(&id_priv->handler_mutex);
rdma_destroy_id(&id_priv->id);
return ret;
}
out:
- cma_enable_remove(id_priv);
+ mutex_unlock(&id_priv->handler_mutex);
return ret;
}
@@ -1097,7 +1091,7 @@ static int cma_req_handler(struct ib_cm_
int offset, ret;
listen_id = cm_id->context;
- if (cma_disable_remove(listen_id, CMA_LISTEN))
+ if (cma_disable_callback(listen_id, CMA_LISTEN))
return -ECONNABORTED;
memset(&event, 0, sizeof event);
@@ -1118,7 +1112,7 @@ static int cma_req_handler(struct ib_cm_
goto out;
}
- atomic_inc(&conn_id->dev_remove);
+ mutex_lock(&conn_id->handler_mutex);
mutex_lock(&lock);
ret = cma_acquire_dev(conn_id);
mutex_unlock(&lock);
@@ -1140,7 +1134,7 @@ static int cma_req_handler(struct ib_cm_
!cma_is_ud_ps(conn_id->id.ps))
ib_send_cm_mra(cm_id, CMA_CM_MRA_SETTING, NULL, 0);
mutex_unlock(&lock);
- cma_enable_remove(conn_id);
+ mutex_unlock(&conn_id->handler_mutex);
goto out;
}
@@ -1149,11 +1143,11 @@ static int cma_req_handler(struct ib_cm_
release_conn_id:
cma_exch(conn_id, CMA_DESTROYING);
- cma_enable_remove(conn_id);
+ mutex_unlock(&conn_id->handler_mutex);
rdma_destroy_id(&conn_id->id);
out:
- cma_enable_remove(listen_id);
+ mutex_unlock(&listen_id->handler_mutex);
return ret;
}
@@ -1219,7 +1213,7 @@ static int cma_iw_handler(struct iw_cm_i
struct sockaddr_in *sin;
int ret = 0;
- if (cma_disable_remove(id_priv, CMA_CONNECT))
+ if (cma_disable_callback(id_priv, CMA_CONNECT))
return 0;
memset(&event, 0, sizeof event);
@@ -1263,12 +1257,12 @@ static int cma_iw_handler(struct iw_cm_i
/* Destroy the CM ID by returning a non-zero value. */
id_priv->cm_id.iw = NULL;
cma_exch(id_priv, CMA_DESTROYING);
- cma_enable_remove(id_priv);
+ mutex_unlock(&id_priv->handler_mutex);
rdma_destroy_id(&id_priv->id);
return ret;
}
- cma_enable_remove(id_priv);
+ mutex_unlock(&id_priv->handler_mutex);
return ret;
}
@@ -1284,7 +1278,7 @@ static int iw_conn_req_handler(struct iw
struct ib_device_attr attr;
listen_id = cm_id->context;
- if (cma_disable_remove(listen_id, CMA_LISTEN))
+ if (cma_disable_callback(listen_id, CMA_LISTEN))
return -ECONNABORTED;
/* Create a new RDMA id for the new IW CM ID */
@@ -1296,19 +1290,19 @@ static int iw_conn_req_handler(struct iw
goto out;
}
conn_id = container_of(new_cm_id, struct rdma_id_private, id);
- atomic_inc(&conn_id->dev_remove);
+ mutex_lock(&conn_id->handler_mutex);
conn_id->state = CMA_CONNECT;
dev = ip_dev_find(&init_net, iw_event->local_addr.sin_addr.s_addr);
if (!dev) {
ret = -EADDRNOTAVAIL;
- cma_enable_remove(conn_id);
+ mutex_unlock(&conn_id->handler_mutex);
rdma_destroy_id(new_cm_id);
goto out;
}
ret = rdma_copy_addr(&conn_id->id.route.addr.dev_addr, dev, NULL);
if (ret) {
- cma_enable_remove(conn_id);
+ mutex_unlock(&conn_id->handler_mutex);
rdma_destroy_id(new_cm_id);
goto out;
}
@@ -1317,7 +1311,7 @@ static int iw_conn_req_handler(struct iw
ret = cma_acquire_dev(conn_id);
mutex_unlock(&lock);
if (ret) {
- cma_enable_remove(conn_id);
+ mutex_unlock(&conn_id->handler_mutex);
rdma_destroy_id(new_cm_id);
goto out;
}
@@ -1333,7 +1327,7 @@ static int iw_conn_req_handler(struct iw
ret = ib_query_device(conn_id->id.device, &attr);
if (ret) {
- cma_enable_remove(conn_id);
+ mutex_unlock(&conn_id->handler_mutex);
rdma_destroy_id(new_cm_id);
goto out;
}
@@ -1349,14 +1343,14 @@ static int iw_conn_req_handler(struct iw
/* User wants to destroy the CM ID */
conn_id->cm_id.iw = NULL;
cma_exch(conn_id, CMA_DESTROYING);
- cma_enable_remove(conn_id);
+ mutex_unlock(&conn_id->handler_mutex);
rdma_destroy_id(&conn_id->id);
}
out:
if (dev)
dev_put(dev);
- cma_enable_remove(listen_id);
+ mutex_unlock(&listen_id->handler_mutex);
return ret;
}
@@ -1588,7 +1582,7 @@ static void cma_work_handler(struct work
struct rdma_id_private *id_priv = work->id;
int destroy = 0;
- atomic_inc(&id_priv->dev_remove);
+ mutex_lock(&id_priv->handler_mutex);
if (!cma_comp_exch(id_priv, work->old_state, work->new_state))
goto out;
@@ -1597,7 +1591,7 @@ static void cma_work_handler(struct work
destroy = 1;
}
out:
- cma_enable_remove(id_priv);
+ mutex_unlock(&id_priv->handler_mutex);
cma_deref_id(id_priv);
if (destroy)
rdma_destroy_id(&id_priv->id);
@@ -1760,7 +1754,7 @@ static void addr_handler(int status, str
struct rdma_cm_event event;
memset(&event, 0, sizeof event);
- atomic_inc(&id_priv->dev_remove);
+ mutex_lock(&id_priv->handler_mutex);
/*
* Grab mutex to block rdma_destroy_id() from removing the device while
@@ -1789,13 +1783,13 @@ static void addr_handler(int status, str
if (id_priv->id.event_handler(&id_priv->id, &event)) {
cma_exch(id_priv, CMA_DESTROYING);
- cma_enable_remove(id_priv);
+ mutex_unlock(&id_priv->handler_mutex);
cma_deref_id(id_priv);
rdma_destroy_id(&id_priv->id);
return;
}
out:
- cma_enable_remove(id_priv);
+ mutex_unlock(&id_priv->handler_mutex);
cma_deref_id(id_priv);
}
@@ -2122,7 +2116,7 @@ static int cma_sidr_rep_handler(struct i
struct ib_cm_sidr_rep_event_param *rep = &ib_event->param.sidr_rep_rcvd;
int ret = 0;
- if (cma_disable_remove(id_priv, CMA_CONNECT))
+ if (cma_disable_callback(id_priv, CMA_CONNECT))
return 0;
memset(&event, 0, sizeof event);
@@ -2163,12 +2157,12 @@ static int cma_sidr_rep_handler(struct i
/* Destroy the CM ID by returning a non-zero value. */
id_priv->cm_id.ib = NULL;
cma_exch(id_priv, CMA_DESTROYING);
- cma_enable_remove(id_priv);
+ mutex_unlock(&id_priv->handler_mutex);
rdma_destroy_id(&id_priv->id);
return ret;
}
out:
- cma_enable_remove(id_priv);
+ mutex_unlock(&id_priv->handler_mutex);
return ret;
}
@@ -2566,8 +2560,8 @@ static int cma_ib_mc_handler(int status,
int ret;
id_priv = mc->id_priv;
- if (cma_disable_remove(id_priv, CMA_ADDR_BOUND) &&
- cma_disable_remove(id_priv, CMA_ADDR_RESOLVED))
+ if (cma_disable_callback(id_priv, CMA_ADDR_BOUND) &&
+ cma_disable_callback(id_priv, CMA_ADDR_RESOLVED))
return 0;
mutex_lock(&id_priv->qp_mutex);
@@ -2592,12 +2586,12 @@ static int cma_ib_mc_handler(int status,
ret = id_priv->id.event_handler(&id_priv->id, &event);
if (ret) {
cma_exch(id_priv, CMA_DESTROYING);
- cma_enable_remove(id_priv);
+ mutex_unlock(&id_priv->handler_mutex);
rdma_destroy_id(&id_priv->id);
return 0;
}
- cma_enable_remove(id_priv);
+ mutex_unlock(&id_priv->handler_mutex);
return 0;
}
@@ -2756,22 +2750,26 @@ static int cma_remove_id_dev(struct rdma
{
struct rdma_cm_event event;
enum cma_state state;
-
+ int ret = 0;
+
/* Record that we want to remove the device */
state = cma_exch(id_priv, CMA_DEVICE_REMOVAL);
if (state == CMA_DESTROYING)
return 0;
cma_cancel_operation(id_priv, state);
- wait_event(id_priv->wait_remove, !atomic_read(&id_priv->dev_remove));
+ mutex_lock(&id_priv->handler_mutex);
/* Check for destruction from another callback. */
if (!cma_comp(id_priv, CMA_DEVICE_REMOVAL))
- return 0;
+ goto out;
memset(&event, 0, sizeof event);
event.event = RDMA_CM_EVENT_DEVICE_REMOVAL;
- return id_priv->id.event_handler(&id_priv->id, &event);
+ ret = id_priv->id.event_handler(&id_priv->id, &event);
+out:
+ mutex_unlock(&id_priv->handler_mutex);
+ return ret;
}
static void cma_process_remove(struct cma_device *cma_dev)
More information about the general
mailing list