Re: [PATCH 1/4] IB/iser: remove iser_reg_data_sg helper function

[Leon Romanovsky <leon@kernel.org>]

On Tue, Feb 15, 2022 at 01:06:29PM +0200, Max Gurtovoy wrote:
> Open coding it makes the code more readable and simple.
> 
> Reviewed-by: Sergey Gorenko <sergeygo@nvidia.com>
> Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
> ---
>  drivers/infiniband/ulp/iser/iser_memory.c | 32 ++++++++---------------
>  1 file changed, 11 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
> index 660982625488..2738ec56c918 100644
> --- a/drivers/infiniband/ulp/iser/iser_memory.c
> +++ b/drivers/infiniband/ulp/iser/iser_memory.c
> @@ -327,40 +327,29 @@ static int iser_fast_reg_mr(struct iscsi_iser_task *iser_task,
>  	return 0;
>  }
>  
> -static int iser_reg_data_sg(struct iscsi_iser_task *task,
> -			    struct iser_data_buf *mem,
> -			    struct iser_fr_desc *desc, bool use_dma_key,
> -			    struct iser_mem_reg *reg)
> -{
> -	struct iser_device *device = task->iser_conn->ib_conn.device;
> -
> -	if (use_dma_key)
> -		return iser_reg_dma(device, mem, reg);
> -
> -	return iser_fast_reg_mr(task, mem, &desc->rsc, reg);
> -}
> -
>  int iser_reg_mem_fastreg(struct iscsi_iser_task *task,
>  			 enum iser_data_dir dir,
>  			 bool all_imm)
>  {
>  	struct ib_conn *ib_conn = &task->iser_conn->ib_conn;
> +	struct iser_device *device = ib_conn->device;
>  	struct iser_data_buf *mem = &task->data[dir];
>  	struct iser_mem_reg *reg = &task->rdma_reg[dir];
> -	struct iser_fr_desc *desc = NULL;
> +	struct iser_fr_desc *desc;
>  	bool use_dma_key;
>  	int err;
>  
>  	use_dma_key = mem->dma_nents == 1 && (all_imm || !iser_always_reg) &&
>  		      scsi_get_prot_op(task->sc) == SCSI_PROT_NORMAL;
> +	if (use_dma_key)
> +		return iser_reg_dma(device, mem, reg);
>  
> -	if (!use_dma_key) {
> -		desc = iser_reg_desc_get_fr(ib_conn);
> -		reg->mem_h = desc;
> -	}
> +	desc = iser_reg_desc_get_fr(ib_conn);

It can't be NULL,
iser_reg_desc_get_fr():
   52         spin_lock_irqsave(&fr_pool->lock, flags);
   53         desc = list_first_entry(&fr_pool->list,
   54                                 struct iser_fr_desc, list);
   55         list_del(&desc->list);

If desc is NULL, it will crash.

   56         spin_unlock_irqrestore(&fr_pool->lock, flags);
   57
   58         return desc;


> +	if (unlikely(!desc))
> +		return -ENOMEM;
>  
>  	if (scsi_get_prot_op(task->sc) == SCSI_PROT_NORMAL) {
> -		err = iser_reg_data_sg(task, mem, desc, use_dma_key, reg);
> +		err = iser_fast_reg_mr(task, mem, &desc->rsc, reg);
>  		if (unlikely(err))
>  			goto err_reg;
>  	} else {
> @@ -372,11 +361,12 @@ int iser_reg_mem_fastreg(struct iscsi_iser_task *task,
>  		desc->sig_protected = true;
>  	}
>  
> +	reg->mem_h = desc;
> +
>  	return 0;
>  
>  err_reg:
> -	if (desc)
> -		iser_reg_desc_put_fr(ib_conn, desc);
> +	iser_reg_desc_put_fr(ib_conn, desc);
>  
>  	return err;
>  }
> -- 
> 2.18.1
>