new ib_drain_qp() API

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

Quoted 9 lines · Steve Wise

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

+static void wait_for_drain(struct ib_cq *cq, struct completion *c)
+{
+    if (cq->poll_ctx == IB_POLL_DIRECT)
+        do
+            ib_process_cq_direct(cq, 1024);
+        while (!wait_for_completion_timeout(c, msecs_to_jiffies(100)));
+    else
+        wait_for_completion(c);
+}

Hello Steve,

Have you verified this patch with checkpatch ? I expect that checkpatch will report that six braces are missing from this function.

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Bart Van Assche
Sent: Thursday, February 11, 2016 5:16 PM
To: Steve Wise; [email protected]
Subject: Re: [PATCH 1/3] IB: new common API for draining queues

Quoted 13 lines · Bart Van Assche

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

Quoted 9 lines · Steve Wise

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

+static void wait_for_drain(struct ib_cq *cq, struct completion *c)
+{
+    if (cq->poll_ctx == IB_POLL_DIRECT)
+        do
+            ib_process_cq_direct(cq, 1024);
+        while (!wait_for_completion_timeout(c, msecs_to_jiffies(100)));
+    else
+        wait_for_completion(c);
+}

Hello Steve,

Quoted 2 lines

Have you verified this patch with checkpatch ? I expect that checkpatch will report that six braces are missing from this function.

(replying to my own e-mail)

BTW, this patch series does not call wait_for_drain() for any queue that uses IB_POLL_DIRECT. This means that the "if (cq->poll_ctx == IB_POLL_DIRECT)" part is untested. Please do propose untested code for upstream inclusion.

I guess you're making your point to not post code that has no user...

Quoted 1 line · Steve Wise

I guess you're making your point to not post code that has no user...

How about using WARN_ONCE(cq->poll_ctx == IB_POLL_DIRECT) instead ?

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Bart Van Assche
Sent: Thursday, February 11, 2016 4:59 PM
To: Steve Wise; [email protected]
Subject: Re: [PATCH 1/3] IB: new common API for draining queues

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

Quoted 9 lines · Steve Wise

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

+static void wait_for_drain(struct ib_cq *cq, struct completion *c)
+{
+	if (cq->poll_ctx == IB_POLL_DIRECT)
+		do
+			ib_process_cq_direct(cq, 1024);
+		while (!wait_for_completion_timeout(c, msecs_to_jiffies(100)));
+	else
+		wait_for_completion(c);
+}

Hello Steve,

Have you verified this patch with checkpatch ? I expect that checkpatch will report that six braces are missing from this function.

Thanks,

Bart.

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

Add provider-specific drain_sq/drain_rq functions for providers needing special drain logic.

Add static functions __ib_drain_sq() and __ib_drain_rq() which post noop WRs to the SQ or RQ and block until their completions are processed. This ensures the applications completions have all been processed.

Add API functions ib_drain_sq(), ib_drain_rq(), and ib_drain_qp().

Reviewed-by Chuck Lever <[email protected]>

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

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

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

index 5af6d02..aed521e 100644
@@ -1657,3 +1657,147 @@ 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);
+}
+
+static void wait_for_drain(struct ib_cq *cq, struct completion *c)
+{
+       if (cq->poll_ctx == IB_POLL_DIRECT)
+               do
+                       ib_process_cq_direct(cq, 1024);
+               while (!wait_for_completion_timeout(c, msecs_to_jiffies(100)));
+       else
+               wait_for_completion(c);
+}
+
+/*
+ * Post a WR and block until its completion is reaped for the SQ.
+ */
+static void __ib_drain_sq(struct ib_qp *qp)
+{
+       struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR };
+       struct ib_drain_cqe sdrain;
+       struct ib_send_wr swr = {}, *bad_swr;
+       int ret;

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

+
+       ret = ib_post_send(qp, &swr, &bad_swr);
+       if (ret) {
+               WARN_ONCE(ret, "failed to drain send queue: %d\n", ret);
+               return;
+       }
+
+       wait_for_drain(qp->send_cq, &sdrain.done);
+}
+
+/*
+ * Post a WR and block until its completion is reaped for the RQ.
+ */
+static void __ib_drain_rq(struct ib_qp *qp)
+{
+       struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR };
+       struct ib_drain_cqe rdrain;
+       struct ib_recv_wr rwr = {}, *bad_rwr;
+       int ret;
+
+       rwr.wr_cqe = &rdrain.cqe;
+       rdrain.cqe.done = ib_drain_qp_done;
+       init_completion(&rdrain.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 send queue: %d\n", ret);
+               return;
+       }
+
+       wait_for_drain(qp->recv_cq, &rdrain.done);
+}
+
+/**
+ * ib_drain_sq() - Block until all SQ 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_sq().
+ *
+ * The consumer must ensure there is room in the CQ and SQ
+ * for a drain work requests.  Also the consumer must allocate the
+ * CQ using ib_alloc_cq().  It is up to the consumer to handle concurrency
+ * issues if the CQs are using the IB_POLL_DIRECT polling context.
+ */
+void ib_drain_sq(struct ib_qp *qp)
+{
+       if (qp->device->drain_sq)
+               qp->device->drain_sq(qp);
+       else
+               __ib_drain_sq(qp);
+}
+EXPORT_SYMBOL(ib_drain_sq);
+
+/**
+ * ib_drain_rq() - Block until all RQ 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_rq().
+ *
+ * The consumer must ensure there is room in the CQ and RQ
+ * for a drain work requests.  Also the consumer must allocate the
+ * CQ using ib_alloc_cq().  It is up to the consumer to handle concurrency
+ * issues if the CQs are using the IB_POLL_DIRECT polling context.
+ */
+void ib_drain_rq(struct ib_qp *qp)
+{
+       if (qp->device->drain_rq)
+               qp->device->drain_rq(qp);
+       else
+               __ib_drain_rq(qp);
+}
+EXPORT_SYMBOL(ib_drain_rq);
+
+/**
+ * ib_drain_qp() - Block until all CQEs have been consumed by the
+ *                application on both the RQ and SQ.
+ * @qp:            queue pair to drain
+ *
+ * The consumer must ensure there is room in the CQ(s), SQ, and RQ
+ * for a drain work requests.  Also the consumer must allocate the
+ * CQ using ib_alloc_cq().  It is up to the consumer to handle concurrency
+ * issues if the CQs are using the IB_POLL_DIRECT polling context.
+ */
+void ib_drain_qp(struct ib_qp *qp)
+{
+       ib_drain_sq(qp);
+       ib_drain_rq(qp);
+}
+EXPORT_SYMBOL(ib_drain_qp);

include/rdma/ib_verbs.h +5 -1

index 284b00c..68b7e97 100644
@@ -1846,6 +1846,8 @@ 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_rq)(struct ib_qp *qp);
+       void                       (*drain_sq)(struct ib_qp *qp);
 
        struct ib_dma_mapping_ops   *dma_ops;
 

@@ -3094,4 +3096,7 @@ int ib_sg_to_pages(struct ib_mr *mr,
                   int sg_nents,
                   int (*set_page)(struct ib_mr *, u64));
 
+void ib_drain_rq(struct ib_qp *qp);
+void ib_drain_sq(struct ib_qp *qp);
+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 15 lines

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

+/*
+ * Post a WR and block until its completion is reaped for the SQ.
+ */
+static void __ib_drain_sq(struct ib_qp *qp)
+{
+       struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR };
+       struct ib_drain_cqe sdrain;
+       struct ib_send_wr swr = {}, *bad_swr;
+       int ret;
+
+       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);

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

Quoted 4 lines

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

+       if (ret) {
+               WARN_ONCE(ret, "failed to drain QP: %d\n", ret);
+               return;
+       }

Should it query qp-state and then modify if needed? I am not liking the the extra step which is highly unlikely in a normal usage conditions? especially for a shared CQ it is really redundant.

Quoted 15 lines

Quoted 15 lines

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

+/*
+ * Post a WR and block until its completion is reaped for the SQ.
+ */
+static void __ib_drain_sq(struct ib_qp *qp)
+{
+       struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR };
+       struct ib_drain_cqe sdrain;
+       struct ib_send_wr swr = {}, *bad_swr;
+       int ret;
+
+       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);

Quoted 9 lines

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

Quoted 4 lines

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

+       if (ret) {
+               WARN_ONCE(ret, "failed to drain QP: %d\n", ret);
+               return;
+       }

Should it query qp-state and then modify if needed? I am not liking the the extra step which is highly unlikely in a normal usage conditions? especially for a shared CQ it is really redundant.

Hey Devesh, on a previous round of reviews for this series, I thought you agreed that this was fine?

See: http://www.spinics.net/lists/linux-rdma/msg33202.html

My personal opinion is that calling query_qp() and only modifying if needed is as much work as just calling modify since ERR->ERR is basically a noop.

Steve.

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

Add provider-specific drain_sq/drain_rq functions for providers needing special drain logic.

Add static functions __ib_drain_sq() and __ib_drain_rq() which post noop WRs to the SQ or RQ and block until their completions are processed. This ensures the applications completions have all been processed.

Add API functions ib_drain_sq(), ib_drain_rq(), and ib_drain_qp().

Reviewed-by Chuck Lever <[email protected]>

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

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

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

index 5af6d02..aed521e 100644
@@ -1657,3 +1657,147 @@ 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);
+}
+
+static void wait_for_drain(struct ib_cq *cq, struct completion *c)
+{
+	if (cq->poll_ctx == IB_POLL_DIRECT)
+		do
+			ib_process_cq_direct(cq, 1024);
+		while (!wait_for_completion_timeout(c, msecs_to_jiffies(100)));
+	else
+		wait_for_completion(c);
+}
+
+/*
+ * Post a WR and block until its completion is reaped for the SQ.
+ */
+static void __ib_drain_sq(struct ib_qp *qp)
+{
+	struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR };
+	struct ib_drain_cqe sdrain;
+	struct ib_send_wr swr = {}, *bad_swr;
+	int ret;

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

+}
+
+/*
+ * Post a WR and block until its completion is reaped for the RQ.
+ */
+static void __ib_drain_rq(struct ib_qp *qp)
+{
+	struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR };
+	struct ib_drain_cqe rdrain;
+	struct ib_recv_wr rwr = {}, *bad_rwr;
+	int ret;
+
+	rwr.wr_cqe = &rdrain.cqe;
+	rdrain.cqe.done = ib_drain_qp_done;
+	init_completion(&rdrain.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 send queue: %d\n", ret);
+		return;
+	}
+
+	wait_for_drain(qp->recv_cq, &rdrain.done);
+}
+
+/**

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

+ * CQ using ib_alloc_cq().  It is up to the consumer to handle concurrency
+ * issues if the CQs are using the IB_POLL_DIRECT polling context.
+ */
+void ib_drain_sq(struct ib_qp *qp)
+{
+	if (qp->device->drain_sq)
+		qp->device->drain_sq(qp);
+	else
+		__ib_drain_sq(qp);
+}
+EXPORT_SYMBOL(ib_drain_sq);
+
+/**

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

+ * CQ using ib_alloc_cq().  It is up to the consumer to handle concurrency
+ * issues if the CQs are using the IB_POLL_DIRECT polling context.
+ */
+void ib_drain_rq(struct ib_qp *qp)
+{
+	if (qp->device->drain_rq)
+		qp->device->drain_rq(qp);
+	else
+		__ib_drain_rq(qp);

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

+ * CQ using ib_alloc_cq().  It is up to the consumer to handle concurrency
+ * issues if the CQs are using the IB_POLL_DIRECT polling context.
+ */
+void ib_drain_qp(struct ib_qp *qp)
+{
+	ib_drain_sq(qp);
+	ib_drain_rq(qp);
+}
+EXPORT_SYMBOL(ib_drain_qp);

include/rdma/ib_verbs.h +5 -0

index 284b00c..68b7e97 100644
@@ -1846,6 +1846,8 @@ 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_rq)(struct ib_qp *qp);
+	void			   (*drain_sq)(struct ib_qp *qp);
 
 	struct ib_dma_mapping_ops   *dma_ops;
 

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

-----Original Message-----
From: Doug Ledford [mailto:[email protected]]
Sent: Monday, February 15, 2016 3:06 PM
To: Steve Wise; [email protected]
Cc: [email protected]
Subject: Re: [PATCH 1/3] IB: new common API for draining queues

Quoted 8 lines · Steve Wise

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

Add provider-specific drain_sq/drain_rq functions for providers needing special drain logic.

Add static functions __ib_drain_sq() and __ib_drain_rq() which post noop WRs to the SQ or RQ and block until their completions are processed. This ensures the applications completions have all been processed.

Except it doesn't, comments inline below...and applications is possessive, so needs a '

Quoted 50 lines

Add API functions ib_drain_sq(), ib_drain_rq(), and ib_drain_qp().

Reviewed-by Chuck Lever <[email protected]>

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

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

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

index 5af6d02..aed521e 100644
@@ -1657,3 +1657,147 @@ 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);
+}
+
+static void wait_for_drain(struct ib_cq *cq, struct completion *c)
+{
+	if (cq->poll_ctx == IB_POLL_DIRECT)
+		do
+			ib_process_cq_direct(cq, 1024);
+		while (!wait_for_completion_timeout(c, msecs_to_jiffies(100)));
+	else
+		wait_for_completion(c);
+}
+
+/*
+ * Post a WR and block until its completion is reaped for the SQ.
+ */
+static void __ib_drain_sq(struct ib_qp *qp)
+{
+	struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR };
+	struct ib_drain_cqe sdrain;
+	struct ib_send_wr swr = {}, *bad_swr;
+	int ret;

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

Quoted 10 lines

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

+
+	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;
+	}

Set QP to ERR state...OK...

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

Quoted 5 lines

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

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

Post command to QP in ERR state (admittedly, I never do this and hadn't even thought about whether or not it is allowed...obviously it is, but my initial reaction would have been "won't ib_post_send return an error when you try to post to a QP in ERR state?")....OK...

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

Quoted 1 line

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

+	wait_for_drain(qp->send_cq, &sdrain.done);

Wait for your posted WR to complete...OK...

As I mentioned in my comment above, I would have thought that the attempt to post a send to a QP in ERR state would have returned an error. It must not or else this patch is worthless because of the order of actions. What that highlights though, is that this code will drain a QP, but only if the caller has taken the time to stop all possible contexts that might run on other cores and post commands to the QP. Those commands will error out, but the caller must, none the less, take steps to block other contexts from sending or else this drain is useless. That might be fine for the API, but it should be clearly documented, and currently it isn't.

Quoted 36 lines

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

+}
+
+/*
+ * Post a WR and block until its completion is reaped for the RQ.
+ */
+static void __ib_drain_rq(struct ib_qp *qp)
+{
+	struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR };
+	struct ib_drain_cqe rdrain;
+	struct ib_recv_wr rwr = {}, *bad_rwr;
+	int ret;
+
+	rwr.wr_cqe = &rdrain.cqe;
+	rdrain.cqe.done = ib_drain_qp_done;
+	init_completion(&rdrain.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 send queue: %d\n", ret);
+		return;
+	}
+
+	wait_for_drain(qp->recv_cq, &rdrain.done);
+}
+
+/**
+ * ib_drain_sq() - Block until all SQ CQEs have been consumed by the
+ *		   application.
+ * @qp:            queue pair to drain
+ *

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

Quoted 6 lines

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

+ * If the device has a provider-specific drain function, then
+ * call that.  Otherwise call the generic drain function
+ * __ib_drain_sq().
+ *
+ * The consumer must ensure there is room in the CQ and SQ
+ * for a drain work requests.  Also the consumer must allocate the

Either requests should be singular, or you should remove the article 'a'.

Quoted 13 lines

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

+ * CQ using ib_alloc_cq().  It is up to the consumer to handle concurrency
+ * issues if the CQs are using the IB_POLL_DIRECT polling context.
+ */
+void ib_drain_sq(struct ib_qp *qp)
+{
+	if (qp->device->drain_sq)
+		qp->device->drain_sq(qp);
+	else
+		__ib_drain_sq(qp);
+}
+EXPORT_SYMBOL(ib_drain_sq);
+
+/**

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

Quoted 10 lines

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

+ * ib_drain_rq() - Block until all RQ 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_rq().
+ *
+ * The consumer must ensure there is room in the CQ and RQ
+ * for a drain work requests.  Also the consumer must allocate the

Ditto...

Quoted 9 lines

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

+ * CQ using ib_alloc_cq().  It is up to the consumer to handle concurrency
+ * issues if the CQs are using the IB_POLL_DIRECT polling context.
+ */
+void ib_drain_rq(struct ib_qp *qp)
+{
+	if (qp->device->drain_rq)
+		qp->device->drain_rq(qp);
+	else
+		__ib_drain_rq(qp);

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

Quoted 10 lines

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

+}
+EXPORT_SYMBOL(ib_drain_rq);
+
+/**
+ * ib_drain_qp() - Block until all CQEs have been consumed by the
+ *		   application on both the RQ and SQ.
+ * @qp:            queue pair to drain
+ *
+ * The consumer must ensure there is room in the CQ(s), SQ, and RQ
+ * for a drain work requests.  Also the consumer must allocate the

Ditto...

Quoted 24 lines

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

+ * CQ using ib_alloc_cq().  It is up to the consumer to handle concurrency
+ * issues if the CQs are using the IB_POLL_DIRECT polling context.
+ */
+void ib_drain_qp(struct ib_qp *qp)
+{
+	ib_drain_sq(qp);
+	ib_drain_rq(qp);
+}
+EXPORT_SYMBOL(ib_drain_qp);

include/rdma/ib_verbs.h +2 -0

index 284b00c..68b7e97 100644
@@ -1846,6 +1846,8 @@ 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_rq)(struct ib_qp *qp);
+	void			   (*drain_sq)(struct ib_qp *qp);
 
 	struct ib_dma_mapping_ops   *dma_ops;
 

include/rdma/ib_verbs.h int ib_sg_to_pages(struct ib_mr *mr, · line 3102

Quoted 7 lines

include/rdma/ib_verbs.h +3 -0

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

--
Doug Ledford <[email protected]>

              GPG KeyID: 0E572FDD

As I mentioned in my comment above, I would have thought that the attempt to post a send to a QP in ERR state would have returned an error. It must not or else this patch is worthless because of the order of actions. What that highlights though, is that this code will drain a QP, but only if the caller has taken the time to stop all possible contexts that might run on other cores and post commands to the QP. Those commands will error out, but the caller must, none the less, take steps to block other contexts from sending or else this drain is useless. That might be fine for the API, but it should be clearly documented, and currently it isn't.

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

This series creates new helper API functions for draining a queue pair. It is a rework of an original patch created by Christoph Hellwig as part of the CQ API rework and was dropped to be resubmitted by me with iw_cxgb4 support.

Original thread: http://www.spinics.net/lists/linux-rdma/msg30296.html

Hi Steve,
For future submission, can you please use versioning on all patches submitted in the series and not only in cover letter?

In such way, internal filtering of new emails will be more informative.

Quoted 5 lines · Leon Romanovsky

Hi Steve,
For future submission, can you please use versioning on all patches submitted in the series and not only in cover letter?

Quoted 1 line

In such way, internal filtering of new emails will be more informative.

That's very unusual and not the git-send-email default for a reason. I'd suggest you fix your 'internal filtering' instead.

From my perspective there are many top developers which are using

--
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 9 lines · Christoph Hellwig

Quoted 5 lines · Leon Romanovsky

Hi Steve,
For future submission, can you please use versioning on all patches submitted in the series and not only in cover letter?

In such way, internal filtering of new emails will be more informative.

That's very unusual and not the git-send-email default for a reason. I'd suggest you fix your 'internal filtering' instead. From my perspective there are many top developers which are using

proposed format as you said "for a reason".

My flow is based on mutt mail reader and I'll be glad to hear how it can be fixed.

Can you suggest me how do I suppose "to fix" my filtering? Right now, the flow is:
1. Open mutt
2. Navigate to linux-rdma mailbox
3. Hit "l" (limit open)
4. Write "new"
5. Get only new (unread) mails.
6. Read all emails and for Steve's mails only, I need to disable limit in order to say if it is new version of patches or old ones.

P.S. get_maintainer.pl script has strange defaults too, it doesn't say that we need to follow blindly them.

The patches are threaded in that they all reference the cover letter email. Is there a way you can thread your view with mutt?