[PATCH 0/4] iSER cleanups and fixes for 5.18

[PATCH 0/4] iSER cleanups and fixes for 5.18

[Max Gurtovoy]

Hi Jason/Sagi,
This series adds one small fix in error flow in case of registration
failures and 3 patches for improving code readability that will ease
on maintaining the ib_iser driver.

Please consider merging this series to kernel 5.18 merge window.

Max Gurtovoy (4):
  IB/iser: remove iser_reg_data_sg helper function
  IB/iser: use iser_fr_desc as registration context
  IB/iser: generalize map/unmap dma tasks
  IB/iser: fix error flow in case of registration failure

 drivers/infiniband/ulp/iser/iscsi_iser.h     | 13 ++--
 drivers/infiniband/ulp/iser/iser_initiator.c | 58 +++++-----------
 drivers/infiniband/ulp/iser/iser_memory.c    | 69 ++++++++++++--------
 drivers/infiniband/ulp/iser/iser_verbs.c     |  2 +-
 4 files changed, 63 insertions(+), 79 deletions(-)

-- 
2.18.1

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

[Max Gurtovoy]

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);
+	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

[PATCH 2/4] IB/iser: use iser_fr_desc as registration context

[Max Gurtovoy]

After removing the FMR support in iSER, there is only one type of
registration context. Replace the void pointer with the explicit
structure for registration (struct iser_fr_desc).

Reviewed-by: Sergey Gorenko <sergeygo@nvidia.com>
Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
---
 drivers/infiniband/ulp/iser/iscsi_iser.h     | 8 ++++----
 drivers/infiniband/ulp/iser/iser_initiator.c | 4 ++--
 drivers/infiniband/ulp/iser/iser_memory.c    | 8 ++++----
 drivers/infiniband/ulp/iser/iser_verbs.c     | 2 +-
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 20af46c4e954..23b922233006 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -203,12 +203,12 @@ struct iser_reg_resources;
  *
  * @sge:          memory region sg element
  * @rkey:         memory region remote key
- * @mem_h:        pointer to registration context (FMR/Fastreg)
+ * @desc:         pointer to fast registration context
  */
 struct iser_mem_reg {
-	struct ib_sge	 sge;
-	u32		 rkey;
-	void		*mem_h;
+	struct ib_sge sge;
+	u32 rkey;
+	struct iser_fr_desc *desc;
 };
 
 enum iser_desc_type {
diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c b/drivers/infiniband/ulp/iser/iser_initiator.c
index 2490150d3085..012decf6905a 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -619,13 +619,13 @@ static int iser_check_remote_inv(struct iser_conn *iser_conn, struct ib_wc *wc,
 			struct iser_fr_desc *desc;
 
 			if (iser_task->dir[ISER_DIR_IN]) {
-				desc = iser_task->rdma_reg[ISER_DIR_IN].mem_h;
+				desc = iser_task->rdma_reg[ISER_DIR_IN].desc;
 				if (unlikely(iser_inv_desc(desc, rkey)))
 					return -EINVAL;
 			}
 
 			if (iser_task->dir[ISER_DIR_OUT]) {
-				desc = iser_task->rdma_reg[ISER_DIR_OUT].mem_h;
+				desc = iser_task->rdma_reg[ISER_DIR_OUT].desc;
 				if (unlikely(iser_inv_desc(desc, rkey)))
 					return -EINVAL;
 			}
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
index 2738ec56c918..72a117cd6fd7 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -130,7 +130,7 @@ void iser_unreg_mem_fastreg(struct iscsi_iser_task *iser_task,
 	struct iser_fr_desc *desc;
 	struct ib_mr_status mr_status;
 
-	desc = reg->mem_h;
+	desc = reg->desc;
 	if (!desc)
 		return;
 
@@ -147,8 +147,8 @@ void iser_unreg_mem_fastreg(struct iscsi_iser_task *iser_task,
 		ib_check_mr_status(desc->rsc.sig_mr, IB_MR_CHECK_SIG_STATUS,
 				   &mr_status);
 	}
-	iser_reg_desc_put_fr(&iser_task->iser_conn->ib_conn, reg->mem_h);
-	reg->mem_h = NULL;
+	iser_reg_desc_put_fr(&iser_task->iser_conn->ib_conn, reg->desc);
+	reg->desc = NULL;
 }
 
 static void iser_set_dif_domain(struct scsi_cmnd *sc,
@@ -361,7 +361,7 @@ int iser_reg_mem_fastreg(struct iscsi_iser_task *task,
 		desc->sig_protected = true;
 	}
 
-	reg->mem_h = desc;
+	reg->desc = desc;
 
 	return 0;
 
diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c
index 8bf87b073d9b..c7607b22a396 100644
--- a/drivers/infiniband/ulp/iser/iser_verbs.c
+++ b/drivers/infiniband/ulp/iser/iser_verbs.c
@@ -905,7 +905,7 @@ u8 iser_check_task_pi_status(struct iscsi_iser_task *iser_task,
 			     enum iser_data_dir cmd_dir, sector_t *sector)
 {
 	struct iser_mem_reg *reg = &iser_task->rdma_reg[cmd_dir];
-	struct iser_fr_desc *desc = reg->mem_h;
+	struct iser_fr_desc *desc = reg->desc;
 	unsigned long sector_size = iser_task->sc->device->sector_size;
 	struct ib_mr_status mr_status;
 	int ret;
-- 
2.18.1

[PATCH 3/4] IB/iser: generalize map/unmap dma tasks

[Max Gurtovoy]

Avoid code duplication and add the mapping/unmapping of the protection
buffers to the iser_dma_map_task_data/iser_dma_unmap_task_data
functions.

Reviewed-by: Sergey Gorenko <sergeygo@nvidia.com>
Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
---
 drivers/infiniband/ulp/iser/iscsi_iser.h     |  5 +--
 drivers/infiniband/ulp/iser/iser_initiator.c | 40 +-------------------
 drivers/infiniband/ulp/iser/iser_memory.c    | 31 +++++++++++++--
 3 files changed, 31 insertions(+), 45 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 23b922233006..7e4faf9c5e9e 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -531,13 +531,12 @@ int  iser_post_recvm(struct iser_conn *iser_conn,
 int  iser_post_send(struct ib_conn *ib_conn, struct iser_tx_desc *tx_desc);
 
 int iser_dma_map_task_data(struct iscsi_iser_task *iser_task,
-			   struct iser_data_buf *data,
 			   enum iser_data_dir iser_dir,
 			   enum dma_data_direction dma_dir);
 
 void iser_dma_unmap_task_data(struct iscsi_iser_task *iser_task,
-			      struct iser_data_buf *data,
-			      enum dma_data_direction dir);
+			      enum iser_data_dir iser_dir,
+			      enum dma_data_direction dma_dir);
 
 int  iser_initialize_task_headers(struct iscsi_task *task,
 			struct iser_tx_desc *tx_desc);
diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c b/drivers/infiniband/ulp/iser/iser_initiator.c
index 012decf6905a..dbc2c268bc0e 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -52,26 +52,13 @@ static int iser_prepare_read_cmd(struct iscsi_task *task)
 	struct iser_mem_reg *mem_reg;
 	int err;
 	struct iser_ctrl *hdr = &iser_task->desc.iser_header;
-	struct iser_data_buf *buf_in = &iser_task->data[ISER_DIR_IN];
 
 	err = iser_dma_map_task_data(iser_task,
-				     buf_in,
 				     ISER_DIR_IN,
 				     DMA_FROM_DEVICE);
 	if (err)
 		return err;
 
-	if (scsi_prot_sg_count(iser_task->sc)) {
-		struct iser_data_buf *pbuf_in = &iser_task->prot[ISER_DIR_IN];
-
-		err = iser_dma_map_task_data(iser_task,
-					     pbuf_in,
-					     ISER_DIR_IN,
-					     DMA_FROM_DEVICE);
-		if (err)
-			return err;
-	}
-
 	err = iser_reg_mem_fastreg(iser_task, ISER_DIR_IN, false);
 	if (err) {
 		iser_err("Failed to set up Data-IN RDMA\n");
@@ -106,23 +93,11 @@ static int iser_prepare_write_cmd(struct iscsi_task *task, unsigned int imm_sz,
 	struct ib_sge *tx_dsg = &iser_task->desc.tx_sg[1];
 
 	err = iser_dma_map_task_data(iser_task,
-				     buf_out,
 				     ISER_DIR_OUT,
 				     DMA_TO_DEVICE);
 	if (err)
 		return err;
 
-	if (scsi_prot_sg_count(iser_task->sc)) {
-		struct iser_data_buf *pbuf_out = &iser_task->prot[ISER_DIR_OUT];
-
-		err = iser_dma_map_task_data(iser_task,
-					     pbuf_out,
-					     ISER_DIR_OUT,
-					     DMA_TO_DEVICE);
-		if (err)
-			return err;
-	}
-
 	err = iser_reg_mem_fastreg(iser_task, ISER_DIR_OUT,
 				   buf_out->data_len == imm_sz);
 	if (err != 0) {
@@ -740,27 +715,16 @@ void iser_task_rdma_init(struct iscsi_iser_task *iser_task)
 
 void iser_task_rdma_finalize(struct iscsi_iser_task *iser_task)
 {
-	int prot_count = scsi_prot_sg_count(iser_task->sc);
 
 	if (iser_task->dir[ISER_DIR_IN]) {
 		iser_unreg_mem_fastreg(iser_task, ISER_DIR_IN);
-		iser_dma_unmap_task_data(iser_task,
-					 &iser_task->data[ISER_DIR_IN],
+		iser_dma_unmap_task_data(iser_task, ISER_DIR_IN,
 					 DMA_FROM_DEVICE);
-		if (prot_count)
-			iser_dma_unmap_task_data(iser_task,
-						 &iser_task->prot[ISER_DIR_IN],
-						 DMA_FROM_DEVICE);
 	}
 
 	if (iser_task->dir[ISER_DIR_OUT]) {
 		iser_unreg_mem_fastreg(iser_task, ISER_DIR_OUT);
-		iser_dma_unmap_task_data(iser_task,
-					 &iser_task->data[ISER_DIR_OUT],
+		iser_dma_unmap_task_data(iser_task, ISER_DIR_OUT,
 					 DMA_TO_DEVICE);
-		if (prot_count)
-			iser_dma_unmap_task_data(iser_task,
-						 &iser_task->prot[ISER_DIR_OUT],
-						 DMA_TO_DEVICE);
 	}
 }
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c
index 72a117cd6fd7..d72384e029a0 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -71,10 +71,10 @@ static void iser_reg_desc_put_fr(struct ib_conn *ib_conn,
 }
 
 int iser_dma_map_task_data(struct iscsi_iser_task *iser_task,
-			   struct iser_data_buf *data,
 			   enum iser_data_dir iser_dir,
 			   enum dma_data_direction dma_dir)
 {
+	struct iser_data_buf *data = &iser_task->data[iser_dir];
 	struct ib_device *dev;
 
 	iser_task->dir[iser_dir] = 1;
@@ -85,17 +85,40 @@ int iser_dma_map_task_data(struct iscsi_iser_task *iser_task,
 		iser_err("dma_map_sg failed!!!\n");
 		return -EINVAL;
 	}
+
+	if (scsi_prot_sg_count(iser_task->sc)) {
+		struct iser_data_buf *pdata = &iser_task->prot[iser_dir];
+
+		pdata->dma_nents = ib_dma_map_sg(dev, pdata->sg, pdata->size, dma_dir);
+		if (unlikely(pdata->dma_nents == 0)) {
+			iser_err("protection dma_map_sg failed!!!\n");
+			goto out_unmap;
+		}
+	}
+
 	return 0;
+
+out_unmap:
+	ib_dma_unmap_sg(dev, data->sg, data->size, dma_dir);
+	return -EINVAL;
 }
 
+
 void iser_dma_unmap_task_data(struct iscsi_iser_task *iser_task,
-			      struct iser_data_buf *data,
-			      enum dma_data_direction dir)
+			      enum iser_data_dir iser_dir,
+			      enum dma_data_direction dma_dir)
 {
+	struct iser_data_buf *data = &iser_task->data[iser_dir];
 	struct ib_device *dev;
 
 	dev = iser_task->iser_conn->ib_conn.device->ib_device;
-	ib_dma_unmap_sg(dev, data->sg, data->size, dir);
+	ib_dma_unmap_sg(dev, data->sg, data->size, dma_dir);
+
+	if (scsi_prot_sg_count(iser_task->sc)) {
+		struct iser_data_buf *pdata = &iser_task->prot[iser_dir];
+
+		ib_dma_unmap_sg(dev, pdata->sg, pdata->size, dma_dir);
+	}
 }
 
 static int iser_reg_dma(struct iser_device *device, struct iser_data_buf *mem,
-- 
2.18.1

[PATCH 4/4] IB/iser: fix error flow in case of registration failure

[Max Gurtovoy]

During READ/WRITE preparation, in case of failure in memory registration
using iser_reg_mem_fastreg we must unmap previously mapped iser task.

Reviewed-by: Sergey Gorenko <sergeygo@nvidia.com>
Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
---
 drivers/infiniband/ulp/iser/iser_initiator.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c b/drivers/infiniband/ulp/iser/iser_initiator.c
index dbc2c268bc0e..bd5f3b5e1727 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -62,7 +62,7 @@ static int iser_prepare_read_cmd(struct iscsi_task *task)
 	err = iser_reg_mem_fastreg(iser_task, ISER_DIR_IN, false);
 	if (err) {
 		iser_err("Failed to set up Data-IN RDMA\n");
-		return err;
+		goto out_err;
 	}
 	mem_reg = &iser_task->rdma_reg[ISER_DIR_IN];
 
@@ -75,6 +75,10 @@ static int iser_prepare_read_cmd(struct iscsi_task *task)
 		 (unsigned long long)mem_reg->sge.addr);
 
 	return 0;
+
+out_err:
+	iser_dma_unmap_task_data(iser_task, ISER_DIR_IN, DMA_FROM_DEVICE);
+	return err;
 }
 
 /* Register user buffer memory and initialize passive rdma
@@ -100,9 +104,9 @@ static int iser_prepare_write_cmd(struct iscsi_task *task, unsigned int imm_sz,
 
 	err = iser_reg_mem_fastreg(iser_task, ISER_DIR_OUT,
 				   buf_out->data_len == imm_sz);
-	if (err != 0) {
+	if (err) {
 		iser_err("Failed to register write cmd RDMA mem\n");
-		return err;
+		goto out_err;
 	}
 
 	mem_reg = &iser_task->rdma_reg[ISER_DIR_OUT];
@@ -129,6 +133,10 @@ static int iser_prepare_write_cmd(struct iscsi_task *task, unsigned int imm_sz,
 	}
 
 	return 0;
+
+out_err:
+	iser_dma_unmap_task_data(iser_task, ISER_DIR_OUT, DMA_TO_DEVICE);
+	return err;
 }
 
 /* creates a new tx descriptor and adds header regd buffer */
-- 
2.18.1

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

[Leon Romanovsky]

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
> 

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

[Max Gurtovoy]

On 2/15/2022 3:24 PM, Leon Romanovsky wrote:
> 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;
>
Right, nice catch.

I'll remove the check in v2.

>> +	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
>>