>-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;
> }
I wasn't clear on this before, but we shouldn't need to take the spinlock here
at all now. We needed it before in order to check the state and increment
dev_remove in one operation. Once the spinlock was released the state could
have changed, but dev_remove would have halted the device removal thread. Under
the new method, device removal is halted while we hold the handler_mutex.
>@@ -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))
This can end up trying to acquire the mutex twice. We could change this to
mutex_lock();
if (id_priv->state == CMA_ADDR_BOUND || id_priv->state == CMA_ADDR_RESOLVED)
- Sean
_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
