[openib-general] [PATCH] librdmacm: updated librdmacm to work with proposed 2.6.20 kernel CMA

[Or Gerlitz]

Sean Hefty wrote:
> Updates the librdmacm to work with ABI version 3, which is the proposed
> kernel changes for inclusion in 2.6.20.

> @@ -929,74 +926,102 @@ int rdma_join_multicast(struct rdma_cm_i
>                         void *context)
>  {
>         struct ucma_abi_join_mcast *cmd;
> +       struct ucma_abi_create_id_resp *resp;
>         struct cma_id_private *id_priv;
> +       struct cma_multicast *mc, **pos;
>         void *msg;
>         int ret, size, addrlen;
>        
> +       id_priv = container_of(id, struct cma_id_private, id);
>         addrlen = ucma_addrlen(addr);
>         if (!addrlen)
>                 return -EINVAL;
> 
> -       CMA_CREATE_MSG_CMD(msg, cmd, UCMA_CMD_JOIN_MCAST, size);
> -       id_priv = container_of(id, struct cma_id_private, id);
> +       mc = malloc(sizeof *mc);
> +       if (!mc)
> +               return -ENOMEM;
> +
> +       memset(mc, 0, sizeof *mc);
> +       mc->context = context;
> +       mc->id_priv = id_priv;
> +       memcpy(&mc->addr, addr, addrlen);
> +       if (pthread_cond_init(&id_priv->cond, NULL)) {
> +               ret = -1;
> +               goto err1;
> +       }
> +
> +       pthread_mutex_lock(&id_priv->mut);
> +       mc->next = id_priv->mc_list;
> +       id_priv->mc_list = mc;
> +       pthread_mutex_unlock(&id_priv->mut);
> +
> +       CMA_CREATE_MSG_CMD_RESP(msg, cmd, resp, UCMA_CMD_JOIN_MCAST, size);
>         cmd->id = id_priv->handle;
>         memcpy(&cmd->addr, addr, addrlen);
> -       cmd->uid = (uintptr_t) context;
> +       cmd->uid = (uintptr_t) mc;
> 
>         ret = write(id->channel->fd, msg, size);
> -       if (ret != size)
> -               return (ret > 0) ? -ENODATA : ret;
> +       if (ret != size) {
> +               ret = (ret > 0) ? -ENODATA : ret;
> +               goto err2;
> +       }
> 
> +       mc->handle = resp->id;
>         return 0;
> +err2:
> +       pthread_mutex_lock(&id_priv->mut);
> +       for (pos = &id_priv->mc_list; *pos != mc; pos = &(*pos)->next)
> +               ;
> +       *pos = mc->next;
> +       pthread_mutex_unlock(&id_priv->mut);
> +err1:
> +       free(mc);
> +       return ret;
>  }
> 
>  int rdma_leave_multicast(struct rdma_cm_id *id, struct sockaddr *addr)
>  {
> -       struct ucma_abi_leave_mcast *cmd;
> +       struct ucma_abi_destroy_id *cmd;
> +       struct ucma_abi_destroy_id_resp *resp;
>         struct cma_id_private *id_priv;
> +       struct cma_multicast *mc, **pos;
>         void *msg;
>         int ret, size, addrlen;
> -       struct ibv_ah_attr ah_attr;
> -       uint32_t qp_info;
>        
>         addrlen = ucma_addrlen(addr);
>         if (!addrlen)
>                 return -EINVAL;
> 
> -       CMA_CREATE_MSG_CMD(msg, cmd, UCMA_CMD_LEAVE_MCAST, size);
>         id_priv = container_of(id, struct cma_id_private, id);
> -       cmd->id = id_priv->handle;
> -       memcpy(&cmd->addr, addr, addrlen);
> +       pthread_mutex_lock(&id_priv->mut);
> +       for (pos = &id_priv->mc_list; *pos; pos = &(*pos)->next)
> +               if (!memcmp(&(*pos)->addr, addr, addrlen))
> +                       break;
> 
> -       if (id->qp) {
> -               ret = rdma_get_dst_attr(id, addr, &ah_attr, &qp_info, 
> &qp_info);
> -               if (ret)
> -                       goto out;
> +       mc = *pos;
> +       if (*pos)
> +               *pos = mc->next;
> +       pthread_mutex_unlock(&id_priv->mut);
> +       if (!mc)
> +               return -EADDRNOTAVAIL;
> 
> -               ret = ibv_detach_mcast(id->qp, &ah_attr.grh.dgid, 
> ah_attr.dlid);
> -               if (ret)
> -                       goto out;
> -       }
> +       if (id->qp)
> +               ibv_detach_mcast(id->qp, &mc->mgid, mc->mlid);
>        
> +       CMA_CREATE_MSG_CMD_RESP(msg, cmd, resp, UCMA_CMD_LEAVE_MCAST, size);
> +       cmd->id = mc->handle;
> +
>         ret = write(id->channel->fd, msg, size);
>         if (ret != size)
>                 ret = (ret > 0) ? -ENODATA : ret;
> -out:
> -       return ret;
> -}
> 
> -static void ucma_copy_event_from_kern(struct rdma_cm_event *dst,
> -                                     struct ucma_abi_event_resp *src)
> -{
> -       dst->event = src->event;
> -       dst->status = src->status;
> -       dst->private_data_len = src->private_data_len;
> -       if (src->private_data_len) {
> -               dst->private_data = dst + 1;
> -               memcpy(dst->private_data, src->private_data,
> -                      src->private_data_len);
> -       } else
> -               dst->private_data = NULL;
> +       pthread_mutex_lock(&id_priv->mut);
> +       while (mc->events_completed < resp->events_reported)
> +               pthread_cond_wait(&mc->cond, &id_priv->mut);
> +       pthread_mutex_unlock(&id_priv->mut);
> +
> +       free(mc);
> +       return ret;
>  }

Sean,

I just noticed that once i apply the patch, the last + lines (that is 
pthread_mutex_lock, while loop doing pthread_cond_wait and then 
pthread_mutex_unlock) become part of rdma_leave_multicast which seems to 
me strictly buggy as no one is going to wake up this code.

Looking in your librdmacm git on openfabrics @ 
http://www2.openfabrics.org/git/?p=~shefty/librdmacm.git;a=blob;f=src/cma.c
this indeed seems to be code of librdmacm/src/cma.c

Can you clarify that?

Or.