> -----Original Message-----
> From: Guoqing Jiang <guoqing.jiang@xxxxxxxxx>
> Sent: Friday, October 27, 2023 4:33 AM
> To: Bernard Metzler <BMT@xxxxxxxxxxxxxx>; jgg@xxxxxxxx; leon@xxxxxxxxxx
> Cc: linux-rdma@xxxxxxxxxxxxxxx
> Subject: [EXTERNAL] [PATCH V3 08/18] RDMA/siw: Factor out siw_generic_rx
> helper
>
> Remove the redundant code given they share the same logic.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@xxxxxxxxx>
> ---
> drivers/infiniband/sw/siw/siw_qp_rx.c | 53 ++++++++++-----------------
> 1 file changed, 20 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/infiniband/sw/siw/siw_qp_rx.c
> b/drivers/infiniband/sw/siw/siw_qp_rx.c
> index 10805a7d0487..2a6473a5abe0 100644
> --- a/drivers/infiniband/sw/siw/siw_qp_rx.c
> +++ b/drivers/infiniband/sw/siw/siw_qp_rx.c
> @@ -405,6 +405,20 @@ static struct siw_wqe *siw_rqe_get(struct siw_qp *qp)
> return wqe;
> }
>
Maybe better call it siw_rx_data()?
For a 'generic receive' I could imagine 1000 things.
You made an abstraction for receiving different types
of payload.
> +static int siw_generic_rx(struct siw_mem *mem_p, struct siw_rx_stream
> *srx,
> + unsigned int *pbl_idx, u64 addr, int bytes)
> +{
> + int rv;
> +
> + if (mem_p->mem_obj == NULL)
> + rv = siw_rx_kva(srx, ib_virt_dma_to_ptr(addr), bytes);
> + else if (!mem_p->is_pbl)
> + rv = siw_rx_umem(srx, mem_p->umem, addr, bytes);
> + else
> + rv = siw_rx_pbl(srx, pbl_idx, mem_p, addr, bytes);
> + return rv;
> +}
> +
> /*
> * siw_proc_send:
> *
> @@ -485,17 +499,8 @@ int siw_proc_send(struct siw_qp *qp)
> break;
> }
> mem_p = *mem;
> - if (mem_p->mem_obj == NULL)
> - rv = siw_rx_kva(srx,
> - ib_virt_dma_to_ptr(sge->laddr + frx->sge_off),
> - sge_bytes);
> - else if (!mem_p->is_pbl)
> - rv = siw_rx_umem(srx, mem_p->umem,
> - sge->laddr + frx->sge_off, sge_bytes);
> - else
> - rv = siw_rx_pbl(srx, &frx->pbl_idx, mem_p,
> - sge->laddr + frx->sge_off, sge_bytes);
> -
> + rv = siw_generic_rx(mem_p, srx, &frx->pbl_idx,
> + sge->laddr + frx->sge_off, sge_bytes);
> if (unlikely(rv != sge_bytes)) {
> wqe->processed += rcvd_bytes;
>
> @@ -598,17 +603,8 @@ int siw_proc_write(struct siw_qp *qp)
> return -EINVAL;
> }
>
> - if (mem->mem_obj == NULL)
> - rv = siw_rx_kva(srx,
> - (void *)(uintptr_t)(srx->ddp_to + srx->fpdu_part_rcvd),
> - bytes);
> - else if (!mem->is_pbl)
> - rv = siw_rx_umem(srx, mem->umem,
> - srx->ddp_to + srx->fpdu_part_rcvd, bytes);
> - else
> - rv = siw_rx_pbl(srx, &frx->pbl_idx, mem,
> - srx->ddp_to + srx->fpdu_part_rcvd, bytes);
> -
> + rv = siw_generic_rx(mem, srx, &frx->pbl_idx,
> + srx->ddp_to + srx->fpdu_part_rcvd, bytes);
> if (unlikely(rv != bytes)) {
> siw_init_terminate(qp, TERM_ERROR_LAYER_DDP,
> DDP_ETYPE_CATASTROPHIC,
> @@ -849,17 +845,8 @@ int siw_proc_rresp(struct siw_qp *qp)
> mem_p = *mem;
>
> bytes = min(srx->fpdu_part_rem, srx->skb_new);
> -
> - if (mem_p->mem_obj == NULL)
> - rv = siw_rx_kva(srx,
> - ib_virt_dma_to_ptr(sge->laddr + wqe->processed),
> - bytes);
> - else if (!mem_p->is_pbl)
> - rv = siw_rx_umem(srx, mem_p->umem, sge->laddr + wqe->processed,
> - bytes);
> - else
> - rv = siw_rx_pbl(srx, &frx->pbl_idx, mem_p,
> - sge->laddr + wqe->processed, bytes);
> + rv = siw_generic_rx(mem_p, srx, &frx->pbl_idx,
> + sge->laddr + wqe->processed, bytes);
> if (rv != bytes) {
> wqe->wc_status = SIW_WC_GENERAL_ERR;
> rv = -EINVAL;
> --
> 2.35.3
