new ib_drain_qp() API

From: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+p6NamaJ0bNTAL8bYrjMMd8@public.gmane.org>

Add provider-specific drain_qp function for providers needing special drain logic.

Add static function __ib_drain_qp() which posts noop WRs to the RQ and SQ and blocks until their completions are processed. This ensures the applications completions have all been processed.

Add API function ib_drain_qp() which calls the provider-specific drain if it exists or __ib_drain_qp().

Signed-off-by Steve Wise <[email protected]>

---
drivers/infiniband/core/verbs.c | 72 +++++++++++++++++++++++++++++++++++++++++
include/rdma/ib_verbs.h | 2 ++

2 files changed, 74 insertions(+)

drivers/infiniband/core/verbs.c +21 -0

index 5af6d02..31b82cd 100644
@@ -1657,3 +1657,75 @@ next_page:
 	return i;
 }
 EXPORT_SYMBOL(ib_sg_to_pages);
+
+struct ib_drain_cqe {
+	struct ib_cqe cqe;
+	struct completion done;
+};
+
+static void ib_drain_qp_done(struct ib_cq *cq, struct ib_wc *wc)
+{
+	struct ib_drain_cqe *cqe = container_of(wc->wr_cqe, struct ib_drain_cqe,
+						cqe);
+
+	complete(&cqe->done);
+}
+
+/*
+ * Post a WR and block until its completion is reaped for both the RQ and SQ.
+ */
+static void __ib_drain_qp(struct ib_qp *qp)
+{
+	struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR };
+	struct ib_drain_cqe rdrain, sdrain;

drivers/infiniband/core/verbs.c +41 -0

+	init_completion(&sdrain.done);
+
+	ret = ib_modify_qp(qp, &attr, IB_QP_STATE);
+	if (ret) {
+		WARN_ONCE(ret, "failed to drain QP: %d\n", ret);
+		return;
+	}
+
+	ret = ib_post_recv(qp, &rwr, &bad_rwr);
+	if (ret) {
+		WARN_ONCE(ret, "failed to drain recv queue: %d\n", ret);
+		return;
+	}
+
+	ret = ib_post_send(qp, &swr, &bad_swr);
+	if (ret) {
+		WARN_ONCE(ret, "failed to drain send queue: %d\n", ret);
+		return;
+	}
+
+	wait_for_completion(&rdrain.done);
+	wait_for_completion(&sdrain.done);
+}
+
+/**
+ * ib_drain_qp() - Block until all CQEs have been consumed by the
+ *		   application.
+ * @qp:            queue pair to drain
+ *
+ * If the device has a provider-specific drain function, then
+ * call that.  Otherwise call the generic drain function
+ * __ib_drain_qp().
+ */
+void ib_drain_qp(struct ib_qp *qp)
+{
+	if (qp->device->drain_qp)
+		qp->device->drain_qp(qp);
+	else
+		__ib_drain_qp(qp);
+}
+EXPORT_SYMBOL(ib_drain_qp);

include/rdma/ib_verbs.h +2 -0

index 284b00c..d8533ab 100644
@@ -1846,6 +1846,7 @@ struct ib_device {
 	int			   (*check_mr_status)(struct ib_mr *mr, u32 check_mask,
 						      struct ib_mr_status *mr_status);
 	void			   (*disassociate_ucontext)(struct ib_ucontext *ibcontext);
+	void			   (*drain_qp)(struct ib_qp *qp);
 
 	struct ib_dma_mapping_ops   *dma_ops;
 

@@ -3094,4 +3095,5 @@ int ib_sg_to_pages(struct ib_mr *mr,
 		   int sg_nents,
 		   int (*set_page)(struct ib_mr *, u64));
 
+void ib_drain_qp(struct ib_qp *qp);
 #endif /* IB_VERBS_H */

Signature · 6 lines

2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 53 lines

From: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+p6NamaJ0bNTAL8bYrjMMd8@public.gmane.org>

Add provider-specific drain_qp function for providers needing special drain logic.

Add static function __ib_drain_qp() which posts noop WRs to the RQ and SQ and blocks until their completions are processed. This ensures the applications completions have all been processed.

Add API function ib_drain_qp() which calls the provider-specific drain if it exists or __ib_drain_qp().

Signed-off-by Steve Wise <[email protected]>

---
drivers/infiniband/core/verbs.c | 72 +++++++++++++++++++++++++++++++++++++++++
include/rdma/ib_verbs.h | 2 ++

2 files changed, 74 insertions(+)

drivers/infiniband/core/verbs.c +27 -0

index 5af6d02..31b82cd 100644
@@ -1657,3 +1657,75 @@ next_page:
 	return i;
 }
 EXPORT_SYMBOL(ib_sg_to_pages);
+
+struct ib_drain_cqe {
+	struct ib_cqe cqe;
+	struct completion done;
+};
+
+static void ib_drain_qp_done(struct ib_cq *cq, struct ib_wc *wc)
+{
+	struct ib_drain_cqe *cqe = container_of(wc->wr_cqe, struct ib_drain_cqe,
+						cqe);
+
+	complete(&cqe->done);
+}
+
+/*
+ * Post a WR and block until its completion is reaped for both the RQ and SQ.
+ */
+static void __ib_drain_qp(struct ib_qp *qp)
+{
+	struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR };
+	struct ib_drain_cqe rdrain, sdrain;
+	struct ib_recv_wr rwr = {}, *bad_rwr;
+	struct ib_send_wr swr = {}, *bad_swr;
+	int ret;
+
+	rwr.wr_cqe = &rdrain.cqe;
+	rdrain.cqe.done = ib_drain_qp_done;

drivers/infiniband/core/verbs.c next_page: · line 1690

Quoted 4 lines

drivers/infiniband/core/verbs.c +4 -0

+	init_completion(&rdrain.done);
+
+	swr.wr_cqe = &sdrain.cqe;
+	sdrain.cqe.done = ib_drain_qp_done;

OK. ib_cqe is what hooks the completion events for these blank WRs, so those completions are never exposed to the RDMA consumer.

But does a consumer have to bump its SQE and RQE count when allocating its CQs, or is that done automatically by ib_alloc_cq() ?

Quoted 54 lines

Quoted 54 lines

From: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+p6NamaJ0bNTAL8bYrjMMd8@public.gmane.org>

Add provider-specific drain_qp function for providers needing special drain logic.

Add static function __ib_drain_qp() which posts noop WRs to the RQ and SQ and blocks until their completions are processed. This ensures the applications completions have all been processed.

Add API function ib_drain_qp() which calls the provider-specific drain if it exists or __ib_drain_qp().

Signed-off-by Steve Wise <[email protected]>

---
drivers/infiniband/core/verbs.c | 72 +++++++++++++++++++++++++++++++++++++++++
include/rdma/ib_verbs.h | 2 ++

2 files changed, 74 insertions(+)

drivers/infiniband/core/verbs.c +27 -0

index 5af6d02..31b82cd 100644
@@ -1657,3 +1657,75 @@ next_page:
 	return i;
 }
 EXPORT_SYMBOL(ib_sg_to_pages);
+
+struct ib_drain_cqe {
+	struct ib_cqe cqe;
+	struct completion done;
+};
+
+static void ib_drain_qp_done(struct ib_cq *cq, struct ib_wc *wc)
+{
+	struct ib_drain_cqe *cqe = container_of(wc->wr_cqe, struct ib_drain_cqe,
+						cqe);
+
+	complete(&cqe->done);
+}
+
+/*
+ * Post a WR and block until its completion is reaped for both the RQ and SQ.
+ */
+static void __ib_drain_qp(struct ib_qp *qp)
+{
+	struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR };
+	struct ib_drain_cqe rdrain, sdrain;
+	struct ib_recv_wr rwr = {}, *bad_rwr;
+	struct ib_send_wr swr = {}, *bad_swr;
+	int ret;
+
+	rwr.wr_cqe = &rdrain.cqe;
+	rdrain.cqe.done = ib_drain_qp_done;

Quoted 9 lines

drivers/infiniband/core/verbs.c next_page: · line 1690

Quoted 4 lines

drivers/infiniband/core/verbs.c +4 -0

+	init_completion(&rdrain.done);
+
+	swr.wr_cqe = &sdrain.cqe;
+	sdrain.cqe.done = ib_drain_qp_done;

OK. ib_cqe is what hooks the completion events for these blank WRs, so those completions are never exposed to the RDMA consumer.

Right, which means only consumers that use the new style CQ processing can make use of this.

Quoted 4 lines

But does a consumer have to bump its SQE and RQE count when allocating its CQs, or is that done automatically by ib_alloc_cq() ?

The consumer has to make sure there is room in the SQ, RQ and CQ.

Going forward, we could enhance QP and CQ allocation to allow the
consumer to specify it wants drain capability so the consumer doesn't have to do this. It could be done under the covers. In fact, if we did that, then ib_destroy_qp() could do the drain if need be.

Quoted 74 lines · Steve Wise

Quoted 54 lines

Quoted 54 lines

From: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+p6NamaJ0bNTAL8bYrjMMd8@public.gmane.org>

Add provider-specific drain_qp function for providers needing special drain logic.

Add static function __ib_drain_qp() which posts noop WRs to the RQ and SQ and blocks until their completions are processed. This ensures the applications completions have all been processed.

Add API function ib_drain_qp() which calls the provider-specific drain if it exists or __ib_drain_qp().

Signed-off-by Steve Wise <[email protected]>

---
drivers/infiniband/core/verbs.c | 72 +++++++++++++++++++++++++++++++++++++++++
include/rdma/ib_verbs.h | 2 ++

2 files changed, 74 insertions(+)

drivers/infiniband/core/verbs.c +27 -0

index 5af6d02..31b82cd 100644
@@ -1657,3 +1657,75 @@ next_page:
 	return i;
 }
 EXPORT_SYMBOL(ib_sg_to_pages);
+
+struct ib_drain_cqe {
+	struct ib_cqe cqe;
+	struct completion done;
+};
+
+static void ib_drain_qp_done(struct ib_cq *cq, struct ib_wc *wc)
+{
+	struct ib_drain_cqe *cqe = container_of(wc->wr_cqe, struct ib_drain_cqe,
+						cqe);
+
+	complete(&cqe->done);
+}
+
+/*
+ * Post a WR and block until its completion is reaped for both the RQ and SQ.
+ */
+static void __ib_drain_qp(struct ib_qp *qp)
+{
+	struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR };
+	struct ib_drain_cqe rdrain, sdrain;
+	struct ib_recv_wr rwr = {}, *bad_rwr;
+	struct ib_send_wr swr = {}, *bad_swr;
+	int ret;
+
+	rwr.wr_cqe = &rdrain.cqe;
+	rdrain.cqe.done = ib_drain_qp_done;

Quoted 9 lines

drivers/infiniband/core/verbs.c next_page: · line 1690

Quoted 4 lines

drivers/infiniband/core/verbs.c +4 -0

+	init_completion(&rdrain.done);
+
+	swr.wr_cqe = &sdrain.cqe;
+	sdrain.cqe.done = ib_drain_qp_done;

OK. ib_cqe is what hooks the completion events for these blank WRs, so those completions are never exposed to the RDMA consumer.

Right, which means only consumers that use the new style CQ processing can make use of this.

Quoted 4 lines

But does a consumer have to bump its SQE and RQE count when allocating its CQs, or is that done automatically by ib_alloc_cq() ?

Quoted 1 line

The consumer has to make sure there is room in the SQ, RQ and CQ.

The documenting comment in front of ib_drain_qp() should mention this, and it should also state the requirement to use ib_alloc_cq().

Quoted 3 lines

Going forward, we could enhance QP and CQ allocation to allow the
consumer to specify it wants drain capability so the consumer doesn't have to do this. It could be done under the covers. In fact, if we did that, then ib_destroy_qp() could do the drain if need be.

--
Chuck Lever

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 12 lines

Quoted 4 lines

OK. ib_cqe is what hooks the completion events for these blank WRs, so those completions are never exposed to the RDMA consumer.

Right, which means only consumers that use the new style CQ processing can make use of this.

Quoted 4 lines

But does a consumer have to bump its SQE and RQE count when allocating its CQs, or is that done automatically by ib_alloc_cq() ?

Quoted 1 line

The consumer has to make sure there is room in the SQ, RQ and CQ.

The documenting comment in front of ib_drain_qp() should mention this, and it should also state the requirement to use ib_alloc_cq().

From: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+p6NamaJ0bNTAL8bYrjMMd8@public.gmane.org>

Add provider-specific drain_qp function for providers needing special drain logic.

Add static function __ib_drain_qp() which posts noop WRs to the RQ and SQ and blocks until their completions are processed. This ensures the applications completions have all been processed.

Add API function ib_drain_qp() which calls the provider-specific drain if it exists or __ib_drain_qp().

Signed-off-by Steve Wise <[email protected]>

---
 drivers/infiniband/core/verbs.c | 72 +++++++++++++++++++++++++++++++++++++++++
 include/rdma/ib_verbs.h         |  2 ++
 2 files changed, 74 insertions(+)

drivers/infiniband/core/verbs.c +28 -0

index 5af6d02..31b82cd 100644
@@ -1657,3 +1657,75 @@ next_page:
        return i;
 }
 EXPORT_SYMBOL(ib_sg_to_pages);
+
+struct ib_drain_cqe {
+       struct ib_cqe cqe;
+       struct completion done;
+};
+
+static void ib_drain_qp_done(struct ib_cq *cq, struct ib_wc *wc)
+{
+       struct ib_drain_cqe *cqe = container_of(wc->wr_cqe, struct ib_drain_cqe,
+                                               cqe);
+
+       complete(&cqe->done);
+}
+
+/*
+ * Post a WR and block until its completion is reaped for both the RQ and SQ.
+ */
+static void __ib_drain_qp(struct ib_qp *qp)
+{
+       struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR };
+       struct ib_drain_cqe rdrain, sdrain;
+       struct ib_recv_wr rwr = {}, *bad_rwr;
+       struct ib_send_wr swr = {}, *bad_swr;
+       int ret;
+
+       rwr.wr_cqe = &rdrain.cqe;
+       rdrain.cqe.done = ib_drain_qp_done;
+       init_completion(&rdrain.done);

drivers/infiniband/core/verbs.c +34 -0

+
+       ret = ib_post_recv(qp, &rwr, &bad_rwr);
+       if (ret) {
+               WARN_ONCE(ret, "failed to drain recv queue: %d\n", ret);
+               return;
+       }
+
+       ret = ib_post_send(qp, &swr, &bad_swr);
+       if (ret) {
+               WARN_ONCE(ret, "failed to drain send queue: %d\n", ret);
+               return;
+       }
+
+       wait_for_completion(&rdrain.done);
+       wait_for_completion(&sdrain.done);
+}
+
+/**
+ * ib_drain_qp() - Block until all CQEs have been consumed by the
+ *                application.
+ * @qp:            queue pair to drain
+ *
+ * If the device has a provider-specific drain function, then
+ * call that.  Otherwise call the generic drain function
+ * __ib_drain_qp().
+ */
+void ib_drain_qp(struct ib_qp *qp)
+{
+       if (qp->device->drain_qp)
+               qp->device->drain_qp(qp);
+       else
+               __ib_drain_qp(qp);
+}
+EXPORT_SYMBOL(ib_drain_qp);

include/rdma/ib_verbs.h +2 -1

index 284b00c..d8533ab 100644
@@ -1846,6 +1846,7 @@ struct ib_device {
        int                        (*check_mr_status)(struct ib_mr *mr, u32 check_mask,
                                                      struct ib_mr_status *mr_status);
        void                       (*disassociate_ucontext)(struct ib_ucontext *ibcontext);
+       void                       (*drain_qp)(struct ib_qp *qp);
 
        struct ib_dma_mapping_ops   *dma_ops;
 

@@ -3094,4 +3095,5 @@ int ib_sg_to_pages(struct ib_mr *mr,
                   int sg_nents,
                   int (*set_page)(struct ib_mr *, u64));
 
+void ib_drain_qp(struct ib_qp *qp);
 #endif /* IB_VERBS_H */
--

2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 23 lines

drivers/infiniband/core/verbs.c +22 -0

+/*
+ * Post a WR and block until its completion is reaped for both the RQ and SQ.
+ */
+static void __ib_drain_qp(struct ib_qp *qp)
+{
+       struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR };
+       struct ib_drain_cqe rdrain, sdrain;
+       struct ib_recv_wr rwr = {}, *bad_rwr;
+       struct ib_send_wr swr = {}, *bad_swr;
+       int ret;
+
+       rwr.wr_cqe = &rdrain.cqe;
+       rdrain.cqe.done = ib_drain_qp_done;
+       init_completion(&rdrain.done);
+
+       swr.wr_cqe = &sdrain.cqe;
+       sdrain.cqe.done = ib_drain_qp_done;
+       init_completion(&sdrain.done);
+
+       ret = ib_modify_qp(qp, &attr, IB_QP_STATE);
+       if (ret) {
+               WARN_ONCE(ret, "failed to drain QP: %d\n", ret);

Review

Quoted 2 lines

(quoted diff) +2 -0

+               return;
+       }

I was thinking if we really need this modify_qp here. generally drain_qp is called on
an error'ed out QP. In a graceful tear down rdma_disconnect takes care to modify-qp
to error. while in error state qp is already in error state.

Quoted 25 lines

Quoted 25 lines

+/*
+ * Post a WR and block until its completion is reaped for both the RQ and SQ.
+ */

drivers/infiniband/core/verbs.c +20 -0

+static void __ib_drain_qp(struct ib_qp *qp)
+{
+       struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR };
+       struct ib_drain_cqe rdrain, sdrain;
+       struct ib_recv_wr rwr = {}, *bad_rwr;
+       struct ib_send_wr swr = {}, *bad_swr;
+       int ret;
+
+       rwr.wr_cqe = &rdrain.cqe;
+       rdrain.cqe.done = ib_drain_qp_done;
+       init_completion(&rdrain.done);
+
+       swr.wr_cqe = &sdrain.cqe;
+       sdrain.cqe.done = ib_drain_qp_done;
+       init_completion(&sdrain.done);
+
+       ret = ib_modify_qp(qp, &attr, IB_QP_STATE);
+       if (ret) {
+               WARN_ONCE(ret, "failed to drain QP: %d\n", ret);
+               return;

Quoted 8 lines

Quoted 1 line

+ }

I was thinking if we really need this modify_qp here. generally drain_qp is called on
an error'ed out QP. In a graceful tear down rdma_disconnect takes care to modify-qp
to error. while in error state qp is already in error state.

We could get it conditional, but I don't see any harm done in having it as it would mean a passed in flag.

It would be better to have the modify_qp implementers do nothing for a ERROR -> ERROR modify...

Quoted 35 lines

Quoted 25 lines

Quoted 25 lines

+/*
+ * Post a WR and block until its completion is reaped for both the RQ and SQ.
+ */

(quoted diff) +20 -0

+static void __ib_drain_qp(struct ib_qp *qp)
+{
+       struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR };
+       struct ib_drain_cqe rdrain, sdrain;
+       struct ib_recv_wr rwr = {}, *bad_rwr;
+       struct ib_send_wr swr = {}, *bad_swr;
+       int ret;
+
+       rwr.wr_cqe = &rdrain.cqe;
+       rdrain.cqe.done = ib_drain_qp_done;
+       init_completion(&rdrain.done);
+
+       swr.wr_cqe = &sdrain.cqe;
+       sdrain.cqe.done = ib_drain_qp_done;
+       init_completion(&sdrain.done);
+
+       ret = ib_modify_qp(qp, &attr, IB_QP_STATE);
+       if (ret) {
+               WARN_ONCE(ret, "failed to drain QP: %d\n", ret);
+               return;

Quoted 8 lines

Quoted 1 line

+ }

I was thinking if we really need this modify_qp here. generally drain_qp is called on
an error'ed out QP. In a graceful tear down rdma_disconnect takes care to modify-qp
to error. while in error state qp is already in error state.

Quoted 3 lines

We could get it conditional, but I don't see any harm done in having it as it would mean a passed in flag.

Since Spec allows this, I would say okay let it be there.

Quoted 2 lines

It would be better to have the modify_qp implementers do nothing for a ERROR -> ERROR modify...

Driver implementer has a choice here which is okay.

Quoted 22 lines

Quoted 22 lines

drivers/infiniband/core/verbs.c +22 -0

+/*
+ * Post a WR and block until its completion is reaped for both the RQ and SQ.
+ */
+static void __ib_drain_qp(struct ib_qp *qp)
+{
+       struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR };
+       struct ib_drain_cqe rdrain, sdrain;
+       struct ib_recv_wr rwr = {}, *bad_rwr;
+       struct ib_send_wr swr = {}, *bad_swr;
+       int ret;
+
+       rwr.wr_cqe = &rdrain.cqe;
+       rdrain.cqe.done = ib_drain_qp_done;
+       init_completion(&rdrain.done);
+
+       swr.wr_cqe = &sdrain.cqe;
+       sdrain.cqe.done = ib_drain_qp_done;
+       init_completion(&sdrain.done);
+
+       ret = ib_modify_qp(qp, &attr, IB_QP_STATE);
+       if (ret) {
+               WARN_ONCE(ret, "failed to drain QP: %d\n", ret);

Quoted 8 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+               return;
+       }

I was thinking if we really need this modify_qp here. generally drain_qp is called on
an error'ed out QP. In a graceful tear down rdma_disconnect takes care to modify-qp
to error. while in error state qp is already in error state.

We could get it conditional, but I don't see any harm done in having it as it would mean a passed in flag.

It would be better to have the modify_qp implementers do nothing for a ERROR -> ERROR modify...

From: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+p6NamaJ0bNTAL8bYrjMMd8@public.gmane.org>

Add provider-specific drain_qp function for providers needing special drain logic.

Add static function __ib_drain_qp() which posts noop WRs to the RQ and SQ and blocks until their completions are processed. This ensures the applications completions have all been processed.

Add API function ib_drain_qp() which calls the provider-specific drain if it exists or __ib_drain_qp().

Signed-off-by Steve Wise <[email protected]>

---
 drivers/infiniband/core/verbs.c | 72 +++++++++++++++++++++++++++++++++++++++++
 include/rdma/ib_verbs.h         |  2 ++
 2 files changed, 74 insertions(+)

drivers/infiniband/core/verbs.c +6 -0

+	ret = ib_modify_qp(qp, &attr, IB_QP_STATE);
+	if (ret) {
+		WARN_ONCE(ret, "failed to drain QP: %d\n", ret);
+		return;
+	}
+

Quoted 13 lines

drivers/infiniband/core/verbs.c +12 -0

+	ret = ib_modify_qp(qp, &attr, IB_QP_STATE);
+	if (ret) {
+		WARN_ONCE(ret, "failed to drain QP: %d\n", ret);
+		return;
+	}
+
+	ret = ib_post_recv(qp, &rwr, &bad_rwr);
+	if (ret) {
+		WARN_ONCE(ret, "failed to drain recv queue: %d\n", ret);
+		return;
+	}
+

drivers/infiniband/core/verbs.c next_page: · line 1708

Quoted 4 lines

drivers/infiniband/core/verbs.c +4 -0

+	ret = ib_post_send(qp, &swr, &bad_swr);
+	if (ret) {
+		WARN_ONCE(ret, "failed to drain send queue: %d\n", ret);
+		return;

You are returning here despite the fact that recv queue drained successfully and you can wait for completion of rdrain safely. Is it done on purpose?

-----Original Message-----
From: Sagi Grimberg [mailto:[email protected]]
Sent: Sunday, February 07, 2016 5:52 AM
To: Steve Wise; [email protected]
Subject: Re: [PATCH 1/3] IB: new common API for draining a queue pair

Quoted 12 lines

Quoted 12 lines

drivers/infiniband/core/verbs.c +12 -0

+	ret = ib_modify_qp(qp, &attr, IB_QP_STATE);
+	if (ret) {
+		WARN_ONCE(ret, "failed to drain QP: %d\n", ret);
+		return;
+	}
+
+	ret = ib_post_recv(qp, &rwr, &bad_rwr);
+	if (ret) {
+		WARN_ONCE(ret, "failed to drain recv queue: %d\n", ret);
+		return;
+	}
+

Quoted 8 lines

drivers/infiniband/core/verbs.c next_page: · line 1708

Quoted 4 lines

drivers/infiniband/core/verbs.c +4 -0

+	ret = ib_post_send(qp, &swr, &bad_swr);
+	if (ret) {
+		WARN_ONCE(ret, "failed to drain send queue: %d\n", ret);
+		return;

You are returning here despite the fact that recv queue drained successfully and you can wait for completion of rdrain safely. Is it done on purpose?

Good catch, the completion structures are on-stack.

Steve, you need to wait for the completion of the
successful post in this case...