-----Original Message----- From: Jason Gunthorpe <jgg@xxxxxxxxxx> Sent: Friday, January 22, 2021 1:27 PM To: Bob Pearson <rpearsonhpe@xxxxxxxxx> Cc: zyjzyj2000@xxxxxxxxx; linux-rdma@xxxxxxxxxxxxxxx; Pearson, Robert B <robert.pearson2@xxxxxxx>; Hillf Danton <hdanton@xxxxxxxx> 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@xxxxxxxx> > Signed-off-by: Bob Pearson <rpearson@xxxxxxx> > --- > 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
