| Message ID | 20220705114603.6768-2-yangx.jy@fujitsu.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] RDMA/rxe: Add common rxe_prepare_res() | expand |
Generally over time I have been adding a rxe_ prefix to all searchable names static or non static. This avoids collisions with similar names in other drivers with e.g. ctags. I agree a unified naming Scheme is good but would like to see one with a common prefix for subroutine names. Bob -----Original Message----- From: Xiao Yang <yangx.jy@fujitsu.com> Sent: Tuesday, July 5, 2022 6:46 AM To: linux-rdma@vger.kernel.org Cc: leon@kernel.org; jgg@ziepe.ca; rpearsonhpe@gmail.com; zyjzyj2000@gmail.com; Xiao Yang <yangx.jy@fujitsu.com> Subject: [PATCH 2/2] RDMA/rxe: Rename rxe_atomic_reply to atomic_reply It's better to use the unified naming format. Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com> --- drivers/infiniband/sw/rxe/rxe_resp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c index 5536582b8fe4..265e46fe050f 100644 --- a/drivers/infiniband/sw/rxe/rxe_resp.c +++ b/drivers/infiniband/sw/rxe/rxe_resp.c @@ -595,7 +595,7 @@ static struct resp_res *rxe_prepare_res(struct rxe_qp *qp, /* Guarantee atomicity of atomic operations at the machine level. */ static DEFINE_SPINLOCK(atomic_ops_lock); -static enum resp_states rxe_atomic_reply(struct rxe_qp *qp, +static enum resp_states atomic_reply(struct rxe_qp *qp, struct rxe_pkt_info *pkt) { u64 *vaddr; @@ -1333,7 +1333,7 @@ int rxe_responder(void *arg) state = read_reply(qp, pkt); break; case RESPST_ATOMIC_REPLY: - state = rxe_atomic_reply(qp, pkt); + state = atomic_reply(qp, pkt); break; case RESPST_ACKNOWLEDGE: state = acknowledge(qp, pkt); -- 2.34.1
On 2022/7/6 15:38, Pearson, Robert B wrote: > Generally over time I have been adding a rxe_ prefix to all searchable names static or non static. > This avoids collisions with similar names in other drivers with e.g. ctags. I agree a unified naming > Scheme is good but would like to see one with a common prefix for subroutine names. Hi Bob, I think it's hard to use unique name in all drivers. I saw that all functions called by different qp states don't use the rxe_ prefix in rxe_responders so just remove the rxe_ prefix. We can drop the patch if you don't agree with this change. Best Regards, Xiao Yang > > Bob > > -----Original Message----- > From: Xiao Yang <yangx.jy@fujitsu.com> > Sent: Tuesday, July 5, 2022 6:46 AM > To: linux-rdma@vger.kernel.org > Cc: leon@kernel.org; jgg@ziepe.ca; rpearsonhpe@gmail.com; zyjzyj2000@gmail.com; Xiao Yang <yangx.jy@fujitsu.com> > Subject: [PATCH 2/2] RDMA/rxe: Rename rxe_atomic_reply to atomic_reply > > It's better to use the unified naming format. > > Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com> > --- > drivers/infiniband/sw/rxe/rxe_resp.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c > index 5536582b8fe4..265e46fe050f 100644 > --- a/drivers/infiniband/sw/rxe/rxe_resp.c > +++ b/drivers/infiniband/sw/rxe/rxe_resp.c > @@ -595,7 +595,7 @@ static struct resp_res *rxe_prepare_res(struct rxe_qp *qp, > /* Guarantee atomicity of atomic operations at the machine level. */ static DEFINE_SPINLOCK(atomic_ops_lock); > > -static enum resp_states rxe_atomic_reply(struct rxe_qp *qp, > +static enum resp_states atomic_reply(struct rxe_qp *qp, > struct rxe_pkt_info *pkt) > { > u64 *vaddr; > @@ -1333,7 +1333,7 @@ int rxe_responder(void *arg) > state = read_reply(qp, pkt); > break; > case RESPST_ATOMIC_REPLY: > - state = rxe_atomic_reply(qp, pkt); > + state = atomic_reply(qp, pkt); > break; > case RESPST_ACKNOWLEDGE: > state = acknowledge(qp, pkt); > -- > 2.34.1 > > >
On 7/6/22 07:14, yangx.jy@fujitsu.com wrote: > On 2022/7/6 15:38, Pearson, Robert B wrote: >> Generally over time I have been adding a rxe_ prefix to all searchable names static or non static. >> This avoids collisions with similar names in other drivers with e.g. ctags. I agree a unified naming >> Scheme is good but would like to see one with a common prefix for subroutine names. > Hi Bob, > > I think it's hard to use unique name in all drivers. I saw that all > functions called by different qp states don't use the rxe_ prefix in > rxe_responders so just remove the rxe_ prefix. We can drop the patch if > you don't agree with this change. > > Best Regards, > Xiao Yang I fought on your side of this argument with Zhu a while back but now I am completely on his side. Linux has a lot of lines and generic subroutine names will very often collide all over the place. The better answer is to add rxe_ not remove it. It's not super critical so I have been doing it as a background task whenever I touch a piece of code. If you get the urge to make things more consistent feel free to do that but by adding a prefix and not deleting it. Bob
diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c index 5536582b8fe4..265e46fe050f 100644 --- a/drivers/infiniband/sw/rxe/rxe_resp.c +++ b/drivers/infiniband/sw/rxe/rxe_resp.c @@ -595,7 +595,7 @@ static struct resp_res *rxe_prepare_res(struct rxe_qp *qp, /* Guarantee atomicity of atomic operations at the machine level. */ static DEFINE_SPINLOCK(atomic_ops_lock); -static enum resp_states rxe_atomic_reply(struct rxe_qp *qp, +static enum resp_states atomic_reply(struct rxe_qp *qp, struct rxe_pkt_info *pkt) { u64 *vaddr; @@ -1333,7 +1333,7 @@ int rxe_responder(void *arg) state = read_reply(qp, pkt); break; case RESPST_ATOMIC_REPLY: - state = rxe_atomic_reply(qp, pkt); + state = atomic_reply(qp, pkt); break; case RESPST_ACKNOWLEDGE: state = acknowledge(qp, pkt);
It's better to use the unified naming format. Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com> --- drivers/infiniband/sw/rxe/rxe_resp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)