> -----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 13/18] RDMA/siw: Cleanup siw_accept
>
> With the initialization of rv and the two added label, we can
> simplifiy code a bit.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@xxxxxxxxx>
> ---
> drivers/infiniband/sw/siw/siw_cm.c | 41 ++++++++++--------------------
> 1 file changed, 14 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/infiniband/sw/siw/siw_cm.c
> b/drivers/infiniband/sw/siw/siw_cm.c
> index 1d2438fbf7c7..cff0fd7ceee6 100644
> --- a/drivers/infiniband/sw/siw/siw_cm.c
> +++ b/drivers/infiniband/sw/siw/siw_cm.c
> @@ -1548,7 +1548,7 @@ int siw_accept(struct iw_cm_id *id, struct
> iw_cm_conn_param *params)
> struct siw_cep *cep = (struct siw_cep *)id->provider_data;
> struct siw_qp *qp;
> struct siw_qp_attrs qp_attrs;
> - int rv, max_priv_data = MPA_MAX_PRIVDATA;
> + int rv = -EINVAL, max_priv_data = MPA_MAX_PRIVDATA;
> bool wait_for_peer_rts = false;
>
> siw_cep_set_inuse(cep);
> @@ -1564,24 +1564,17 @@ int siw_accept(struct iw_cm_id *id, struct
> iw_cm_conn_param *params)
>
> if (cep->state != SIW_EPSTATE_RECVD_MPAREQ) {
> siw_dbg_cep(cep, "out of state\n");
> -
> - siw_cep_set_free_and_put(cep);
> -
> - return -ECONNRESET;
> + rv = -ECONNRESET;
> + goto free_cep;
> }
> qp = siw_qp_id2obj(sdev, params->qpn);
> if (!qp) {
> WARN(1, "[QP %d] does not exist\n", params->qpn);
> - siw_cep_set_free_and_put(cep);
> -
> - return -EINVAL;
> + goto free_cep;
> }
> down_write(&qp->state_lock);
> - if (qp->attrs.state > SIW_QP_STATE_RTR) {
> - rv = -EINVAL;
> - up_write(&qp->state_lock);
> - goto error;
> - }
> + if (qp->attrs.state > SIW_QP_STATE_RTR)
> + goto error_unlock;
> siw_dbg_cep(cep, "[QP %d]\n", params->qpn);
>
> if (try_gso && cep->mpa.hdr.params.bits & MPA_RR_FLAG_GSO_EXP) {
> @@ -1595,9 +1588,7 @@ int siw_accept(struct iw_cm_id *id, struct
> iw_cm_conn_param *params)
> "[QP %u]: ord %d (max %d), ird %d (max %d)\n",
> qp_id(qp), params->ord, sdev->attrs.max_ord,
> params->ird, sdev->attrs.max_ird);
> - rv = -EINVAL;
> - up_write(&qp->state_lock);
> - goto error;
> + goto error_unlock;
> }
> if (cep->enhanced_rdma_conn_est)
> max_priv_data -= sizeof(struct mpa_v2_data);
> @@ -1607,9 +1598,7 @@ int siw_accept(struct iw_cm_id *id, struct
> iw_cm_conn_param *params)
> cep,
> "[QP %u]: private data length: %d (max %d)\n",
> qp_id(qp), params->private_data_len, max_priv_data);
> - rv = -EINVAL;
> - up_write(&qp->state_lock);
> - goto error;
> + goto error_unlock;
> }
> if (cep->enhanced_rdma_conn_est) {
> if (params->ord > cep->ord) {
> @@ -1618,9 +1607,7 @@ int siw_accept(struct iw_cm_id *id, struct
> iw_cm_conn_param *params)
> } else {
> cep->ird = params->ird;
> cep->ord = params->ord;
> - rv = -EINVAL;
> - up_write(&qp->state_lock);
> - goto error;
> + goto error_unlock;
> }
> }
> if (params->ird < cep->ird) {
> @@ -1629,8 +1616,7 @@ int siw_accept(struct iw_cm_id *id, struct
> iw_cm_conn_param *params)
> params->ird = cep->ird;
> else {
> rv = -ENOMEM;
> - up_write(&qp->state_lock);
> - goto error;
> + goto error_unlock;
> }
> }
> if (cep->mpa.v2_ctrl.ord &
> @@ -1677,7 +1663,6 @@ int siw_accept(struct iw_cm_id *id, struct
> iw_cm_conn_param *params)
> SIW_QP_ATTR_ORD | SIW_QP_ATTR_IRD |
> SIW_QP_ATTR_MPA);
> up_write(&qp->state_lock);
> -
> if (rv)
> goto error;
>
> @@ -1700,6 +1685,9 @@ int siw_accept(struct iw_cm_id *id, struct
> iw_cm_conn_param *params)
> siw_cep_set_free(cep);
>
> return 0;
> +
> +error_unlock:
> + up_write(&qp->state_lock);
> error:
> siw_socket_disassoc(cep->sock);
> sock_release(cep->sock);
> @@ -1714,9 +1702,8 @@ int siw_accept(struct iw_cm_id *id, struct
> iw_cm_conn_param *params)
> }
> cep->qp = NULL;
> siw_qp_put(qp);
> -
> +free_cep:
> siw_cep_set_free_and_put(cep);
> -
> return rv;
> }
>
> --
> 2.35.3
Looks good.
Acked-by: Bernard Metzler <bmt@xxxxxxxxxxxxxx>
