| Message ID | 1627583182-81330-1-git-send-email-mike.marciniszyn@cornelisnetworks.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | [for-rc] RDMA/cma: Revert INIT-INIT patch | expand |
On Thu, Jul 29, 2021 at 02:26:22PM -0400, mike.marciniszyn@cornelisnetworks.com wrote: > From: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com> > > The net/sunrpc/xprtrdma module creates its QP using rdma_create_qp() and > immediately post receives, implicitly assuming the QP is in the INIT > state and thus valid for ib_post_recv(). > > The patch noted in Fixes: removed the RESET->INIT modifiy from > rdma_create_qp(), breaking NFS rdma for verbs providers that fail the > ib_post_recv() for a bad state. > > This situation was proven using kprobes in rvt_post_recv() and > rvt_modify_qp(). The traces showed that the rvt_post_recv() failed before > ANY modify QP and that the current state was RESET. > > Fix by reverting the patch below. > > Fixes: dc70f7c3ed34 ("RDMA/cma: Remove unnecessary INIT->INIT transition") > Cc: Haakon Bugge <haakon.bugge@oracle.com> > Cc: Chuck Lever III <chuck.lever@oracle.com> > Signed-off-by: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com> > --- > drivers/infiniband/core/cma.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) Applied to for-rc, thanks Jason
> On 29 Jul 2021, at 20:26, mike.marciniszyn@cornelisnetworks.com wrote: > > From: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com> > > The net/sunrpc/xprtrdma module creates its QP using rdma_create_qp() and > immediately post receives, implicitly assuming the QP is in the INIT > state and thus valid for ib_post_recv(). > > The patch noted in Fixes: removed the RESET->INIT modifiy from > rdma_create_qp(), breaking NFS rdma for verbs providers that fail the > ib_post_recv() for a bad state. > > This situation was proven using kprobes in rvt_post_recv() and > rvt_modify_qp(). The traces showed that the rvt_post_recv() failed before > ANY modify QP and that the current state was RESET. > > Fix by reverting the patch below. > > Fixes: dc70f7c3ed34 ("RDMA/cma: Remove unnecessary INIT->INIT transition") > Cc: Haakon Bugge <haakon.bugge@oracle.com> > Cc: Chuck Lever III <chuck.lever@oracle.com> > Signed-off-by: Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com> Reviewed-by: Håkon Bugge <haakon.bugge@oracle.com> Thxs, Håkon > --- > drivers/infiniband/core/cma.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c > index 515a7e9..5d3b8b8 100644 > --- a/drivers/infiniband/core/cma.c > +++ b/drivers/infiniband/core/cma.c > @@ -926,12 +926,25 @@ static int cma_init_ud_qp(struct rdma_id_private *id_priv, struct ib_qp *qp) > return ret; > } > > +static int cma_init_conn_qp(struct rdma_id_private *id_priv, struct ib_qp *qp) > +{ > + struct ib_qp_attr qp_attr; > + int qp_attr_mask, ret; > + > + qp_attr.qp_state = IB_QPS_INIT; > + ret = rdma_init_qp_attr(&id_priv->id, &qp_attr, &qp_attr_mask); > + if (ret) > + return ret; > + > + return ib_modify_qp(qp, &qp_attr, qp_attr_mask); > +} > + > int rdma_create_qp(struct rdma_cm_id *id, struct ib_pd *pd, > struct ib_qp_init_attr *qp_init_attr) > { > struct rdma_id_private *id_priv; > struct ib_qp *qp; > - int ret = 0; > + int ret; > > id_priv = container_of(id, struct rdma_id_private, id); > if (id->device != pd->device) { > @@ -948,6 +961,8 @@ int rdma_create_qp(struct rdma_cm_id *id, struct ib_pd *pd, > > if (id->qp_type == IB_QPT_UD) > ret = cma_init_ud_qp(id_priv, qp); > + else > + ret = cma_init_conn_qp(id_priv, qp); > if (ret) > goto out_destroy; > > -- > 1.8.3.1 >
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c index 515a7e9..5d3b8b8 100644 --- a/drivers/infiniband/core/cma.c +++ b/drivers/infiniband/core/cma.c @@ -926,12 +926,25 @@ static int cma_init_ud_qp(struct rdma_id_private *id_priv, struct ib_qp *qp) return ret; } +static int cma_init_conn_qp(struct rdma_id_private *id_priv, struct ib_qp *qp) +{ + struct ib_qp_attr qp_attr; + int qp_attr_mask, ret; + + qp_attr.qp_state = IB_QPS_INIT; + ret = rdma_init_qp_attr(&id_priv->id, &qp_attr, &qp_attr_mask); + if (ret) + return ret; + + return ib_modify_qp(qp, &qp_attr, qp_attr_mask); +} + int rdma_create_qp(struct rdma_cm_id *id, struct ib_pd *pd, struct ib_qp_init_attr *qp_init_attr) { struct rdma_id_private *id_priv; struct ib_qp *qp; - int ret = 0; + int ret; id_priv = container_of(id, struct rdma_id_private, id); if (id->device != pd->device) { @@ -948,6 +961,8 @@ int rdma_create_qp(struct rdma_cm_id *id, struct ib_pd *pd, if (id->qp_type == IB_QPT_UD) ret = cma_init_ud_qp(id_priv, qp); + else + ret = cma_init_conn_qp(id_priv, qp); if (ret) goto out_destroy;