Re: [PATCH for-next] RDMA/hns: Refactor the hns_roce_buf allocation flow

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, Oct 29, 2020 at 06:31:02PM +0800, Weihang Li wrote:
>  	/* The minimum shift of the page accessed by hw is HNS_HW_PAGE_SHIFT */
> -	buf->page_shift = max_t(int, HNS_HW_PAGE_SHIFT, page_shift);
> +	WARN_ON(page_shift < HNS_HW_PAGE_SHIFT);

Stuff like this should be written as

  if (WARN_ON(page_shift < HNS_HW_PAGE_SHIFT))
    return ERR_PTR(-EINVAL);

Rather than wrongly continuing on

> @@ -780,19 +769,16 @@ static int mtr_alloc_bufs(struct hns_roce_dev *hr_dev, struct hns_roce_mtr *mtr,
>  		ret = 0;
>  	} else {
>  		mtr->umem = NULL;
> -		mtr->kmem = kzalloc(sizeof(*mtr->kmem), GFP_KERNEL);
> -		if (!mtr->kmem) {
> -			ibdev_err(ibdev, "Failed to alloc kmem\n");
> +		mtr->kmem =
> +			hns_roce_buf_alloc(hr_dev, total_size,
> +					   buf_attr->page_shift,
> +					   is_direct ? HNS_ROCE_BUF_DIRECT : 0);
> +		if (IS_ERR_OR_NULL(mtr->kmem)) {

Please do not use IS_ERR_OR_NULL

Routines should not return NULL and an error pointer, one or the other
- or NULL needs to have special meaning and is not an error.

> +			ibdev_err(ibdev, "failed to alloc kmem, ret = %ld.\n",
> +				  PTR_ERR(mtr->kmem));
>  			return -ENOMEM;

Here you should return PTR_ERR((mtr->kmem)

In other places in this driver too, please check all the
IS_ERR_OR_NULL's

Jason