[openib-general] [PATCH 1/1] IWCM cm_work_handler improvements

[Tom Tucker]

I remember now why the work was removed from the list early. If the
connection is going away, the loop exits early and you won't delete the
work item and you'll end up leaking the work queue structure. 

I know what to do, but I don't have time to do it properly because I've
got to get on a plane for SC'06. Please ignore this for now and sorry
for the patch noise.

Tom

On Fri, 2006-11-10 at 13:33 -0600, Tom Tucker wrote:
> Move the removal of the work queue element to the 
> end of the processing loop. This avoids the race with 
> the cm_event_handler and obviates the need for the local
> copy of the work structure.
> 
> I'll be testing this concurrently, but here's the patch for review...
> 
> Signed-off-by: Tom Tucker <tom at opengridcomputing.com>
> 
> ---
> 
>  drivers/infiniband/core/iwcm.c |   12 ++++--------
>  1 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/infiniband/core/iwcm.c b/drivers/infiniband/core/iwcm.c
> index c3fb304..3a14355 100644
> --- a/drivers/infiniband/core/iwcm.c
> +++ b/drivers/infiniband/core/iwcm.c
> @@ -830,21 +830,15 @@ static int process_event(struct iwcm_id_
>   */
>  static void cm_work_handler(void *arg)
>  {
> -	struct iwcm_work *work = arg, lwork;
> +	struct iwcm_work *work = arg;
>  	struct iwcm_id_private *cm_id_priv = work->cm_id;
>  	unsigned long flags;
> -	int empty;
>  	int ret = 0;
>  
>  	spin_lock_irqsave(&cm_id_priv->lock, flags);
> -	empty = list_empty(&cm_id_priv->work_list);
> -	while (!empty) {
> +	while (!list_empty(&cm_id_priv->work_list)) {
>  		work = list_entry(cm_id_priv->work_list.next,
>  				  struct iwcm_work, list);
> -		list_del_init(&work->list);
> -		empty = list_empty(&cm_id_priv->work_list);
> -		lwork = *work;
> -		put_work(work);
>  		spin_unlock_irqrestore(&cm_id_priv->lock, flags);
>  
>  		ret = process_event(cm_id_priv, &work->event);
> @@ -863,6 +857,8 @@ static void cm_work_handler(void *arg)
>  			return;
>  		}
>  		spin_lock_irqsave(&cm_id_priv->lock, flags);
> +		list_del_init(&work->list);
> +		put_work(work);
>  	}
>  	spin_unlock_irqrestore(&cm_id_priv->lock, flags);
>  }
> 
> 
> _______________________________________________
> openib-general mailing list
> openib-general at openib.org
> http://openib.org/mailman/listinfo/openib-general
> 
> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
>