Re: [PATCH for-next] RDMA/rxe: rxe_get_av always receives ahp hence no put is needed

[Bob Pearson <rpearsonhpe@gmail.com>]

On 10/20/22 10:13, Md Haris Iqbal wrote:
> The function rxe_get_av is only used by rxe_requester, and the ahp double
> pointer is always sent. Hence there is no need to do the check.
> rxe_requester also always performs the put for ah, hence that is also not
> needed.
> 
> Signed-off-by: Md Haris Iqbal <haris.phnx@gmail.com>
> ---
>  drivers/infiniband/sw/rxe/rxe_av.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_av.c b/drivers/infiniband/sw/rxe/rxe_av.c
> index 3b05314ca739..0780ffcf24a6 100644
> --- a/drivers/infiniband/sw/rxe/rxe_av.c
> +++ b/drivers/infiniband/sw/rxe/rxe_av.c
> @@ -130,11 +130,7 @@ struct rxe_av *rxe_get_av(struct rxe_pkt_info *pkt, struct rxe_ah **ahp)
>  			rxe_put(ah);
>  			return NULL;
>  		}
> -
> -		if (ahp)
> -			*ahp = ah;
> -		else
> -			rxe_put(ah);
> +		*ahp = ah;
>  
>  		return &ah->av;
>  	}

That doesn't sound right. There are several cases depending on the version of the user library
and whether the QP is UD or RC/UC. The old driver/library computed the address vector in
user space and passed it back to the kernel in the WR. If both the kernel and user library are using
the new API the user space passes back the AH# in the WR for UD commands. In both cases for
connected QPs the AV is stored in the rxe_struct_qp and there is no AH. At the point where
rxe_get_av is called the requester needs the AV it gets it from one of the three places:
the QP, the WR (old), or the kernel AH after looking up the AH#. If the kernel AH was involved
its pointer is returned so the requester can continue to hold a reference to it until it
is through sending the packet and then it can drop the reference. This to protect from
someone calling destroy_ah in a race with the send queue. Hope this makes it clearer.

Bob