Re: [PATCH for-rc v6] RDMA/rxe: Fix pd ref counting in rxe mr verbs.

[Leon Romanovsky <leon@kernel.org>]

On Thu, Sep 01, 2022 at 03:04:27PM -0500, Bob Pearson wrote:
> Move referencing pd in mr objects ahead of any possible errors
> so that it will always be set in rxe_mr_complete() to avoid
> seg faults when rxe_put(mr_pd(mr)) is called. Adjust the reference
> counts so that each call to rxe_mr_init_xxx() takes one reference.
> This reference count is dropped in rxe_mr_cleanup() in error paths
> in the reg mr verbs and the dereg mr verb. Minor white space cleanups.
> 
> These errors have been seen in rxe_mr_init_user() when there isn't
> enough memory to create the mr maps. Previously the error return
> path didn't reach setting ibpd in mr->ibmr which caused a seg fault.
> 
> Fixes: 364e282c4fe7e ("RDMA/rxe: Split MEM into MR and MW")
> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
> ---
> v6:
>   Separated from other patch in original series and resubmitted
>   rebased to current for-rc.
>   Renamed from "RDMA/rxe: Set pd early in mr alloc routines" to
>   "RDMA/rxe: Fix pd ref counting in rxe mr verbs"
>   Added more text to describe the change.
>   Added fixes line.
>   Simplified the patch by leaving pd code in rxe_mr.c instead of
>   moving it up to rxe_verbs.c
> v5:
>   Dropped cleanup code from patch per Li Zhijian.
>   Split up into two small patches.
> v4:
>   Added set mr->ibmr.pd back to avoid an -ENOMEM error causing
>   rxe_put(mr_pd(mr)); to seg fault in rxe_mr_cleanup() since pd
>   is not getting set in the error path.
> v3:
>   Rebased to apply to current for-next after
>   	Revert "RDMA/rxe: Create duplicate mapping tables for FMRs"
> v2:
>   Moved setting mr->umem until after checks to avoid sending
>   an ERR_PTR to ib_umem_release().
>   Cleaned up umem and map sets if errors occur in alloc mr calls.
>   Rebased to current for-next.
> ---
>  drivers/infiniband/sw/rxe/rxe_mr.c    | 24 ++++++++++++++----------
>  drivers/infiniband/sw/rxe/rxe_verbs.c | 27 +++++++--------------------
>  2 files changed, 21 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
> index 850b80f5ad8b..5f4daffccb40 100644
> --- a/drivers/infiniband/sw/rxe/rxe_mr.c
> +++ b/drivers/infiniband/sw/rxe/rxe_mr.c
> @@ -107,7 +107,9 @@ void rxe_mr_init_dma(struct rxe_pd *pd, int access, struct rxe_mr *mr)
>  {
>  	rxe_mr_init(access, mr);
>  
> +	rxe_get(pd);

rxe_get() can fail, why don't you check for failure here and in all
places?

Thanks