[ofa-general] Re: [RFC v2 PATCH 4/5] rdma/cma: implement RDMA_ALIGN_WITH_NETDEVICE ha mode

[Or Gerlitz]

Sean Hefty wrote:
>> +static int cma_netdev_align_id(struct net_device *ndev, struct rdma_id_private
>> *id_priv)
>> +{
>> +	struct rdma_dev_addr *dev_addr;
>> +	struct cma_work *work;
>> +
>> +	dev_addr = &id_priv->id.route.addr.dev_addr;
>> +
>> +	if (!memcmp(dev_addr->src_dev_name, ndev->name, IFNAMSIZ) &&
>> +	  memcmp(dev_addr->src_dev_addr, ndev->dev_addr, ndev->addr_len)) {
>> +		printk(KERN_ERR "addr change for device %s used by id %p,
>> notifying\n",
>> +				ndev->name, &id_priv->id);
>> +		work = kzalloc(sizeof *work, GFP_KERNEL);
>> +		if (!work)
>> +			return -ENOMEM;
>> +		work->id = id_priv;
>> +		INIT_WORK(&work->work, cma_work_handler);
>> +		work->old_state = id_priv->state;
>> +		work->new_state = id_priv->state;
>> +		work->event.event = RDMA_CM_EVENT_NETDEV_CHANGE;
>> +		atomic_inc(&id_priv->refcount);
>> +		queue_work(cma_wq, &work->work);
>> +	}
>> +}
>
> My initial thought on this is to see if we can just queue a single work item
> that can be used to invoke the user callbacks.  I'd have to see how the locking
> worked out though to know if that approach is 'cleaner'.
Sean,

Yes, it is possible to queue a single work item that can be used to 
invoke the user callbacks, eg cma_netdev_change_handler() would be 
queued to be executed by a thread (eg cma_wq mentioned below)  and do 
what this code does. What makes you think that its 'cleaner' to do it 
this way?
> Currently, the rdma_cm ensures that only a single callback to the user is
> invoked at a time.  This is needed to support the user trying to destroy their
> rdma_cm_id from the callback.  I didn't look to see if this still maintains
> that.
OK, so I understand from the code that the callback to the user may be 
delivered not only through cma_work_handler (that is in the context of 
the work queue thread that is created by the rdmn-cm). So is the design 
keeps on serialization through tracking the ID state/changes before 
invoking the callback, or its a different method?

Or.