* [PATCH for-next] qedr: Move variables reset to qedr_set_common_qp_params()
@ 2021-08-11 5:16 Prabhakar Kushwaha
2021-08-11 11:44 ` Jason Gunthorpe
2021-08-19 17:28 ` Jason Gunthorpe
0 siblings, 2 replies; 5+ messages in thread
From: Prabhakar Kushwaha @ 2021-08-11 5:16 UTC (permalink / raw)
To: linux-rdma, dledford, jgg, mkalderon
Cc: davem, kuba, smalin, aelior, pkushwaha, prabhakar.pkin, malin1024
Qedr code is tightly coupled with existing both INIT transitions.
Here, during first INIT transition all variables are reset and
the RESET state is checked in post_recv() before any posting.
Commit dc70f7c3ed34 ("RDMA/cma: Remove unnecessary INIT->INIT
transition") exposed this bug.
So moving variables reset to qedr_set_common_qp_params() and also
avoid RESET state check for post_recv().
Signed-off-by: Michal Kalderon <mkalderon@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
Signed-off-by: Shai Malin <smalin@marvell.com>
Signed-off-by: Prabhakar Kushwaha <pkushwaha@marvell.com>
---
drivers/infiniband/hw/qedr/verbs.c | 32 +++++++++++++-----------------
1 file changed, 14 insertions(+), 18 deletions(-)
diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c
index fdc47ef7d861..6b3c7bfbd3bd 100644
--- a/drivers/infiniband/hw/qedr/verbs.c
+++ b/drivers/infiniband/hw/qedr/verbs.c
@@ -1339,6 +1339,15 @@ static int qedr_copy_qp_uresp(struct qedr_dev *dev,
return rc;
}
+static void qedr_reset_qp_hwq_info(struct qedr_qp_hwq_info *qph)
+{
+ qed_chain_reset(&qph->pbl);
+ qph->prod = 0;
+ qph->cons = 0;
+ qph->wqe_cons = 0;
+ qph->db_data.data.value = cpu_to_le16(0);
+}
+
static void qedr_set_common_qp_params(struct qedr_dev *dev,
struct qedr_qp *qp,
struct qedr_pd *pd,
@@ -1354,9 +1363,13 @@ static void qedr_set_common_qp_params(struct qedr_dev *dev,
qp->qp_type = attrs->qp_type;
qp->max_inline_data = attrs->cap.max_inline_data;
qp->state = QED_ROCE_QP_STATE_RESET;
+
+ qp->prev_wqe_size = 0;
+
qp->signaled = (attrs->sq_sig_type == IB_SIGNAL_ALL_WR) ? true : false;
qp->dev = dev;
if (qedr_qp_has_sq(qp)) {
+ qedr_reset_qp_hwq_info(&qp->sq);
qp->sq.max_sges = attrs->cap.max_send_sge;
qp->sq_cq = get_qedr_cq(attrs->send_cq);
DP_DEBUG(dev, QEDR_MSG_QP,
@@ -1368,6 +1381,7 @@ static void qedr_set_common_qp_params(struct qedr_dev *dev,
qp->srq = get_qedr_srq(attrs->srq);
if (qedr_qp_has_rq(qp)) {
+ qedr_reset_qp_hwq_info(&qp->rq);
qp->rq_cq = get_qedr_cq(attrs->recv_cq);
qp->rq.max_sges = attrs->cap.max_recv_sge;
DP_DEBUG(dev, QEDR_MSG_QP,
@@ -2359,15 +2373,6 @@ static enum qed_roce_qp_state qedr_get_state_from_ibqp(
}
}
-static void qedr_reset_qp_hwq_info(struct qedr_qp_hwq_info *qph)
-{
- qed_chain_reset(&qph->pbl);
- qph->prod = 0;
- qph->cons = 0;
- qph->wqe_cons = 0;
- qph->db_data.data.value = cpu_to_le16(0);
-}
-
static int qedr_update_qp_state(struct qedr_dev *dev,
struct qedr_qp *qp,
enum qed_roce_qp_state cur_state,
@@ -2382,9 +2387,6 @@ static int qedr_update_qp_state(struct qedr_dev *dev,
case QED_ROCE_QP_STATE_RESET:
switch (new_state) {
case QED_ROCE_QP_STATE_INIT:
- qp->prev_wqe_size = 0;
- qedr_reset_qp_hwq_info(&qp->sq);
- qedr_reset_qp_hwq_info(&qp->rq);
break;
default:
status = -EINVAL;
@@ -3915,12 +3917,6 @@ int qedr_post_recv(struct ib_qp *ibqp, const struct ib_recv_wr *wr,
spin_lock_irqsave(&qp->q_lock, flags);
- if (qp->state == QED_ROCE_QP_STATE_RESET) {
- spin_unlock_irqrestore(&qp->q_lock, flags);
- *bad_wr = wr;
- return -EINVAL;
- }
-
while (wr) {
int i;
--
2.24.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH for-next] qedr: Move variables reset to qedr_set_common_qp_params()
2021-08-11 5:16 [PATCH for-next] qedr: Move variables reset to qedr_set_common_qp_params() Prabhakar Kushwaha
@ 2021-08-11 11:44 ` Jason Gunthorpe
2021-08-11 11:50 ` Haakon Bugge
2021-08-12 9:44 ` Prabhakar Kushwaha
2021-08-19 17:28 ` Jason Gunthorpe
1 sibling, 2 replies; 5+ messages in thread
From: Jason Gunthorpe @ 2021-08-11 11:44 UTC (permalink / raw)
To: Prabhakar Kushwaha
Cc: linux-rdma, dledford, mkalderon, davem, kuba, smalin, aelior,
prabhakar.pkin, malin1024
On Wed, Aug 11, 2021 at 08:16:50AM +0300, Prabhakar Kushwaha wrote:
> Qedr code is tightly coupled with existing both INIT transitions.
> Here, during first INIT transition all variables are reset and
> the RESET state is checked in post_recv() before any posting.
>
> Commit dc70f7c3ed34 ("RDMA/cma: Remove unnecessary INIT->INIT
> transition") exposed this bug.
Since we are reverting this the patch still makse sense? Certainly
having a driver depend on two init->init transitions seems wrong to me
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH for-next] qedr: Move variables reset to qedr_set_common_qp_params()
2021-08-11 11:44 ` Jason Gunthorpe
@ 2021-08-11 11:50 ` Haakon Bugge
2021-08-12 9:44 ` Prabhakar Kushwaha
1 sibling, 0 replies; 5+ messages in thread
From: Haakon Bugge @ 2021-08-11 11:50 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Prabhakar Kushwaha, OFED mailing list, dledford, mkalderon,
davem, kuba, smalin, aelior, prabhakar.pkin, malin1024
> On 11 Aug 2021, at 13:44, Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> On Wed, Aug 11, 2021 at 08:16:50AM +0300, Prabhakar Kushwaha wrote:
>> Qedr code is tightly coupled with existing both INIT transitions.
>> Here, during first INIT transition all variables are reset and
>> the RESET state is checked in post_recv() before any posting.
>>
>> Commit dc70f7c3ed34 ("RDMA/cma: Remove unnecessary INIT->INIT
>> transition") exposed this bug.
>
>
> Since we are reverting this the patch still makse sense? Certainly
> having a driver depend on two init->init transitions seems wrong to me
If what I wrote about adhering to the IBTA spec and transition the QP to INIT during rdma_{connect,accept} makes sense, I think we need a two-phased approach.
1. Get the ULPs to adhere. That would not demand any changes in cma.
2. Once 1 has been proven working, we can fix cma and make the transition to INIT in rdma_{connect,accept}.
OK?
Thxs, Håkon
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH for-next] qedr: Move variables reset to qedr_set_common_qp_params()
2021-08-11 11:44 ` Jason Gunthorpe
2021-08-11 11:50 ` Haakon Bugge
@ 2021-08-12 9:44 ` Prabhakar Kushwaha
1 sibling, 0 replies; 5+ messages in thread
From: Prabhakar Kushwaha @ 2021-08-12 9:44 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Prabhakar Kushwaha, linux-rdma, dledford, Michal Kalderon,
David Miller, Jakub Kicinski, Shai Malin, Ariel Elior,
Shai Malin
Dear Jason,
On Wed, Aug 11, 2021 at 5:15 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
>
> On Wed, Aug 11, 2021 at 08:16:50AM +0300, Prabhakar Kushwaha wrote:
> > Qedr code is tightly coupled with existing both INIT transitions.
> > Here, during first INIT transition all variables are reset and
> > the RESET state is checked in post_recv() before any posting.
> >
> > Commit dc70f7c3ed34 ("RDMA/cma: Remove unnecessary INIT->INIT
> > transition") exposed this bug.
>
>
> Since we are reverting this the patch still makse sense? Certainly
> having a driver depend on two init->init transitions seems wrong to me
>
Yes, definitely still makes sense, thanks.
-- prabhakar (pk)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH for-next] qedr: Move variables reset to qedr_set_common_qp_params()
2021-08-11 5:16 [PATCH for-next] qedr: Move variables reset to qedr_set_common_qp_params() Prabhakar Kushwaha
2021-08-11 11:44 ` Jason Gunthorpe
@ 2021-08-19 17:28 ` Jason Gunthorpe
1 sibling, 0 replies; 5+ messages in thread
From: Jason Gunthorpe @ 2021-08-19 17:28 UTC (permalink / raw)
To: Prabhakar Kushwaha
Cc: linux-rdma, dledford, mkalderon, davem, kuba, smalin, aelior,
prabhakar.pkin, malin1024
On Wed, Aug 11, 2021 at 08:16:50AM +0300, Prabhakar Kushwaha wrote:
> Qedr code is tightly coupled with existing both INIT transitions.
> Here, during first INIT transition all variables are reset and
> the RESET state is checked in post_recv() before any posting.
>
> Commit dc70f7c3ed34 ("RDMA/cma: Remove unnecessary INIT->INIT
> transition") exposed this bug.
>
> So moving variables reset to qedr_set_common_qp_params() and also
> avoid RESET state check for post_recv().
>
> Signed-off-by: Michal Kalderon <mkalderon@marvell.com>
> Signed-off-by: Ariel Elior <aelior@marvell.com>
> Signed-off-by: Shai Malin <smalin@marvell.com>
> Signed-off-by: Prabhakar Kushwaha <pkushwaha@marvell.com>
> ---
> drivers/infiniband/hw/qedr/verbs.c | 32 +++++++++++++-----------------
> 1 file changed, 14 insertions(+), 18 deletions(-)
Applied to for-next, thanks
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-08-19 17:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-11 5:16 [PATCH for-next] qedr: Move variables reset to qedr_set_common_qp_params() Prabhakar Kushwaha
2021-08-11 11:44 ` Jason Gunthorpe
2021-08-11 11:50 ` Haakon Bugge
2021-08-12 9:44 ` Prabhakar Kushwaha
2021-08-19 17:28 ` Jason Gunthorpe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).