在 2019/8/28 23:19, Doug Ledford 写道:
> On Wed, 2019-08-21 at 21:14 +0800, Lijun Ou wrote:
>> +static int hns_roce_alloc_recv_inline_buffer(struct hns_roce_qp
>> *hr_qp,
>> + struct ib_qp_init_attr
>> *init_attr)
>> +{
>> + int ret;
>> + int i;
>> +
>> + /* allocate recv inline buf */
>> + hr_qp->rq_inl_buf.wqe_list = kcalloc(hr_qp->rq.wqe_cnt,
>> + sizeof(struct
>> hns_roce_rinl_wqe),
>> + GFP_KERNEL);
>> + if (!hr_qp->rq_inl_buf.wqe_list) {
>> + ret = -ENOMEM;
>> + goto err;
>> + }
>> +
>> + hr_qp->rq_inl_buf.wqe_cnt = hr_qp->rq.wqe_cnt;
>> +
>> + /* Firstly, allocate a list of sge space buffer */
>> + hr_qp->rq_inl_buf.wqe_list[0].sg_list =
>> + kcalloc(hr_qp-
>>> rq_inl_buf.wqe_cnt,
>> + init_attr->cap.max_recv_sge *
>> + sizeof(struct
>> hns_roce_rinl_sge),
>> + GFP_KERNEL);
>> + if (!hr_qp->rq_inl_buf.wqe_list[0].sg_list) {
>> + ret = -ENOMEM;
>> + goto err_wqe_list;
>> + }
>> +
>> + for (i = 1; i < hr_qp->rq_inl_buf.wqe_cnt; i++)
>> + /* Secondly, reallocate the buffer */
>> + hr_qp->rq_inl_buf.wqe_list[i].sg_list =
>> + &hr_qp-
>>> rq_inl_buf.wqe_list[0].sg_list[i *
>> + init_attr->cap.max_recv_sge];
>> +
>> + return 0;
>> +
>> +err_wqe_list:
>> + kfree(hr_qp->rq_inl_buf.wqe_list);
>> +
>> +err:
>> + return ret;
>> +}
> This function is klunky. You don't need int ret; at all as there are
> only two possible return values and you have distinct locations for each
> return, so each return can use a constant. It would be much more
> readable like this:
>
> +static int hns_roce_alloc_recv_inline_buffer(struct hns_roce_qp *hr_qp,
> + struct ib_qp_init_attr *init_attr)
> +{
> + int num_sge = init_attr->cap.max_recv_sge;
> + int wqe_cnt = hr_qp->rq.wqe_cnt;
> + int i;
> +
> + /* allocate recv inline WQE bufs */
> + hr_qp->rq_inl_buf.wqe_list = kcalloc(wqe_cnt,
> + sizeof(struct hns_roce_rinl_wqe),
> + GFP_KERNEL);
> + if (!hr_qp->rq_inl_buf.wqe_list)
> + goto err;
> +
> + hr_qp->rq_inl_buf.wqe_cnt = wqe_cnt;
> +
> + /* allocate a single sge array for all WQEs */
> + hr_qp->rq_inl_buf.wqe_list[0].sg_list =
> + kcalloc(wqe_cnt,
> + num_sge *
> + sizeof(struct hns_roce_rinl_sge),
> + GFP_KERNEL);
> + if (!hr_qp->rq_inl_buf.wqe_list[0].sg_list)
> + goto err_wqe_list;
> +
> + for (i = 1; i < wqe_cnt; i++)
> + /* give each WQE a pointer to its array space */
> + hr_qp->rq_inl_buf.wqe_list[i].sg_list =
> + &hr_qp->rq_inl_buf.wqe_list[0].sg_list[i * num_sge];
> +
> + return 0;
> +
> +err_wqe_list:
> + kfree(hr_qp->rq_inl_buf.wqe_list);
> +err:
> + return -ENOMEM;
> +}
>
Thanks， I will consider accept your advice and fixes it.
