Most code in send_ack() and send_atomic_ack() are duplicate, move them
to a new helper send_common_ack().
In newer IBA SPEC, some opcodes require acknowledge with a zero-length read
response, with this new helper, we can easily implement it later.
Signed-off-by: Li Zhijian <[email protected]>
---
drivers/infiniband/sw/rxe/rxe_resp.c | 43 ++++++++++++++----------------------
1 file changed, 17 insertions(+), 26 deletions(-)
diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
index b36ec5c4d5e0..4c398fa220fa 100644
--- a/drivers/infiniband/sw/rxe/rxe_resp.c
+++ b/drivers/infiniband/sw/rxe/rxe_resp.c
@@ -1028,50 +1028,41 @@ static enum resp_states do_complete(struct rxe_qp *qp,
return RESPST_CLEANUP;
}
-static int send_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
+
+static int send_common_ack(struct rxe_qp *qp, u8 syndrome, u32 psn,
+ int opcode, const char *msg)
{
- int err = 0;
+ int err;
struct rxe_pkt_info ack_pkt;
struct sk_buff *skb;
- skb = prepare_ack_packet(qp, &ack_pkt, IB_OPCODE_RC_ACKNOWLEDGE,
- 0, psn, syndrome);
- if (!skb) {
- err = -ENOMEM;
- goto err1;
- }
+ skb = prepare_ack_packet(qp, &ack_pkt, opcode, 0, psn, syndrome);
+ if (!skb)
+ return -ENOMEM;
err = rxe_xmit_packet(qp, &ack_pkt, skb);
if (err)
- pr_err_ratelimited("Failed sending ack\n");
+ pr_err_ratelimited("Failed sending %s\n", msg);
-err1:
return err;
}
-static int send_atomic_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
+static int send_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
{
- int err = 0;
- struct rxe_pkt_info ack_pkt;
- struct sk_buff *skb;
-
- skb = prepare_ack_packet(qp, &ack_pkt, IB_OPCODE_RC_ATOMIC_ACKNOWLEDGE,
- 0, psn, syndrome);
- if (!skb) {
- err = -ENOMEM;
- goto out;
- }
+ return send_common_ack(qp, syndrome, psn,
+ IB_OPCODE_RC_ACKNOWLEDGE, "ACK");
+}
- err = rxe_xmit_packet(qp, &ack_pkt, skb);
- if (err)
- pr_err_ratelimited("Failed sending atomic ack\n");
+static int send_atomic_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
+{
+ int ret = send_common_ack(qp, syndrome, psn,
+ IB_OPCODE_RC_ATOMIC_ACKNOWLEDGE, "ATOMIC ACK");
/* have to clear this since it is used to trigger
* long read replies
*/
qp->resp.res = NULL;
-out:
- return err;
+ return ret;
}
static enum resp_states acknowledge(struct rxe_qp *qp,
--
1.8.3.1
On Mon, Aug 1, 2022 at 2:16 PM Li Zhijian <[email protected]> wrote:
>
> Most code in send_ack() and send_atomic_ack() are duplicate, move them
> to a new helper send_common_ack().
>
> In newer IBA SPEC, some opcodes require acknowledge with a zero-length read
> response, with this new helper, we can easily implement it later.
>
> Signed-off-by: Li Zhijian <[email protected]>
> ---
> drivers/infiniband/sw/rxe/rxe_resp.c | 43 ++++++++++++++----------------------
> 1 file changed, 17 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
> index b36ec5c4d5e0..4c398fa220fa 100644
> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
> @@ -1028,50 +1028,41 @@ static enum resp_states do_complete(struct rxe_qp *qp,
> return RESPST_CLEANUP;
> }
>
> -static int send_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
> +
> +static int send_common_ack(struct rxe_qp *qp, u8 syndrome, u32 psn,
The function is better with rxe_send_common_ack?
So when debug, rxe_ prefix can help us.
> + int opcode, const char *msg)
> {
> - int err = 0;
> + int err;
> struct rxe_pkt_info ack_pkt;
> struct sk_buff *skb;
>
> - skb = prepare_ack_packet(qp, &ack_pkt, IB_OPCODE_RC_ACKNOWLEDGE,
> - 0, psn, syndrome);
> - if (!skb) {
> - err = -ENOMEM;
> - goto err1;
> - }
> + skb = prepare_ack_packet(qp, &ack_pkt, opcode, 0, psn, syndrome);
> + if (!skb)
> + return -ENOMEM;
>
> err = rxe_xmit_packet(qp, &ack_pkt, skb);
> if (err)
> - pr_err_ratelimited("Failed sending ack\n");
> + pr_err_ratelimited("Failed sending %s\n", msg);
>
> -err1:
> return err;
> }
>
> -static int send_atomic_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
> +static int send_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
rxe_send_ack
> {
> - int err = 0;
> - struct rxe_pkt_info ack_pkt;
> - struct sk_buff *skb;
> -
> - skb = prepare_ack_packet(qp, &ack_pkt, IB_OPCODE_RC_ATOMIC_ACKNOWLEDGE,
> - 0, psn, syndrome);
> - if (!skb) {
> - err = -ENOMEM;
> - goto out;
> - }
> + return send_common_ack(qp, syndrome, psn,
> + IB_OPCODE_RC_ACKNOWLEDGE, "ACK");
> +}
>
> - err = rxe_xmit_packet(qp, &ack_pkt, skb);
> - if (err)
> - pr_err_ratelimited("Failed sending atomic ack\n");
> +static int send_atomic_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
rxe_send_atomic_ack
Thanks and Regards,
Zhu Yanjun
> +{
> + int ret = send_common_ack(qp, syndrome, psn,
> + IB_OPCODE_RC_ATOMIC_ACKNOWLEDGE, "ATOMIC ACK");
>
> /* have to clear this since it is used to trigger
> * long read replies
> */
> qp->resp.res = NULL;
> -out:
> - return err;
> + return ret;
> }
>
> static enum resp_states acknowledge(struct rxe_qp *qp,
> --
> 1.8.3.1
>
On 01/08/2022 15:11, Zhu Yanjun wrote:
> On Mon, Aug 1, 2022 at 2:16 PM Li Zhijian <[email protected]> wrote:
>> Most code in send_ack() and send_atomic_ack() are duplicate, move them
>> to a new helper send_common_ack().
>>
>> In newer IBA SPEC, some opcodes require acknowledge with a zero-length read
>> response, with this new helper, we can easily implement it later.
>>
>> Signed-off-by: Li Zhijian <[email protected]>
>> ---
>> drivers/infiniband/sw/rxe/rxe_resp.c | 43 ++++++++++++++----------------------
>> 1 file changed, 17 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
>> index b36ec5c4d5e0..4c398fa220fa 100644
>> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
>> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
>> @@ -1028,50 +1028,41 @@ static enum resp_states do_complete(struct rxe_qp *qp,
>> return RESPST_CLEANUP;
>> }
>>
>> -static int send_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
>> +
>> +static int send_common_ack(struct rxe_qp *qp, u8 syndrome, u32 psn,
> The function is better with rxe_send_common_ack?
I'm not clear the exact rule about the naming prefix. if it has, please let me know :)
IMHO, if a function is either a public API(export function) or a callback to a upper layer, it's a good idea to a fixed prefix.
Instead, if they are just static, no prefix is not too bad.
BTW, current RXE are mixing the two rules, it should be another standalone patch to do the cleanup if needed.
Thanks
Zhijian
> So when debug, rxe_ prefix can help us.
>
>> + int opcode, const char *msg)
>> {
>> - int err = 0;
>> + int err;
>> struct rxe_pkt_info ack_pkt;
>> struct sk_buff *skb;
>>
>> - skb = prepare_ack_packet(qp, &ack_pkt, IB_OPCODE_RC_ACKNOWLEDGE,
>> - 0, psn, syndrome);
>> - if (!skb) {
>> - err = -ENOMEM;
>> - goto err1;
>> - }
>> + skb = prepare_ack_packet(qp, &ack_pkt, opcode, 0, psn, syndrome);
>> + if (!skb)
>> + return -ENOMEM;
>>
>> err = rxe_xmit_packet(qp, &ack_pkt, skb);
>> if (err)
>> - pr_err_ratelimited("Failed sending ack\n");
>> + pr_err_ratelimited("Failed sending %s\n", msg);
>>
>> -err1:
>> return err;
>> }
>>
>> -static int send_atomic_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
>> +static int send_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
> rxe_send_ack
>
>> {
>> - int err = 0;
>> - struct rxe_pkt_info ack_pkt;
>> - struct sk_buff *skb;
>> -
>> - skb = prepare_ack_packet(qp, &ack_pkt, IB_OPCODE_RC_ATOMIC_ACKNOWLEDGE,
>> - 0, psn, syndrome);
>> - if (!skb) {
>> - err = -ENOMEM;
>> - goto out;
>> - }
>> + return send_common_ack(qp, syndrome, psn,
>> + IB_OPCODE_RC_ACKNOWLEDGE, "ACK");
>> +}
>>
>> - err = rxe_xmit_packet(qp, &ack_pkt, skb);
>> - if (err)
>> - pr_err_ratelimited("Failed sending atomic ack\n");
>> +static int send_atomic_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
> rxe_send_atomic_ack
>
> Thanks and Regards,
> Zhu Yanjun
>> +{
>> + int ret = send_common_ack(qp, syndrome, psn,
>> + IB_OPCODE_RC_ATOMIC_ACKNOWLEDGE, "ATOMIC ACK");
>>
>> /* have to clear this since it is used to trigger
>> * long read replies
>> */
>> qp->resp.res = NULL;
>> -out:
>> - return err;
>> + return ret;
>> }
>>
>> static enum resp_states acknowledge(struct rxe_qp *qp,
>> --
>> 1.8.3.1
>>
On Mon, Aug 1, 2022 at 3:28 PM [email protected]
<[email protected]> wrote:
>
>
>
> On 01/08/2022 15:11, Zhu Yanjun wrote:
> > On Mon, Aug 1, 2022 at 2:16 PM Li Zhijian <[email protected]> wrote:
> >> Most code in send_ack() and send_atomic_ack() are duplicate, move them
> >> to a new helper send_common_ack().
> >>
> >> In newer IBA SPEC, some opcodes require acknowledge with a zero-length read
> >> response, with this new helper, we can easily implement it later.
> >>
> >> Signed-off-by: Li Zhijian <[email protected]>
> >> ---
> >> drivers/infiniband/sw/rxe/rxe_resp.c | 43 ++++++++++++++----------------------
> >> 1 file changed, 17 insertions(+), 26 deletions(-)
> >>
> >> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
> >> index b36ec5c4d5e0..4c398fa220fa 100644
> >> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
> >> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
> >> @@ -1028,50 +1028,41 @@ static enum resp_states do_complete(struct rxe_qp *qp,
> >> return RESPST_CLEANUP;
> >> }
> >>
> >> -static int send_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
> >> +
> >> +static int send_common_ack(struct rxe_qp *qp, u8 syndrome, u32 psn,
> > The function is better with rxe_send_common_ack?
> I'm not clear the exact rule about the naming prefix. if it has, please let me know :)
>
> IMHO, if a function is either a public API(export function) or a callback to a upper layer, it's a good idea to a fixed prefix.
> Instead, if they are just static, no prefix is not too bad.
When debug, a rxe_ prefix can help us filter the functions whatever
the function static or public.
>
> BTW, current RXE are mixing the two rules, it should be another standalone patch to do the cleanup if needed.
Yes. Please make this standalone patch to complete this.
Thanks and Regards,
Zhu Yanjun
>
> Thanks
> Zhijian
>
>
> > So when debug, rxe_ prefix can help us.
> >
> >> + int opcode, const char *msg)
> >> {
> >> - int err = 0;
> >> + int err;
> >> struct rxe_pkt_info ack_pkt;
> >> struct sk_buff *skb;
> >>
> >> - skb = prepare_ack_packet(qp, &ack_pkt, IB_OPCODE_RC_ACKNOWLEDGE,
> >> - 0, psn, syndrome);
> >> - if (!skb) {
> >> - err = -ENOMEM;
> >> - goto err1;
> >> - }
> >> + skb = prepare_ack_packet(qp, &ack_pkt, opcode, 0, psn, syndrome);
> >> + if (!skb)
> >> + return -ENOMEM;
> >>
> >> err = rxe_xmit_packet(qp, &ack_pkt, skb);
> >> if (err)
> >> - pr_err_ratelimited("Failed sending ack\n");
> >> + pr_err_ratelimited("Failed sending %s\n", msg);
> >>
> >> -err1:
> >> return err;
> >> }
> >>
> >> -static int send_atomic_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
> >> +static int send_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
> > rxe_send_ack
> >
> >> {
> >> - int err = 0;
> >> - struct rxe_pkt_info ack_pkt;
> >> - struct sk_buff *skb;
> >> -
> >> - skb = prepare_ack_packet(qp, &ack_pkt, IB_OPCODE_RC_ATOMIC_ACKNOWLEDGE,
> >> - 0, psn, syndrome);
> >> - if (!skb) {
> >> - err = -ENOMEM;
> >> - goto out;
> >> - }
> >> + return send_common_ack(qp, syndrome, psn,
> >> + IB_OPCODE_RC_ACKNOWLEDGE, "ACK");
> >> +}
> >>
> >> - err = rxe_xmit_packet(qp, &ack_pkt, skb);
> >> - if (err)
> >> - pr_err_ratelimited("Failed sending atomic ack\n");
> >> +static int send_atomic_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
> > rxe_send_atomic_ack
> >
> > Thanks and Regards,
> > Zhu Yanjun
> >> +{
> >> + int ret = send_common_ack(qp, syndrome, psn,
> >> + IB_OPCODE_RC_ATOMIC_ACKNOWLEDGE, "ATOMIC ACK");
> >>
> >> /* have to clear this since it is used to trigger
> >> * long read replies
> >> */
> >> qp->resp.res = NULL;
> >> -out:
> >> - return err;
> >> + return ret;
> >> }
> >>
> >> static enum resp_states acknowledge(struct rxe_qp *qp,
> >> --
> >> 1.8.3.1
> >>
On 01/08/2022 15:47, Zhu Yanjun wrote:
> On Mon, Aug 1, 2022 at 3:28 PM [email protected]
> <[email protected]> wrote:
>>
>>
>> On 01/08/2022 15:11, Zhu Yanjun wrote:
>>> On Mon, Aug 1, 2022 at 2:16 PM Li Zhijian <[email protected]> wrote:
>>>> Most code in send_ack() and send_atomic_ack() are duplicate, move them
>>>> to a new helper send_common_ack().
>>>>
>>>> In newer IBA SPEC, some opcodes require acknowledge with a zero-length read
>>>> response, with this new helper, we can easily implement it later.
>>>>
>>>> Signed-off-by: Li Zhijian <[email protected]>
>>>> ---
>>>> drivers/infiniband/sw/rxe/rxe_resp.c | 43 ++++++++++++++----------------------
>>>> 1 file changed, 17 insertions(+), 26 deletions(-)
>>>>
>>>> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
>>>> index b36ec5c4d5e0..4c398fa220fa 100644
>>>> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
>>>> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
>>>> @@ -1028,50 +1028,41 @@ static enum resp_states do_complete(struct rxe_qp *qp,
>>>> return RESPST_CLEANUP;
>>>> }
>>>>
>>>> -static int send_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
>>>> +
>>>> +static int send_common_ack(struct rxe_qp *qp, u8 syndrome, u32 psn,
>>> The function is better with rxe_send_common_ack?
>> I'm not clear the exact rule about the naming prefix. if it has, please let me know :)
>>
>> IMHO, if a function is either a public API(export function) or a callback to a upper layer, it's a good idea to a fixed prefix.
>> Instead, if they are just static, no prefix is not too bad.
> When debug, a rxe_ prefix can help us filter the functions whatever
> the function static or public.
>
>> BTW, current RXE are mixing the two rules, it should be another standalone patch to do the cleanup if needed.
> Yes. Please make this standalone patch to complete this.
i tried to do a rough statistic.
all functions:
$ git grep -E '^[a-z].*\(' drivers/infiniband/sw/rxe | awk -F'(' '{print $1}' | awk '{print $NF}' | tr -d '\*' | grep -E ^[a-z]+ | wc -l
474
without rxe_ prefix:
git grep -E '^[a-z].*\(' drivers/infiniband/sw/rxe | awk -F'(' '{print $1}' | awk '{print $NF}' | tr -d '\*' | grep -E ^[a-z]+ | grep -v -e ^rxe | wc -l
199
Similarly, the mlx5 have the same situations.
$ git grep -h -E '^[a-z].*\(' drivers/infiniband/hw/mlx5 | awk -F'(' '{print $1}' | awk '{print $NF}' | tr -d '\*' | grep -E ^[a-z]+ | wc -l
1083
$ git grep -h -E '^[a-z].*\(' drivers/infiniband/hw/mlx5 | awk -F'(' '{print $1}' | awk '{print $NF}' | tr -d '\*' | grep -E ^[a-z]+ | grep -v -e ^mlx5 | wc -l
476
TBH, i have no strong stomach to do such cleanup so far :)
Thanks
Zhijian
>
> Thanks and Regards,
> Zhu Yanjun
>
>> Thanks
>> Zhijian
>>
>>
>>> So when debug, rxe_ prefix can help us.
>>>
>>>> + int opcode, const char *msg)
>>>> {
>>>> - int err = 0;
>>>> + int err;
>>>> struct rxe_pkt_info ack_pkt;
>>>> struct sk_buff *skb;
>>>>
>>>> - skb = prepare_ack_packet(qp, &ack_pkt, IB_OPCODE_RC_ACKNOWLEDGE,
>>>> - 0, psn, syndrome);
>>>> - if (!skb) {
>>>> - err = -ENOMEM;
>>>> - goto err1;
>>>> - }
>>>> + skb = prepare_ack_packet(qp, &ack_pkt, opcode, 0, psn, syndrome);
>>>> + if (!skb)
>>>> + return -ENOMEM;
>>>>
>>>> err = rxe_xmit_packet(qp, &ack_pkt, skb);
>>>> if (err)
>>>> - pr_err_ratelimited("Failed sending ack\n");
>>>> + pr_err_ratelimited("Failed sending %s\n", msg);
>>>>
>>>> -err1:
>>>> return err;
>>>> }
>>>>
>>>> -static int send_atomic_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
>>>> +static int send_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
>>> rxe_send_ack
>>>
>>>> {
>>>> - int err = 0;
>>>> - struct rxe_pkt_info ack_pkt;
>>>> - struct sk_buff *skb;
>>>> -
>>>> - skb = prepare_ack_packet(qp, &ack_pkt, IB_OPCODE_RC_ATOMIC_ACKNOWLEDGE,
>>>> - 0, psn, syndrome);
>>>> - if (!skb) {
>>>> - err = -ENOMEM;
>>>> - goto out;
>>>> - }
>>>> + return send_common_ack(qp, syndrome, psn,
>>>> + IB_OPCODE_RC_ACKNOWLEDGE, "ACK");
>>>> +}
>>>>
>>>> - err = rxe_xmit_packet(qp, &ack_pkt, skb);
>>>> - if (err)
>>>> - pr_err_ratelimited("Failed sending atomic ack\n");
>>>> +static int send_atomic_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
>>> rxe_send_atomic_ack
>>>
>>> Thanks and Regards,
>>> Zhu Yanjun
>>>> +{
>>>> + int ret = send_common_ack(qp, syndrome, psn,
>>>> + IB_OPCODE_RC_ATOMIC_ACKNOWLEDGE, "ATOMIC ACK");
>>>>
>>>> /* have to clear this since it is used to trigger
>>>> * long read replies
>>>> */
>>>> qp->resp.res = NULL;
>>>> -out:
>>>> - return err;
>>>> + return ret;
>>>> }
>>>>
>>>> static enum resp_states acknowledge(struct rxe_qp *qp,
>>>> --
>>>> 1.8.3.1
>>>>
在 2022/8/2 13:52, [email protected] 写道:
>
>
> On 01/08/2022 15:47, Zhu Yanjun wrote:
>> On Mon, Aug 1, 2022 at 3:28 PM [email protected]
>> <[email protected]> wrote:
>>>
>>>
>>> On 01/08/2022 15:11, Zhu Yanjun wrote:
>>>> On Mon, Aug 1, 2022 at 2:16 PM Li Zhijian <[email protected]> wrote:
>>>>> Most code in send_ack() and send_atomic_ack() are duplicate, move them
>>>>> to a new helper send_common_ack().
>>>>>
>>>>> In newer IBA SPEC, some opcodes require acknowledge with a zero-length read
>>>>> response, with this new helper, we can easily implement it later.
>>>>>
>>>>> Signed-off-by: Li Zhijian <[email protected]>
>>>>> ---
>>>>> drivers/infiniband/sw/rxe/rxe_resp.c | 43 ++++++++++++++----------------------
>>>>> 1 file changed, 17 insertions(+), 26 deletions(-)
>>>>>
>>>>> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
>>>>> index b36ec5c4d5e0..4c398fa220fa 100644
>>>>> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
>>>>> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
>>>>> @@ -1028,50 +1028,41 @@ static enum resp_states do_complete(struct rxe_qp *qp,
>>>>> return RESPST_CLEANUP;
>>>>> }
>>>>>
>>>>> -static int send_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
>>>>> +
>>>>> +static int send_common_ack(struct rxe_qp *qp, u8 syndrome, u32 psn,
>>>> The function is better with rxe_send_common_ack?
>>> I'm not clear the exact rule about the naming prefix. if it has, please let me know :)
>>>
>>> IMHO, if a function is either a public API(export function) or a callback to a upper layer, it's a good idea to a fixed prefix.
>>> Instead, if they are just static, no prefix is not too bad.
>> When debug, a rxe_ prefix can help us filter the functions whatever
>> the function static or public.
>>
>>> BTW, current RXE are mixing the two rules, it should be another standalone patch to do the cleanup if needed.
>> Yes. Please make this standalone patch to complete this.
>
> i tried to do a rough statistic.
>
> all functions:
> $ git grep -E '^[a-z].*\(' drivers/infiniband/sw/rxe | awk -F'(' '{print $1}' | awk '{print $NF}' | tr -d '\*' | grep -E ^[a-z]+ | wc -l
> 474
>
> without rxe_ prefix:
> git grep -E '^[a-z].*\(' drivers/infiniband/sw/rxe | awk -F'(' '{print $1}' | awk '{print $NF}' | tr -d '\*' | grep -E ^[a-z]+ | grep -v -e ^rxe | wc -l
> 199
The followings are the no rxe_ prefix functions. About 22 functions.
do_complete
retransmit_timer
next_opcode
rnr_nak_timer
find_resource
send_data_in
prepare_ack_packet
send_ack
check_keys
check_type_state
resize_finish
do_mmap_info
parent_show
post_one_recv
free_rd_atomic_resources
free_rd_atomic_resource
lookup_iova
mr_check_range
iova_to_vaddr
advance_dma_data
lookup_mr
copy_data
Zhu Yanjun
>
> Similarly, the mlx5 have the same situations.
> $ git grep -h -E '^[a-z].*\(' drivers/infiniband/hw/mlx5 | awk -F'(' '{print $1}' | awk '{print $NF}' | tr -d '\*' | grep -E ^[a-z]+ | wc -l
> 1083
> $ git grep -h -E '^[a-z].*\(' drivers/infiniband/hw/mlx5 | awk -F'(' '{print $1}' | awk '{print $NF}' | tr -d '\*' | grep -E ^[a-z]+ | grep -v -e ^mlx5 | wc -l
> 476
>
> TBH, i have no strong stomach to do such cleanup so far :)
>
> Thanks
> Zhijian
>
>
>>
>> Thanks and Regards,
>> Zhu Yanjun
>>
>>> Thanks
>>> Zhijian
>>>
>>>
>>>> So when debug, rxe_ prefix can help us.
>>>>
>>>>> + int opcode, const char *msg)
>>>>> {
>>>>> - int err = 0;
>>>>> + int err;
>>>>> struct rxe_pkt_info ack_pkt;
>>>>> struct sk_buff *skb;
>>>>>
>>>>> - skb = prepare_ack_packet(qp, &ack_pkt, IB_OPCODE_RC_ACKNOWLEDGE,
>>>>> - 0, psn, syndrome);
>>>>> - if (!skb) {
>>>>> - err = -ENOMEM;
>>>>> - goto err1;
>>>>> - }
>>>>> + skb = prepare_ack_packet(qp, &ack_pkt, opcode, 0, psn, syndrome);
>>>>> + if (!skb)
>>>>> + return -ENOMEM;
>>>>>
>>>>> err = rxe_xmit_packet(qp, &ack_pkt, skb);
>>>>> if (err)
>>>>> - pr_err_ratelimited("Failed sending ack\n");
>>>>> + pr_err_ratelimited("Failed sending %s\n", msg);
>>>>>
>>>>> -err1:
>>>>> return err;
>>>>> }
>>>>>
>>>>> -static int send_atomic_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
>>>>> +static int send_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
>>>> rxe_send_ack
>>>>
>>>>> {
>>>>> - int err = 0;
>>>>> - struct rxe_pkt_info ack_pkt;
>>>>> - struct sk_buff *skb;
>>>>> -
>>>>> - skb = prepare_ack_packet(qp, &ack_pkt, IB_OPCODE_RC_ATOMIC_ACKNOWLEDGE,
>>>>> - 0, psn, syndrome);
>>>>> - if (!skb) {
>>>>> - err = -ENOMEM;
>>>>> - goto out;
>>>>> - }
>>>>> + return send_common_ack(qp, syndrome, psn,
>>>>> + IB_OPCODE_RC_ACKNOWLEDGE, "ACK");
>>>>> +}
>>>>>
>>>>> - err = rxe_xmit_packet(qp, &ack_pkt, skb);
>>>>> - if (err)
>>>>> - pr_err_ratelimited("Failed sending atomic ack\n");
>>>>> +static int send_atomic_ack(struct rxe_qp *qp, u8 syndrome, u32 psn)
>>>> rxe_send_atomic_ack
>>>>
>>>> Thanks and Regards,
>>>> Zhu Yanjun
>>>>> +{
>>>>> + int ret = send_common_ack(qp, syndrome, psn,
>>>>> + IB_OPCODE_RC_ATOMIC_ACKNOWLEDGE, "ATOMIC ACK");
>>>>>
>>>>> /* have to clear this since it is used to trigger
>>>>> * long read replies
>>>>> */
>>>>> qp->resp.res = NULL;
>>>>> -out:
>>>>> - return err;
>>>>> + return ret;
>>>>> }
>>>>>
>>>>> static enum resp_states acknowledge(struct rxe_qp *qp,
>>>>> --
>>>>> 1.8.3.1
>>>>>
On Mon, Aug 01, 2022 at 06:23:30AM +0000, Li Zhijian wrote:
> Most code in send_ack() and send_atomic_ack() are duplicate, move them
> to a new helper send_common_ack().
>
> In newer IBA SPEC, some opcodes require acknowledge with a zero-length read
> response, with this new helper, we can easily implement it later.
>
> Signed-off-by: Li Zhijian <[email protected]>
> ---
> drivers/infiniband/sw/rxe/rxe_resp.c | 43 ++++++++++++++----------------------
> 1 file changed, 17 insertions(+), 26 deletions(-)
Applied to for-next, thanks
If someone wants to standardize on rxe_ prefixes in the driver please
send that as a series
Jason