在 2022/8/2 13:52, lizhijian@xxxxxxxxxxx 写道:
On 01/08/2022 15:47, Zhu Yanjun wrote:
On Mon, Aug 1, 2022 at 3:28 PM lizhijian@xxxxxxxxxxx
<lizhijian@xxxxxxxxxxx> wrote:
On 01/08/2022 15:11, Zhu Yanjun wrote:
On Mon, Aug 1, 2022 at 2:16 PM Li Zhijian <lizhijian@xxxxxxxxxxx> 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 <lizhijian@xxxxxxxxxxx>
---
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
