On Mon, Jan 20, 2020 at 04:19:34PM +0800, Weihang Li wrote:
> From: Xi Wang <wangxi11@xxxxxxxxxx>
>
> Encapsulate qp buffer allocation related code into 3 functions:
> alloc_qp_buf(), map_qp_buf() and free_qp_buf().
>
> Signed-off-by: Xi Wang <wangxi11@xxxxxxxxxx>
> Signed-off-by: Weihang Li <liweihang@xxxxxxxxxx>
> ---
> drivers/infiniband/hw/hns/hns_roce_device.h | 1 -
> drivers/infiniband/hw/hns/hns_roce_qp.c | 268 +++++++++++++++-------------
> 2 files changed, 147 insertions(+), 122 deletions(-)
>
> diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
> index 1f361e6..9ddeb2b 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_device.h
> +++ b/drivers/infiniband/hw/hns/hns_roce_device.h
> @@ -660,7 +660,6 @@ struct hns_roce_qp {
> /* this define must less than HNS_ROCE_MAX_BT_REGION */
> #define HNS_ROCE_WQE_REGION_MAX 3
> struct hns_roce_buf_region regions[HNS_ROCE_WQE_REGION_MAX];
> - int region_cnt;
> int wqe_bt_pg_shift;
>
> u32 buff_size;
> diff --git a/drivers/infiniband/hw/hns/hns_roce_qp.c b/drivers/infiniband/hw/hns/hns_roce_qp.c
> index 3bd5809..5184cb4 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_qp.c
> +++ b/drivers/infiniband/hw/hns/hns_roce_qp.c
> @@ -716,23 +716,150 @@ static void free_rq_inline_buf(struct hns_roce_qp *hr_qp)
> kfree(hr_qp->rq_inl_buf.wqe_list);
> }
>
> +static int map_qp_buf(struct hns_roce_dev *hr_dev, struct hns_roce_qp *hr_qp,
> + u32 page_shift, bool is_user)
> +{
> + dma_addr_t *buf_list[ARRAY_SIZE(hr_qp->regions)] = { NULL };
> + struct ib_device *ibdev = &hr_dev->ib_dev;
> + struct hns_roce_buf_region *r;
> + int region_count;
> + int buf_count;
> + int ret;
> + int i;
> +
> + region_count = split_wqe_buf_region(hr_dev, hr_qp, hr_qp->regions,
> + ARRAY_SIZE(hr_qp->regions), page_shift);
> +
> + /* alloc a tmp list for storing wqe buf address */
> + ret = hns_roce_alloc_buf_list(hr_qp->regions, buf_list, region_count);
> + if (ret) {
> + ibdev_err(ibdev, "alloc buf_list error for create qp\n");
> + return ret;
> + }
> +
> + for (i = 0; i < region_count; i++) {
> + r = &hr_qp->regions[i];
> + if (is_user)
> + buf_count = hns_roce_get_umem_bufs(hr_dev, buf_list[i],
> + r->count, r->offset, hr_qp->umem,
> + page_shift);
> + else
> + buf_count = hns_roce_get_kmem_bufs(hr_dev, buf_list[i],
> + r->count, r->offset, &hr_qp->hr_buf);
> +
> + if (buf_count != r->count) {
> + ibdev_err(ibdev, "get %s qp buf err,expect %d,ret %d.\n",
> + is_user ? "user" : "kernel",
> + r->count, buf_count);
> + ret = -ENOBUFS;
> + goto done;
> + }
> + }
> +
> + hr_qp->wqe_bt_pg_shift = calc_wqe_bt_page_shift(hr_dev, hr_qp->regions,
> + region_count);
> + hns_roce_mtr_init(&hr_qp->mtr, PAGE_SHIFT + hr_qp->wqe_bt_pg_shift,
> + page_shift);
> + ret = hns_roce_mtr_attach(hr_dev, &hr_qp->mtr, buf_list, hr_qp->regions,
> + region_count);
> + if (ret)
> + ibdev_err(ibdev, "mtr attatch error for create qp\n");
> +
> + goto done;
> +
> + hns_roce_mtr_cleanup(hr_dev, &hr_qp->mtr);
> +done:
> + hns_roce_free_buf_list(buf_list, region_count);
> +
> + return ret;
> +}
> +
> +static int alloc_qp_buf(struct hns_roce_dev *hr_dev, struct hns_roce_qp *hr_qp,
> + struct ib_qp_init_attr *init_attr,
> + struct ib_udata *udata, unsigned long addr)
> +{
> + u32 page_shift = PAGE_SHIFT + hr_dev->caps.mtt_buf_pg_sz;
> + struct ib_device *ibdev = &hr_dev->ib_dev;
> + bool is_rq_buf_inline;
> + int ret;
> +
> + is_rq_buf_inline = (hr_dev->caps.flags & HNS_ROCE_CAP_FLAG_RQ_INLINE) &&
> + hns_roce_qp_has_rq(init_attr);
> + if (is_rq_buf_inline) {
> + ret = alloc_rq_inline_buf(hr_qp, init_attr);
> + if (ret) {
> + ibdev_err(ibdev, "alloc recv inline buffer error\n");
> + return ret;
> + }
> + }
> +
> + if (udata) {
> + hr_qp->umem = ib_umem_get(udata, addr, hr_qp->buff_size, 0);
> + if (IS_ERR(hr_qp->umem)) {
> + ibdev_err(ibdev, "get umem error for qp buf\n");
> + ret = PTR_ERR(hr_qp->umem);
> + goto err_inline;
> + }
> + } else {
> + ret = hns_roce_buf_alloc(hr_dev, hr_qp->buff_size,
> + (1 << page_shift) * 2,
> + &hr_qp->hr_buf, page_shift);
> + if (ret) {
> + ibdev_err(ibdev, "alloc roce buf error\n");
> + goto err_inline;
> + }
> + }
> +
> + ret = map_qp_buf(hr_dev, hr_qp, page_shift, udata);
I don't remember what was the resolution if it is ok to rely on "udata"
as an indicator of user/kernel flow.
> + if (ret) {
> + ibdev_err(ibdev, "map roce buf error\n");
You put ibdev_err() on almost every line in map_qp_buf(), please leave
only one place.
Thanks
