RE: [PATCH for-next 2/5] RDMA/rxe: Fix misleading comments and names

["Pearson, Robert B" <robert.pearson2@hpe.com>]

-----Original Message-----
From: Jason Gunthorpe <jgg@nvidia.com> 
Sent: Friday, January 22, 2021 1:27 PM
To: Bob Pearson <rpearsonhpe@gmail.com>
Cc: zyjzyj2000@gmail.com; linux-rdma@vger.kernel.org; Pearson, Robert B <robert.pearson2@hpe.com>; Hillf Danton <hdanton@sina.com>
Subject: Re: [PATCH for-next 2/5] RDMA/rxe: Fix misleading comments and names

On Thu, Jan 21, 2021 at 10:23:10PM -0600, Bob Pearson wrote:
> The names and comments of the 'unlocked' pool APIs are very misleading 
> and not what was intended. This patch replaces 'rxe_xxx_nl' with 
> 'rxe_xxx__' with comments indicating that the caller is expected to 
> hold the rxe pool lock.
> 
> Reported-by: Hillf Danton <hdanton@sina.com>
> Signed-off-by: Bob Pearson <rpearson@hpe.com>
> ---
>  drivers/infiniband/sw/rxe/rxe_mcast.c |  8 ++--  
> drivers/infiniband/sw/rxe/rxe_pool.c  | 22 +++++------  
> drivers/infiniband/sw/rxe/rxe_pool.h  | 55 +++++++++++++--------------
>  3 files changed, 42 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_mcast.c 
> b/drivers/infiniband/sw/rxe/rxe_mcast.c
> index 5be47ce7d319..b9f06f87866e 100644
> --- a/drivers/infiniband/sw/rxe/rxe_mcast.c
> +++ b/drivers/infiniband/sw/rxe/rxe_mcast.c
> @@ -15,18 +15,18 @@ static struct rxe_mc_grp *create_grp(struct rxe_dev *rxe,
>  	int err;
>  	struct rxe_mc_grp *grp;
>  
> -	grp = rxe_alloc_nl(&rxe->mc_grp_pool);
> +	grp = rxe_alloc__(&rxe->mc_grp_pool);

Everything else seems fine, but this trailing __ is too weird

If a lock is supposed to be held then name it foo_locked() or locked_()

If it auto-locks then name it foo()

If there is some #define wrapper then it is 

#define foo() __foo()

Jason


I have both in some cases. A #define and a locked version in some cases. I'll try xxx_locked().
To be clear that means caller should hold some lock, yes?

bob