On Fri, Apr 23, 2021 at 12:13 AM Bob Pearson <rpearsonhpe@xxxxxxxxx> wrote:
>
> Simplify rxe_requester() by moving the local operations
> to a subroutine. Add an error return for illegal send WR opcode.
> Moved next_index ahead of rxe_run_task which fixed a small bug where
> work completions were delayed until after the next wqe which was not
> the intended behavior.
>
> Signed-off-by: Bob Pearson <rpearson@xxxxxxx>
> ---
> drivers/infiniband/sw/rxe/rxe_req.c | 89 +++++++++++++++++------------
> 1 file changed, 54 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
> index 0d4dcd514c55..0cf97e3db29f 100644
> --- a/drivers/infiniband/sw/rxe/rxe_req.c
> +++ b/drivers/infiniband/sw/rxe/rxe_req.c
> @@ -555,6 +555,56 @@ static void update_state(struct rxe_qp *qp, struct rxe_send_wqe *wqe,
> jiffies + qp->qp_timeout_jiffies);
> }
>
> +static int do_local_ops(struct rxe_qp *qp, struct rxe_send_wqe *wqe)
rxe_do_local_ops if not used out of softroce.
> +{
> + u8 opcode = wqe->wr.opcode;
> + struct rxe_dev *rxe;
> + struct rxe_mr *mr;
> + u32 rkey;
> +
> + switch (opcode) {
> + case IB_WR_LOCAL_INV:
> + rxe = to_rdev(qp->ibqp.device);
> + rkey = wqe->wr.ex.invalidate_rkey;
> + mr = rxe_pool_get_index(&rxe->mr_pool, rkey >> 8);
> + if (!mr) {
> + pr_err("No MR for rkey %#x\n", rkey);
> + wqe->state = wqe_state_error;
> + wqe->status = IB_WC_LOC_QP_OP_ERR;
> + return -EINVAL;
> + }
> + mr->state = RXE_MR_STATE_FREE;
> + rxe_drop_ref(mr);
> + break;
> + case IB_WR_REG_MR:
> + mr = to_rmr(wqe->wr.wr.reg.mr);
> +
> + rxe_add_ref(mr);
> + mr->state = RXE_MR_STATE_VALID;
> + mr->access = wqe->wr.wr.reg.access;
> + mr->ibmr.lkey = wqe->wr.wr.reg.key;
> + mr->ibmr.rkey = wqe->wr.wr.reg.key;
> + mr->iova = wqe->wr.wr.reg.mr->iova;
> + rxe_drop_ref(mr);
> + break;
> + default:
> + pr_err("Unexpected send wqe opcode %d\n", opcode);
> + wqe->state = wqe_state_error;
> + wqe->status = IB_WC_LOC_QP_OP_ERR;
> + return -EINVAL;
> + }
> +
> + wqe->state = wqe_state_done;
> + wqe->status = IB_WC_SUCCESS;
> + qp->req.wqe_index = next_index(qp->sq.queue, qp->req.wqe_index);
> +
> + if ((wqe->wr.send_flags & IB_SEND_SIGNALED) ||
> + qp->sq_sig_type == IB_SIGNAL_ALL_WR)
> + rxe_run_task(&qp->comp.task, 1);
> +
> + return 0;
> +}
> +
> int rxe_requester(void *arg)
> {
> struct rxe_qp *qp = (struct rxe_qp *)arg;
> @@ -594,42 +644,11 @@ int rxe_requester(void *arg)
> goto exit;
>
> if (wqe->mask & WR_LOCAL_OP_MASK) {
> - if (wqe->wr.opcode == IB_WR_LOCAL_INV) {
> - struct rxe_dev *rxe = to_rdev(qp->ibqp.device);
> - struct rxe_mr *rmr;
> -
> - rmr = rxe_pool_get_index(&rxe->mr_pool,
> - wqe->wr.ex.invalidate_rkey >> 8);
> - if (!rmr) {
> - pr_err("No mr for key %#x\n",
> - wqe->wr.ex.invalidate_rkey);
> - wqe->state = wqe_state_error;
> - wqe->status = IB_WC_MW_BIND_ERR;
> - goto exit;
> - }
> - rmr->state = RXE_MR_STATE_FREE;
> - rxe_drop_ref(rmr);
> - wqe->state = wqe_state_done;
> - wqe->status = IB_WC_SUCCESS;
> - } else if (wqe->wr.opcode == IB_WR_REG_MR) {
> - struct rxe_mr *rmr = to_rmr(wqe->wr.wr.reg.mr);
> -
> - rmr->state = RXE_MR_STATE_VALID;
> - rmr->access = wqe->wr.wr.reg.access;
> - rmr->ibmr.lkey = wqe->wr.wr.reg.key;
> - rmr->ibmr.rkey = wqe->wr.wr.reg.key;
> - rmr->iova = wqe->wr.wr.reg.mr->iova;
> - wqe->state = wqe_state_done;
> - wqe->status = IB_WC_SUCCESS;
> - } else {
> + ret = do_local_ops(qp, wqe);
> + if (unlikely(ret))
> goto exit;
> - }
> - if ((wqe->wr.send_flags & IB_SEND_SIGNALED) ||
> - qp->sq_sig_type == IB_SIGNAL_ALL_WR)
> - rxe_run_task(&qp->comp.task, 1);
> - qp->req.wqe_index = next_index(qp->sq.queue,
> - qp->req.wqe_index);
> - goto next_wqe;
> + else
> + goto next_wqe;
> }
>
> if (unlikely(qp_type(qp) == IB_QPT_RC &&
> --
> 2.27.0
>
