[openib-general] [PATCH] Optimize cma_process_remove()

[Sean Hefty]

Krishna Kumar wrote:
>  static void cma_process_remove(struct cma_device *cma_dev)
>  {
>  	struct list_head remove_list;
> -	struct rdma_id_private *id_priv;
> +	struct rdma_id_private *id_priv, *tmp;
>  	int ret;
>  
>  	INIT_LIST_HEAD(&remove_list);
> @@ -2344,22 +2344,20 @@ static void cma_process_remove(struct cm
>  
>  		if (cma_internal_listen(id_priv)) {
>  			cma_destroy_listen(id_priv);
> -			continue;
> +		} else {
> +			list_del(&id_priv->list);
> +			list_add_tail(&id_priv->list, &remove_list);
>  		}
> +	}
> +	mutex_unlock(&lock);
>  
> -		list_del(&id_priv->list);
> -		list_add_tail(&id_priv->list, &remove_list);
> +	list_for_each_entry_safe(id_priv, tmp, &remove_list, list) {
>  		atomic_inc(&id_priv->refcount);
> -		mutex_unlock(&lock);
> -

I don't think that this will work.  The issue is that we need to walk a list of 
IDs associated with a particular device to notify the user that the device is 
being removed.  While we're doing that, the user could try to destroy the ID, 
which removes the ID from the device list.

The original code takes a reference on the ID before removing it from the from 
cma_dev's list to ensure that the ID will be valid while we process it.  The 
remove list ensures that the user is only notified once of a device removal. 
(We don't know where the thread calling rdma_destroy_id() is at.)

We can eliminate the remove_list by calling list_del_init().

- Sean