Re: [PATCH for-4.1 10/11] RDMA/ocrdma: Prevent returning failures in the resource destroy path

[Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 3613 bytes --]

On Sat, 2015-05-16 at 13:21 +0530, Selvin Xavier wrote:
> Avoid returning FW failures in the resource destroy verbs.

This patch looks extremely unsafe to someone that doesn't have the
benefit of being able to look at your firmware code.  What firmware
errors are you ignoring?  What state is the card in whenever these
errors are returned?  Is there even a remote possibility that the
firmware being in this state and returning these errors will either A)
cause erroneous attempts to write to user memory or B) interfere with
future attempts to create PDs or other contexts on the card?

> Signed-off-by: Padmanabh Ratnakar <padmanabh.ratnakar-1wcpHE2jlwO1Z/+hSey0Gg@public.gmane.org>
> Signed-off-by: Devesh Sharma <devesh.sharma-1wcpHE2jlwO1Z/+hSey0Gg@public.gmane.org>
> Signed-off-by: Selvin Xavier <selvin.xavier-1wcpHE2jlwO1Z/+hSey0Gg@public.gmane.org>
> ---
>  drivers/infiniband/hw/ocrdma/ocrdma_verbs.c | 17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
> index 48ec56f..b1f9d7d 100644
> --- a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
> +++ b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
> @@ -531,13 +531,12 @@ map_err:
>  
>  int ocrdma_dealloc_ucontext(struct ib_ucontext *ibctx)
>  {
> -	int status = 0;
>  	struct ocrdma_mm *mm, *tmp;
>  	struct ocrdma_ucontext *uctx = get_ocrdma_ucontext(ibctx);
>  	struct ocrdma_dev *dev = get_ocrdma_dev(ibctx->device);
>  	struct pci_dev *pdev = dev->nic_info.pdev;
>  
> -	status = ocrdma_dealloc_ucontext_pd(uctx);
> +	(void)ocrdma_dealloc_ucontext_pd(uctx);
>  
>  	ocrdma_del_mmap(uctx, uctx->ah_tbl.pa, uctx->ah_tbl.len);
>  	dma_free_coherent(&pdev->dev, uctx->ah_tbl.len, uctx->ah_tbl.va,
> @@ -548,7 +547,7 @@ int ocrdma_dealloc_ucontext(struct ib_ucontext *ibctx)
>  		kfree(mm);
>  	}
>  	kfree(uctx);
> -	return status;
> +	return 0;
>  }
>  
>  int ocrdma_mmap(struct ib_ucontext *context, struct vm_area_struct *vma)
> @@ -690,7 +689,6 @@ int ocrdma_dealloc_pd(struct ib_pd *ibpd)
>  	struct ocrdma_pd *pd = get_ocrdma_pd(ibpd);
>  	struct ocrdma_dev *dev = get_ocrdma_dev(ibpd->device);
>  	struct ocrdma_ucontext *uctx = NULL;
> -	int status = 0;
>  	u64 usr_db;
>  
>  	uctx = pd->uctx;
> @@ -704,11 +702,11 @@ int ocrdma_dealloc_pd(struct ib_pd *ibpd)
>  
>  		if (is_ucontext_pd(uctx, pd)) {
>  			ocrdma_release_ucontext_pd(uctx);
> -			return status;
> +			return 0;
>  		}
>  	}
> -	status = _ocrdma_dealloc_pd(dev, pd);
> -	return status;
> +	(void)_ocrdma_dealloc_pd(dev, pd);
> +	return 0;
>  }
>  
>  static int ocrdma_alloc_lkey(struct ocrdma_dev *dev, struct ocrdma_mr *mr,
> @@ -1905,13 +1903,12 @@ int ocrdma_query_srq(struct ib_srq *ibsrq, struct ib_srq_attr *srq_attr)
>  
>  int ocrdma_destroy_srq(struct ib_srq *ibsrq)
>  {
> -	int status;
>  	struct ocrdma_srq *srq;
>  	struct ocrdma_dev *dev = get_ocrdma_dev(ibsrq->device);
>  
>  	srq = get_ocrdma_srq(ibsrq);
>  
> -	status = ocrdma_mbx_destroy_srq(dev, srq);
> +	(void)ocrdma_mbx_destroy_srq(dev, srq);
>  
>  	if (srq->pd->uctx)
>  		ocrdma_del_mmap(srq->pd->uctx, (u64) srq->rq.pa,
> @@ -1920,7 +1917,7 @@ int ocrdma_destroy_srq(struct ib_srq *ibsrq)
>  	kfree(srq->idx_bit_fields);
>  	kfree(srq->rqe_wr_id_tbl);
>  	kfree(srq);
> -	return status;
> +	return 0;
>  }
>  
>  /* unprivileged verbs and their support functions. */


-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]