On Fri, Feb 25, 2022 at 05:56:54PM +0800, Wenpeng Liang wrote:
> From: Yixing Liu <liuyixing1@xxxxxxxxxx>
>
> Before destroying MPT, the reserved loopback QPs send loopback IOs (one
> write operation per SL). Completing these loopback IOs represents that
> there isn't any outstanding request in MPT, then it's safe to destroy MPT.
>
> Signed-off-by: Yixing Liu <liuyixing1@xxxxxxxxxx>
> Signed-off-by: Wenpeng Liang <liangwenpeng@xxxxxxxxxx>
> ---
> drivers/infiniband/hw/hns/hns_roce_device.h | 2 +
> drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 334 +++++++++++++++++++-
> drivers/infiniband/hw/hns/hns_roce_hw_v2.h | 20 ++
> drivers/infiniband/hw/hns/hns_roce_mr.c | 6 +-
> 4 files changed, 358 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
> index 1e0bae136997..da0b4b310aab 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_device.h
> +++ b/drivers/infiniband/hw/hns/hns_roce_device.h
> @@ -624,6 +624,7 @@ struct hns_roce_qp {
> u32 next_sge;
> enum ib_mtu path_mtu;
> u32 max_inline_data;
> + u8 free_mr_en;
>
> /* 0: flush needed, 1: unneeded */
> unsigned long flush_flag;
> @@ -882,6 +883,7 @@ struct hns_roce_hw {
> enum ib_qp_state new_state);
> int (*qp_flow_control_init)(struct hns_roce_dev *hr_dev,
> struct hns_roce_qp *hr_qp);
> + void (*dereg_mr)(struct hns_roce_dev *hr_dev);
> int (*init_eq)(struct hns_roce_dev *hr_dev);
> void (*cleanup_eq)(struct hns_roce_dev *hr_dev);
> int (*write_srqc)(struct hns_roce_srq *srq, void *mb_buf);
> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
> index b33e948fd060..62ee9c0bba74 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
> @@ -2664,6 +2664,217 @@ static void free_dip_list(struct hns_roce_dev *hr_dev)
> spin_unlock_irqrestore(&hr_dev->dip_list_lock, flags);
> }
>
> +static int free_mr_alloc_pd(struct hns_roce_dev *hr_dev,
> + struct hns_roce_v2_free_mr *free_mr)
> +{
You chose very non-intuitive name "free_mr...", but I don't have anything
concrete to suggest.
> + struct ib_device *ibdev = &hr_dev->ib_dev;
> + struct ib_pd *pd;
> +
> + pd = ib_alloc_pd(ibdev, 0);
> + if (IS_ERR(pd)) {
> + ibdev_err(ibdev, "failed to create pd for free mr.\n");
> + return PTR_ERR(pd);
> + }
> +
> + free_mr->rsv_pd = pd;
> + return 0;
> +}
> +
> +static int free_mr_create_cq(struct hns_roce_dev *hr_dev,
> + struct hns_roce_v2_free_mr *free_mr)
> +{
> + struct ib_device *ibdev = &hr_dev->ib_dev;
> + struct ib_cq_init_attr cq_init_attr = {};
> + struct ib_cq *cq;
> +
> + cq_init_attr.cqe = HNS_ROCE_FREE_MR_USED_CQE_NUM;
> +
> + cq = ib_create_cq(ibdev, NULL, NULL, NULL, &cq_init_attr);
> + if (IS_ERR(cq)) {
> + ibdev_err(ibdev, "failed to create cq for free mr.\n");
> + return PTR_ERR(cq);
> + }
> +
> + free_mr->rsv_cq = cq;
> + return 0;
> +}
> +
> +static int free_mr_create_loopback_qp(struct hns_roce_dev *hr_dev,
> + struct hns_roce_v2_free_mr *free_mr,
> + int sl_num)
> +{
> + struct ib_device *ibdev = &hr_dev->ib_dev;
> + struct ib_qp_init_attr init_attr = {};
> + struct ib_pd *pd = free_mr->rsv_pd;
> + struct ib_cq *cq = free_mr->rsv_cq;
> + struct ib_qp *qp;
> +
> + init_attr.qp_type = IB_QPT_RC;
> + init_attr.sq_sig_type = IB_SIGNAL_ALL_WR;
> + init_attr.send_cq = cq;
> + init_attr.recv_cq = cq;
> + init_attr.cap.max_send_wr = HNS_ROCE_FREE_MR_USED_SQWQE_NUM;
> + init_attr.cap.max_send_sge = HNS_ROCE_FREE_MR_USED_SQSGE_NUM;
> + init_attr.cap.max_recv_wr = HNS_ROCE_FREE_MR_USED_RQWQE_NUM;
> + init_attr.cap.max_recv_sge = HNS_ROCE_FREE_MR_USED_RQSGE_NUM;
No vertical alignment in new code, please.
> +
> + qp = ib_create_qp(pd, &init_attr);
> + if (IS_ERR(qp)) {
> + ibdev_err(ibdev, "failed to create qp for free mr.\n");
> + return PTR_ERR(qp);
> + }
> +
> + free_mr->rsv_qp[sl_num] = qp;
> + return 0;
> +}
> +
> +static int free_mr_create_qp(struct hns_roce_dev *hr_dev,
> + struct hns_roce_v2_free_mr *free_mr)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(free_mr->rsv_qp); i++)
> + if (free_mr_create_loopback_qp(hr_dev, free_mr, i))
> + return -ENOMEM;
Please don't overwrite returned error code - in all places.
> +
> + return 0;
> +}
> +
> +static void free_mr_init_qp_attr(struct ib_qp_attr *attr)
> +{
> + rdma_ah_set_grh(&attr->ah_attr, NULL, 0, 0, 1, 0);
> + rdma_ah_set_static_rate(&attr->ah_attr, 3);
> + rdma_ah_set_port_num(&attr->ah_attr, 1);
> +}
> +
> +static int free_mr_modify_loopback_qp(struct hns_roce_dev *hr_dev,
> + struct ib_qp_attr *attr, int sl_num)
> +{
> + struct hns_roce_v2_priv *priv = hr_dev->priv;
> + struct hns_roce_v2_free_mr *free_mr = &priv->free_mr;
> + struct ib_device *ibdev = &hr_dev->ib_dev;
> + struct hns_roce_qp *hr_qp;
> + int loopback;
> + int mask;
> + int ret;
> +
> + hr_qp = to_hr_qp(free_mr->rsv_qp[sl_num]);
> + hr_qp->free_mr_en = 1;
> +
> + mask = IB_QP_STATE | IB_QP_PKEY_INDEX | IB_QP_PORT | IB_QP_ACCESS_FLAGS;
> + attr->qp_state = IB_QPS_INIT;
> + attr->port_num = 1;
> + attr->qp_access_flags = IB_ACCESS_REMOTE_WRITE;
> + ret = ib_modify_qp(&hr_qp->ibqp, attr, mask);
> + if (ret) {
> + ibdev_err(ibdev, "failed to modify qp to init, ret = %d.\n",
> + ret);
> + return ret;
> + }
> +
> + loopback = hr_dev->loop_idc;
> + /* Set qpc lbi = 1 incidate loopback IO */
> + hr_dev->loop_idc = 1;
> +
> + mask = IB_QP_STATE | IB_QP_AV | IB_QP_PATH_MTU | IB_QP_DEST_QPN |
> + IB_QP_RQ_PSN | IB_QP_MAX_DEST_RD_ATOMIC | IB_QP_MIN_RNR_TIMER;
> + attr->qp_state = IB_QPS_RTR;
> + attr->ah_attr.type = RDMA_AH_ATTR_TYPE_ROCE;
> + attr->path_mtu = IB_MTU_256;
> + attr->dest_qp_num = hr_qp->qpn;
> + attr->rq_psn = HNS_ROCE_FREE_MR_USED_PSN;
> +
> + rdma_ah_set_sl(&attr->ah_attr, (u8)sl_num);
> +
> + ret = ib_modify_qp(&hr_qp->ibqp, attr, mask);
> + hr_dev->loop_idc = loopback;
> + if (ret) {
> + ibdev_err(ibdev, "failed to modify qp to rtr, ret = %d.\n",
> + ret);
> + return ret;
> + }
> +
> + mask = IB_QP_STATE | IB_QP_SQ_PSN | IB_QP_RETRY_CNT | IB_QP_TIMEOUT |
> + IB_QP_RNR_RETRY | IB_QP_MAX_QP_RD_ATOMIC;
> + attr->qp_state = IB_QPS_RTS;
> + attr->sq_psn = HNS_ROCE_FREE_MR_USED_PSN;
> + attr->retry_cnt = HNS_ROCE_FREE_MR_USED_QP_RETRY_CNT;
> + attr->timeout = HNS_ROCE_FREE_MR_USED_QP_TIMEOUT;
> + ret = ib_modify_qp(&hr_qp->ibqp, attr, mask);
> + if (ret)
> + ibdev_err(ibdev, "failed to modify qp to rts, ret = %d.\n",
> + ret);
> +
> + return ret;
> +}
> +
> +static int free_mr_modify_qp(struct hns_roce_dev *hr_dev,
> + struct hns_roce_v2_free_mr *free_mr)
> +{
> + struct ib_qp_attr attr = {};
> + int ret;
> + int i;
> +
> + free_mr_init_qp_attr(&attr);
> +
> + for (i = 0; i < ARRAY_SIZE(free_mr->rsv_qp); i++) {
> + ret = free_mr_modify_loopback_qp(hr_dev, &attr, i);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void free_mr_exit(struct hns_roce_dev *hr_dev)
> +{
> + struct hns_roce_v2_priv *priv = hr_dev->priv;
> + struct hns_roce_v2_free_mr *free_mr = &priv->free_mr;
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(free_mr->rsv_qp); i++) {
> + if (free_mr->rsv_qp[i]) {
> + (void)ib_destroy_qp(free_mr->rsv_qp[i]);
Please don't cast. It is not kernel coding style.
Thanks
