On 5/17/22 14:08, yanjun.zhu@xxxxxxxxx wrote:
> From: Zhu Yanjun <yanjun.zhu@xxxxxxxxx>
>
> Compact the function and move it to the header file.
I have two issues with this patch.
There is no advantage of having an inline function in a header file
that is only called once. The compiler is perfectly capable of (and does)
inlining a static function in a .c file if only called once. This just
makes the code harder to read.
There is a patch in for-rc that gets rid of read.mr in favor of an rkey.
This patch is out of date.
Bob
>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@xxxxxxxxx>
> ---
> drivers/infiniband/sw/rxe/rxe_loc.h | 11 ++++++++++-
> drivers/infiniband/sw/rxe/rxe_qp.c | 15 ++-------------
> drivers/infiniband/sw/rxe/rxe_resp.c | 4 ++--
> 3 files changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_loc.h b/drivers/infiniband/sw/rxe/rxe_loc.h
> index 409efeecd581..6517b4f104b1 100644
> --- a/drivers/infiniband/sw/rxe/rxe_loc.h
> +++ b/drivers/infiniband/sw/rxe/rxe_loc.h
> @@ -145,7 +145,16 @@ static inline int rcv_wqe_size(int max_sge)
> max_sge * sizeof(struct ib_sge);
> }
>
> -void free_rd_atomic_resource(struct rxe_qp *qp, struct resp_res *res);
> +static inline void free_rd_atomic_resource(struct resp_res *res)
> +{
> + if (res->type == RXE_ATOMIC_MASK) {
> + kfree_skb(res->atomic.skb);
> + } else if (res->type == RXE_READ_MASK) {
> + if (res->read.mr)
> + rxe_drop_ref(res->read.mr);
> + }
> + res->type = 0;
> +}
>
> static inline void rxe_advance_resp_resource(struct rxe_qp *qp)
> {
> diff --git a/drivers/infiniband/sw/rxe/rxe_qp.c b/drivers/infiniband/sw/rxe/rxe_qp.c
> index 5f270cbf18c6..b29208852bc4 100644
> --- a/drivers/infiniband/sw/rxe/rxe_qp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_qp.c
> @@ -126,24 +126,13 @@ static void free_rd_atomic_resources(struct rxe_qp *qp)
> for (i = 0; i < qp->attr.max_dest_rd_atomic; i++) {
> struct resp_res *res = &qp->resp.resources[i];
>
> - free_rd_atomic_resource(qp, res);
> + free_rd_atomic_resource(res);
> }
> kfree(qp->resp.resources);
> qp->resp.resources = NULL;
> }
> }
>
> -void free_rd_atomic_resource(struct rxe_qp *qp, struct resp_res *res)
> -{
> - if (res->type == RXE_ATOMIC_MASK) {
> - kfree_skb(res->atomic.skb);
> - } else if (res->type == RXE_READ_MASK) {
> - if (res->read.mr)
> - rxe_drop_ref(res->read.mr);
> - }
> - res->type = 0;
> -}
> -
> static void cleanup_rd_atomic_resources(struct rxe_qp *qp)
> {
> int i;
> @@ -152,7 +141,7 @@ static void cleanup_rd_atomic_resources(struct rxe_qp *qp)
> if (qp->resp.resources) {
> for (i = 0; i < qp->attr.max_dest_rd_atomic; i++) {
> res = &qp->resp.resources[i];
> - free_rd_atomic_resource(qp, res);
> + free_rd_atomic_resource(res);
> }
> }
> }
> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c
> index c369d78fc8e8..923a71ff305c 100644
> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
> @@ -663,7 +663,7 @@ static enum resp_states read_reply(struct rxe_qp *qp,
> */
> res = &qp->resp.resources[qp->resp.res_head];
>
> - free_rd_atomic_resource(qp, res);
> + free_rd_atomic_resource(res);
> rxe_advance_resp_resource(qp);
>
> res->type = RXE_READ_MASK;
> @@ -977,7 +977,7 @@ static int send_atomic_ack(struct rxe_qp *qp, struct rxe_pkt_info *pkt,
> }
>
> res = &qp->resp.resources[qp->resp.res_head];
> - free_rd_atomic_resource(qp, res);
> + free_rd_atomic_resource(res);
> rxe_advance_resp_resource(qp);
>
> skb_get(skb);
