Message ID | 20220215110632.10697-2-mgurtovoy@nvidia.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Leon Romanovsky |
Headers | show |
Series | iSER cleanups and fixes for 5.18 | expand |
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 >
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 >>
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; }