[openib-general] [PATCH] Extra kfrees, clean up unregisters, etc ...

[Krishna Kumar]

Yes, looks good.

- KK


On Fri, 5 Nov 2004, Roland Dreier wrote:

> Thanks for the audit.  I applied this version of your patch.  Does
> this still look correct?
>
> Index: infiniband/core/sa_query.c
> ===================================================================
> --- infiniband/core/sa_query.c	(revision 1164)
> +++ infiniband/core/sa_query.c	(working copy)
> @@ -699,29 +710,28 @@
>  	sa_dev->start_port = s;
>  	sa_dev->end_port   = e;
>
> -	for (i = s; i <= e; ++i) {
> -		sa_dev->port[i - s].mr       = NULL;
> -		sa_dev->port[i - s].sm_ah    = NULL;
> -		sa_dev->port[i - s].port_num = i;
> -		spin_lock_init(&sa_dev->port[i - s].ah_lock);
> +	for (i = 0; i <= e - s; ++i) {
> +		sa_dev->port[i].mr       = NULL;
> +		sa_dev->port[i].sm_ah    = NULL;
> +		sa_dev->port[i].port_num = i + s;
> +		spin_lock_init(&sa_dev->port[i].ah_lock);
>
> -		sa_dev->port[i - s].agent =
> -			ib_register_mad_agent(device, i, IB_QPT_GSI,
> +		sa_dev->port[i].agent =
> +			ib_register_mad_agent(device, i + s, IB_QPT_GSI,
>  					      NULL, 0, send_handler,
>  					      recv_handler, sa_dev);
> -		if (IS_ERR(sa_dev->port[i - s].agent))
> +		if (IS_ERR(sa_dev->port[i].agent))
>  			goto err;
>
> -		sa_dev->port[i - s].mr = ib_get_dma_mr(sa_dev->port[i - s].agent->qp->pd,
> -						       IB_ACCESS_LOCAL_WRITE);
> -		if (IS_ERR(sa_dev->port[i - s].mr)) {
> -			/* Bump i so agent from this iter. is freed */
> -			++i;
> +		sa_dev->port[i].mr = ib_get_dma_mr(sa_dev->port[i].agent->qp->pd,
> +						   IB_ACCESS_LOCAL_WRITE);
> +		if (IS_ERR(sa_dev->port[i].mr)) {
> +			ib_unregister_mad_agent(sa_dev->port[i].agent);
>  			goto err;
>  		}
>
> -		INIT_WORK(&sa_dev->port[i - s].update_task,
> -			  update_sm_ah, &sa_dev->port[i - s]);
> +		INIT_WORK(&sa_dev->port[i].update_task,
> +			  update_sm_ah, &sa_dev->port[i]);
>  	}
>
>  	/*
> @@ -732,27 +742,20 @@
>  	 */
>
>  	INIT_IB_EVENT_HANDLER(&sa_dev->event_handler, device, ib_sa_event);
> -	if (ib_register_event_handler(&sa_dev->event_handler)) {
> -		kfree(sa_dev);
> +	if (ib_register_event_handler(&sa_dev->event_handler))
>  		goto err;
> -	}
>
> -	for (i = s; i <= e; ++i)
> -		update_sm_ah(&sa_dev->port[i - s]);
> +	for (i = 0; i <= e - s; ++i)
> +		update_sm_ah(&sa_dev->port[i]);
>
>  	ib_set_client_data(device, &sa_client, sa_dev);
>
>  	return;
>
>  err:
> -	while (--i >= s) {
> -		if (sa_dev->port[i - s].mr && !IS_ERR(sa_dev->port[i - s].mr))
> -			ib_dereg_mr(sa_dev->port[i - s].mr);
> -
> -		if (sa_dev->port[i - s].sm_ah)
> -			kref_put(&sa_dev->port[i].sm_ah->ref, free_sm_ah);
> -
> -		ib_unregister_mad_agent(sa_dev->port[i - s].agent);
> +	while (--i >= 0) {
> +		ib_dereg_mr(sa_dev->port[i].mr);
> +		ib_unregister_mad_agent(sa_dev->port[i].agent);
>  	}
>
>  	kfree(sa_dev);
>
>