Re: [PATCH 1/2] RDMA/erdma: Use the bitmap API to allocate bitmaps

[Cheng Xu]

On 7/9/22 1:37 AM, Christophe JAILLET wrote:
> Use [devm_]bitmap_zalloc()/bitmap_free() instead of hand-writing them.
> 
> It is less verbose and it improves the semantic.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>
> ---
>  drivers/infiniband/hw/erdma/erdma_cmdq.c | 7 +++----
>  drivers/infiniband/hw/erdma/erdma_main.c | 9 ++++-----
>  2 files changed, 7 insertions(+), 9 deletions(-)
> 

Hi Christophe,

Thanks for your two patches of erdma.

The erdma code your got is our first upstreaming code, so I would like to squash your
changes into the relevant commit in our next patchset to make the commit history cleaner.

BTW, the coding style in the patches is OK, but has a little differences with clang-format's
result. I will use the format from clang-format to minimize manual adjustments.
 
Thanks,
Cheng Xu
 

> diff --git a/drivers/infiniband/hw/erdma/erdma_cmdq.c b/drivers/infiniband/hw/erdma/erdma_cmdq.c
> index 0cf5032d4b78..0489838d9717 100644
> --- a/drivers/infiniband/hw/erdma/erdma_cmdq.c
> +++ b/drivers/infiniband/hw/erdma/erdma_cmdq.c
> @@ -78,10 +78,9 @@ static int erdma_cmdq_wait_res_init(struct erdma_dev *dev,
>  		return -ENOMEM;
>  
>  	spin_lock_init(&cmdq->lock);
> -	cmdq->comp_wait_bitmap =
> -		devm_kcalloc(&dev->pdev->dev,
> -			     BITS_TO_LONGS(cmdq->max_outstandings),
> -			     sizeof(unsigned long), GFP_KERNEL);
> +	cmdq->comp_wait_bitmap = devm_bitmap_zalloc(&dev->pdev->dev,
> +						    cmdq->max_outstandings,
> +						    GFP_KERNEL);
>  	if (!cmdq->comp_wait_bitmap) {
>  		devm_kfree(&dev->pdev->dev, cmdq->wait_pool);
>  		return -ENOMEM;
> diff --git a/drivers/infiniband/hw/erdma/erdma_main.c b/drivers/infiniband/hw/erdma/erdma_main.c
> index 27484bea51d9..7e1e27acb404 100644
> --- a/drivers/infiniband/hw/erdma/erdma_main.c
> +++ b/drivers/infiniband/hw/erdma/erdma_main.c
> @@ -423,9 +423,8 @@ static int erdma_res_cb_init(struct erdma_dev *dev)
>  	for (i = 0; i < ERDMA_RES_CNT; i++) {
>  		dev->res_cb[i].next_alloc_idx = 1;
>  		spin_lock_init(&dev->res_cb[i].lock);
> -		dev->res_cb[i].bitmap =
> -			kcalloc(BITS_TO_LONGS(dev->res_cb[i].max_cap),
> -				sizeof(unsigned long), GFP_KERNEL);
> +		dev->res_cb[i].bitmap = bitmap_zalloc(dev->res_cb[i].max_cap,
> +						      GFP_KERNEL);
>  		/* We will free the memory in erdma_res_cb_free */
>  		if (!dev->res_cb[i].bitmap)
>  			goto err;
> @@ -435,7 +434,7 @@ static int erdma_res_cb_init(struct erdma_dev *dev)
>  
>  err:
>  	for (j = 0; j < i; j++)
> -		kfree(dev->res_cb[j].bitmap);
> +		bitmap_free(dev->res_cb[j].bitmap);
>  
>  	return -ENOMEM;
>  }
> @@ -445,7 +444,7 @@ static void erdma_res_cb_free(struct erdma_dev *dev)
>  	int i;
>  
>  	for (i = 0; i < ERDMA_RES_CNT; i++)
> -		kfree(dev->res_cb[i].bitmap);
> +		bitmap_free(dev->res_cb[i].bitmap);
>  }
>  
>  static const struct ib_device_ops erdma_device_ops = {