Re: [PATCH 1/2] RDMA/rtrs: remove unnecessary argument dir of rtrs_iu_free

From: Jinpu Wang
Date: Thu Oct 01 2020 - 05:43:26 EST


On Thu, Oct 1, 2020 at 9:26 AM Gioh Kim <gi-oh.kim@xxxxxxxxxxxxxxx> wrote:
>
> From: Gioh Kim <gi-oh.kim@xxxxxxxxxxxxxxx>
>
> The direction of DMA operation is already in the rtrs_iu.
>
> Signed-off-by: Gioh Kim <gi-oh.kim@xxxxxxxxxxxxxxx>
Looks fine to me!
Acked-by: Jack Wang <jinpu.wang@xxxxxxxxxxxxxxx>
> ---
> drivers/infiniband/ulp/rtrs/rtrs-clt.c | 14 ++++++--------
> drivers/infiniband/ulp/rtrs/rtrs-pri.h | 3 +--
> drivers/infiniband/ulp/rtrs/rtrs-srv.c | 14 ++++++--------
> drivers/infiniband/ulp/rtrs/rtrs.c | 10 +++++-----
> 4 files changed, 18 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> index 776e89231c52..7af5f1559451 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> @@ -1236,8 +1236,7 @@ static void free_sess_reqs(struct rtrs_clt_sess *sess)
> if (req->mr)
> ib_dereg_mr(req->mr);
> kfree(req->sge);
> - rtrs_iu_free(req->iu, DMA_TO_DEVICE,
> - sess->s.dev->ib_dev, 1);
> + rtrs_iu_free(req->iu, sess->s.dev->ib_dev, 1);
> }
> kfree(sess->reqs);
> sess->reqs = NULL;
> @@ -1605,8 +1604,7 @@ static void destroy_con_cq_qp(struct rtrs_clt_con *con)
>
> rtrs_cq_qp_destroy(&con->c);
> if (con->rsp_ius) {
> - rtrs_iu_free(con->rsp_ius, DMA_FROM_DEVICE,
> - sess->s.dev->ib_dev, con->queue_size);
> + rtrs_iu_free(con->rsp_ius, sess->s.dev->ib_dev, con->queue_size);
> con->rsp_ius = NULL;
> con->queue_size = 0;
> }
> @@ -2245,7 +2243,7 @@ static void rtrs_clt_info_req_done(struct ib_cq *cq, struct ib_wc *wc)
> struct rtrs_iu *iu;
>
> iu = container_of(wc->wr_cqe, struct rtrs_iu, cqe);
> - rtrs_iu_free(iu, DMA_TO_DEVICE, sess->s.dev->ib_dev, 1);
> + rtrs_iu_free(iu, sess->s.dev->ib_dev, 1);
>
> if (unlikely(wc->status != IB_WC_SUCCESS)) {
> rtrs_err(sess->clt, "Sess info request send failed: %s\n",
> @@ -2374,7 +2372,7 @@ static void rtrs_clt_info_rsp_done(struct ib_cq *cq, struct ib_wc *wc)
>
> out:
> rtrs_clt_update_wc_stats(con);
> - rtrs_iu_free(iu, DMA_FROM_DEVICE, sess->s.dev->ib_dev, 1);
> + rtrs_iu_free(iu, sess->s.dev->ib_dev, 1);
> rtrs_clt_change_state(sess, state);
> }
>
> @@ -2436,9 +2434,9 @@ static int rtrs_send_sess_info(struct rtrs_clt_sess *sess)
>
> out:
> if (tx_iu)
> - rtrs_iu_free(tx_iu, DMA_TO_DEVICE, sess->s.dev->ib_dev, 1);
> + rtrs_iu_free(tx_iu, sess->s.dev->ib_dev, 1);
> if (rx_iu)
> - rtrs_iu_free(rx_iu, DMA_FROM_DEVICE, sess->s.dev->ib_dev, 1);
> + rtrs_iu_free(rx_iu, sess->s.dev->ib_dev, 1);
> if (unlikely(err))
> /* If we've never taken async path because of malloc problems */
> rtrs_clt_change_state(sess, RTRS_CLT_CONNECTING_ERR);
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-pri.h b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> index 0a93c87ef92b..63b128c6eb04 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> @@ -288,8 +288,7 @@ struct rtrs_msg_rdma_hdr {
> struct rtrs_iu *rtrs_iu_alloc(u32 queue_size, size_t size, gfp_t t,
> struct ib_device *dev, enum dma_data_direction,
> void (*done)(struct ib_cq *cq, struct ib_wc *wc));
> -void rtrs_iu_free(struct rtrs_iu *iu, enum dma_data_direction dir,
> - struct ib_device *dev, u32 queue_size);
> +void rtrs_iu_free(struct rtrs_iu *iu, struct ib_device *dev, u32 queue_size);
> int rtrs_iu_post_recv(struct rtrs_con *con, struct rtrs_iu *iu);
> int rtrs_iu_post_send(struct rtrs_con *con, struct rtrs_iu *iu, size_t size,
> struct ib_send_wr *head);
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> index a219bd1bdbc2..b8763fe9152f 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> @@ -575,8 +575,7 @@ static void unmap_cont_bufs(struct rtrs_srv_sess *sess)
> struct rtrs_srv_mr *srv_mr;
>
> srv_mr = &sess->mrs[i];
> - rtrs_iu_free(srv_mr->iu, DMA_TO_DEVICE,
> - sess->s.dev->ib_dev, 1);
> + rtrs_iu_free(srv_mr->iu, sess->s.dev->ib_dev, 1);
> ib_dereg_mr(srv_mr->mr);
> ib_dma_unmap_sg(sess->s.dev->ib_dev, srv_mr->sgt.sgl,
> srv_mr->sgt.nents, DMA_BIDIRECTIONAL);
> @@ -680,8 +679,7 @@ static int map_cont_bufs(struct rtrs_srv_sess *sess)
> sgt = &srv_mr->sgt;
> mr = srv_mr->mr;
> free_iu:
> - rtrs_iu_free(srv_mr->iu, DMA_TO_DEVICE,
> - sess->s.dev->ib_dev, 1);
> + rtrs_iu_free(srv_mr->iu, sess->s.dev->ib_dev, 1);
> dereg_mr:
> ib_dereg_mr(mr);
> unmap_sg:
> @@ -733,7 +731,7 @@ static void rtrs_srv_info_rsp_done(struct ib_cq *cq, struct ib_wc *wc)
> struct rtrs_iu *iu;
>
> iu = container_of(wc->wr_cqe, struct rtrs_iu, cqe);
> - rtrs_iu_free(iu, DMA_TO_DEVICE, sess->s.dev->ib_dev, 1);
> + rtrs_iu_free(iu, sess->s.dev->ib_dev, 1);
>
> if (unlikely(wc->status != IB_WC_SUCCESS)) {
> rtrs_err(s, "Sess info response send failed: %s\n",
> @@ -859,7 +857,7 @@ static int process_info_req(struct rtrs_srv_con *con,
> if (unlikely(err)) {
> rtrs_err(s, "rtrs_iu_post_send(), err: %d\n", err);
> iu_free:
> - rtrs_iu_free(tx_iu, DMA_TO_DEVICE, sess->s.dev->ib_dev, 1);
> + rtrs_iu_free(tx_iu, sess->s.dev->ib_dev, 1);
> }
> rwr_free:
> kfree(rwr);
> @@ -904,7 +902,7 @@ static void rtrs_srv_info_req_done(struct ib_cq *cq, struct ib_wc *wc)
> goto close;
>
> out:
> - rtrs_iu_free(iu, DMA_FROM_DEVICE, sess->s.dev->ib_dev, 1);
> + rtrs_iu_free(iu, sess->s.dev->ib_dev, 1);
> return;
> close:
> close_sess(sess);
> @@ -927,7 +925,7 @@ static int post_recv_info_req(struct rtrs_srv_con *con)
> err = rtrs_iu_post_recv(&con->c, rx_iu);
> if (unlikely(err)) {
> rtrs_err(s, "rtrs_iu_post_recv(), err: %d\n", err);
> - rtrs_iu_free(rx_iu, DMA_FROM_DEVICE, sess->s.dev->ib_dev, 1);
> + rtrs_iu_free(rx_iu, sess->s.dev->ib_dev, 1);
> return err;
> }
>
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs.c b/drivers/infiniband/ulp/rtrs/rtrs.c
> index ff1093d6e4bc..5163e662f86f 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs.c
> @@ -31,6 +31,8 @@ struct rtrs_iu *rtrs_iu_alloc(u32 queue_size, size_t size, gfp_t gfp_mask,
> return NULL;
> for (i = 0; i < queue_size; i++) {
> iu = &ius[i];
> + iu->direction = dir;
> +
> iu->buf = kzalloc(size, gfp_mask);
> if (!iu->buf)
> goto err;
> @@ -41,17 +43,15 @@ struct rtrs_iu *rtrs_iu_alloc(u32 queue_size, size_t size, gfp_t gfp_mask,
>
> iu->cqe.done = done;
> iu->size = size;
> - iu->direction = dir;
> }
> return ius;
> err:
> - rtrs_iu_free(ius, dir, dma_dev, i);
> + rtrs_iu_free(ius, dma_dev, i);
> return NULL;
> }
> EXPORT_SYMBOL_GPL(rtrs_iu_alloc);
>
> -void rtrs_iu_free(struct rtrs_iu *ius, enum dma_data_direction dir,
> - struct ib_device *ibdev, u32 queue_size)
> +void rtrs_iu_free(struct rtrs_iu *ius, struct ib_device *ibdev, u32 queue_size)
> {
> struct rtrs_iu *iu;
> int i;
> @@ -61,7 +61,7 @@ void rtrs_iu_free(struct rtrs_iu *ius, enum dma_data_direction dir,
>
> for (i = 0; i < queue_size; i++) {
> iu = &ius[i];
> - ib_dma_unmap_single(ibdev, iu->dma_addr, iu->size, dir);
> + ib_dma_unmap_single(ibdev, iu->dma_addr, iu->size, iu->direction);
> kfree(iu->buf);
> }
> kfree(ius);
> --
> 2.20.1
>