Skip to contentSkip to conversation

new ib_drain_qp() API

PATCH In review Steve Wise 8 participants 21 messages Feb 17, 2016 – Mar 24, 2016 12 messages not in the archive

Subsystems (4)
lore
Share & tools
lore

Steve Wise

author
#0 lore raw

v4 cover new ib_drain_qp() API

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

Changes since v3:

- WARN_ONCE() if the CQ is using IB_POLL_DIRECT context.

- added iser patch to use ib_drain_sq().

- fixed up function comments to specify caller requirements

- checkpatched

Changes since v2:

- created 3 drain API functions: ib_drain_rq(), ib_drain_sq(), and ib_drain_qp()

- add provider-specific drain function pointers for the sq and rq

- refactored the code a bit

- support for IB_DIRECT_POLL CQs

Changes since v1:

- added comments to the ib_drain_qp() function header specifying the consumer requirements

- in __ib_drain_qp(), if the ib_post_send() fails, still wait for the recv wr to drain since we already posted it.

- CC the SRP maintainer, [email protected]

---

Steve Wise (4):
IB: new common API for draining queues
iw_cxgb4: add queue drain functions
IB/srp: Use ib_drain_rq()
IB/iser: Use ib_drain_sq()

 drivers/infiniband/core/verbs.c              | 164 +++++++++++++++++++++++++++
 drivers/infiniband/hw/cxgb4/cq.c             |   9 +-
 drivers/infiniband/hw/cxgb4/iw_cxgb4.h       |   4 +
 drivers/infiniband/hw/cxgb4/provider.c       |   2 +
 drivers/infiniband/hw/cxgb4/qp.c             |  16 +++
 drivers/infiniband/ulp/iser/iscsi_iser.h     |   7 --
 drivers/infiniband/ulp/iser/iser_initiator.c |   7 --
 drivers/infiniband/ulp/iser/iser_verbs.c     |  15 +--
 drivers/infiniband/ulp/srp/ib_srp.c          |  40 +------
 include/rdma/ib_verbs.h                      |   5 +
 10 files changed, 205 insertions(+), 64 deletions(-)
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

A message that is not in the archive (sent off-list or not yet received): cover.1455741336.git.swise-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org

author
#2 lore raw

PATCH v4 1/4 IB: new common API for draining queues

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 for work requests posted prior to the drain work request have all been processed.

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

For the drain logic to work, the caller must:

ensure there is room in the CQ(s) and QP for the drain work request and completion.

allocate the CQ using ib_alloc_cq() and the CQ poll context cannot be IB_POLL_DIRECT.

ensure that there are no other contexts that are posting WRs concurrently. Otherwise the drain is not guaranteed.

---
 drivers/infiniband/core/verbs.c | 164 ++++++++++++++++++++++++++++++++++++++++
 include/rdma/ib_verbs.h         |   5 ++
 2 files changed, 169 insertions(+)
drivers/infiniband/core/verbs.c +164 -0
index 5af6d02..48dc43c 100644
@@ -1657,3 +1657,167 @@ 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 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;
+
+	if (qp->send_cq->poll_ctx == IB_POLL_DIRECT) {
+		WARN_ONCE(qp->send_cq->poll_ctx == IB_POLL_DIRECT,
+			  "IB_POLL_DIRECT poll_ctx not supported for drain\n");
+		return;
+	}
+
+	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 send 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(&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;
+
+	if (qp->recv_cq->poll_ctx == IB_POLL_DIRECT) {
+		WARN_ONCE(qp->recv_cq->poll_ctx == IB_POLL_DIRECT,
+			  "IB_POLL_DIRECT poll_ctx not supported for drain\n");
+		return;
+	}
+
+	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 recv queue: %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;
+	}
+
+	wait_for_completion(&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 caller must:
+ *
+ * ensure there is room in the CQ and SQ for the drain work request and
+ * completion.
+ *
+ * allocate the CQ using ib_alloc_cq() and the CQ poll context cannot be
+ * IB_POLL_DIRECT.
+ *
+ * ensure that there are no other contexts that are posting WRs concurrently.
+ * Otherwise the drain is not guaranteed.
+ */
+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 caller must:
+ *
+ * ensure there is room in the CQ and RQ for the drain work request and
+ * completion.
+ *
+ * allocate the CQ using ib_alloc_cq() and the CQ poll context cannot be
+ * IB_POLL_DIRECT.
+ *
+ * ensure that there are no other contexts that are posting WRs concurrently.
+ * Otherwise the drain is not guaranteed.
+ */
+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 caller must:
+ *
+ * ensure there is room in the CQ(s), SQ, and RQ for drain work requests
+ * and completions.
+ *
+ * allocate the CQs using ib_alloc_cq() and the CQ poll context cannot be
+ * IB_POLL_DIRECT.
+ *
+ * ensure that there are no other contexts that are posting WRs concurrently.
+ * Otherwise the drain is not guaranteed.
+ */
+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 */
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

A message that is not in the archive (sent off-list or not yet received): 5b4799151723516ec440191b3f0ae98a46b356f0.1455741336.git.swise-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org

#4 lore raw

Re: [PATCH v4 1/4] IB: new common API for draining queues

Quoted 56 lines · Steve Wise
drivers/infiniband/core/verbs.c +56 -0
+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;
+
+	if (qp->send_cq->poll_ctx == IB_POLL_DIRECT) {
+		WARN_ONCE(qp->send_cq->poll_ctx == IB_POLL_DIRECT,
+			  "IB_POLL_DIRECT poll_ctx not supported for drain\n");
+		return;
+	}
+
+	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 send 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(&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;
+
+	if (qp->recv_cq->poll_ctx == IB_POLL_DIRECT) {
+		WARN_ONCE(qp->recv_cq->poll_ctx == IB_POLL_DIRECT,
+			  "IB_POLL_DIRECT poll_ctx not supported for drain\n");
+		return;
+	}
+
+	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 recv queue: %d\n", ret);
+		return;

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

Quoted 10 lines
drivers/infiniband/core/verbs.c +10 -0
+	}
+
+	ret = ib_post_recv(qp, &rwr, &bad_rwr);
+	if (ret) {
+		WARN_ONCE(ret, "failed to drain recv queue: %d\n", ret);
+		return;
+	}
+
+	wait_for_completion(&rdrain.done);
+}

Hello Steve,

I only have one comment about this patch: are you aware that the six invocations of WARN_ONCE() can be made less verbose by rewriting if (c) { WARN_ONCE(c, m); ... } as if (WARN_ONCE(c, m)) { ... } ?

Bart.
--
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

A message that is not in the archive (sent off-list or not yet received): [email protected]

#6 lore raw

RE: [PATCH v4 1/4] IB: new common API for draining queues

Quoted 79 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Bart Van Assche
Sent: Wednesday, February 17, 2016 5:36 PM
To: Steve Wise; [email protected]
Cc: ogerlitz-VPRAkNaXOzVWk0Htik3J/[email protected]; sagig-VPRAkNaXOzVWk0Htik3J/[email protected]; roid-VPRAkNaXOzVWk0Htik3J/[email protected]
Subject: Re: [PATCH v4 1/4] IB: new common API for draining queues

Quoted 56 lines · Steve Wise
drivers/infiniband/core/verbs.c +56 -0
+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;
+
+	if (qp->send_cq->poll_ctx == IB_POLL_DIRECT) {
+		WARN_ONCE(qp->send_cq->poll_ctx == IB_POLL_DIRECT,
+			  "IB_POLL_DIRECT poll_ctx not supported for drain\n");
+		return;
+	}
+
+	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 send 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(&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;
+
+	if (qp->recv_cq->poll_ctx == IB_POLL_DIRECT) {
+		WARN_ONCE(qp->recv_cq->poll_ctx == IB_POLL_DIRECT,
+			  "IB_POLL_DIRECT poll_ctx not supported for drain\n");
+		return;
+	}
+
+	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 recv queue: %d\n", ret);
+		return;

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

Quoted 10 lines
drivers/infiniband/core/verbs.c +10 -0
+	}
+
+	ret = ib_post_recv(qp, &rwr, &bad_rwr);
+	if (ret) {
+		WARN_ONCE(ret, "failed to drain recv queue: %d\n", ret);
+		return;
+	}
+
+	wait_for_completion(&rdrain.done);
+}

Hello Steve,

Quoted 4 lines

I only have one comment about this patch: are you aware that the six invocations of WARN_ONCE() can be made less verbose by rewriting if (c) { WARN_ONCE(c, m); ... } as if (WARN_ONCE(c, m)) { ... } ?

I am now. :) I didn't know WARN* macros returned the condition.

--
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

A message that is not in the archive (sent off-list or not yet received): 56C9D14B.9010307-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org

Reviewed-by
#9 lore raw

Re: [PATCH v4 1/4] IB: new common API for draining queues

Hi Steve,

The patch looks good to me.

Quoted 8 lines · Sagi Grimberg

Thanks Steve, this looks good to me,

--
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

--
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

#10 lore raw

PATCH v4 2/4 iw_cxgb4: add queue drain functions

In reply to a message not in the archive

Add completion objects, named sq_drained and rq_drained, to the c4iw_qp struct. The queue-specific completion object is signaled when the last CQE is drained from the CQ for that queue.

Add c4iw_drain_sq() to block until qp->rq_drained is completed.

Add c4iw_drain_rq() to block until qp->sq_drained is completed.

---
 drivers/infiniband/hw/cxgb4/cq.c       |  9 ++++++++-
 drivers/infiniband/hw/cxgb4/iw_cxgb4.h |  4 ++++
 drivers/infiniband/hw/cxgb4/provider.c |  2 ++
 drivers/infiniband/hw/cxgb4/qp.c       | 16 ++++++++++++++++
 4 files changed, 30 insertions(+), 1 deletion(-)
drivers/infiniband/hw/cxgb4/cq.c +8 -1
index cf21df4..b4eeb78 100644
@@ -815,8 +815,15 @@ static int c4iw_poll_cq_one(struct c4iw_cq *chp, struct ib_wc *wc)
 		}
 	}
 out:
-	if (wq)
+	if (wq) {
+		if (unlikely(qhp->attr.state != C4IW_QP_STATE_RTS)) {
+			if (t4_sq_empty(wq))
+				complete(&qhp->sq_drained);
+			if (t4_rq_empty(wq))
+				complete(&qhp->rq_drained);
+		}
 		spin_unlock(&qhp->lock);
+	}
 	return ret;
 }
 
drivers/infiniband/hw/cxgb4/iw_cxgb4.h +4 -0
index fb2de75..7c6a6e1 100644
@@ -476,6 +476,8 @@ struct c4iw_qp {
 	wait_queue_head_t wait;
 	struct timer_list timer;
 	int sq_sig_all;
+	struct completion rq_drained;
+	struct completion sq_drained;
 };
 
 static inline struct c4iw_qp *to_c4iw_qp(struct ib_qp *ibqp)
@@ -1016,6 +1018,8 @@ extern int c4iw_wr_log;
 extern int db_fc_threshold;
 extern int db_coalescing_threshold;
 extern int use_dsgl;
+void c4iw_drain_rq(struct ib_qp *qp);
+void c4iw_drain_sq(struct ib_qp *qp);
 
 
 #endif
drivers/infiniband/hw/cxgb4/provider.c +2 -0
index ec04272..104662d 100644
@@ -564,6 +564,8 @@ int c4iw_register_device(struct c4iw_dev *dev)
 	dev->ibdev.get_protocol_stats = c4iw_get_mib;
 	dev->ibdev.uverbs_abi_ver = C4IW_UVERBS_ABI_VERSION;
 	dev->ibdev.get_port_immutable = c4iw_port_immutable;
+	dev->ibdev.drain_sq = c4iw_drain_sq;
+	dev->ibdev.drain_rq = c4iw_drain_rq;
 
 	dev->ibdev.iwcm = kmalloc(sizeof(struct iw_cm_verbs), GFP_KERNEL);
 	if (!dev->ibdev.iwcm)
drivers/infiniband/hw/cxgb4/qp.c +16 -0
index e99345e..7b1b1e8 100644
@@ -1697,6 +1697,8 @@ struct ib_qp *c4iw_create_qp(struct ib_pd *pd, struct ib_qp_init_attr *attrs,
 	qhp->attr.max_ird = 0;
 	qhp->sq_sig_all = attrs->sq_sig_type == IB_SIGNAL_ALL_WR;
 	spin_lock_init(&qhp->lock);
+	init_completion(&qhp->sq_drained);
+	init_completion(&qhp->rq_drained);
 	mutex_init(&qhp->mutex);
 	init_waitqueue_head(&qhp->wait);
 	atomic_set(&qhp->refcnt, 1);
@@ -1888,3 +1890,17 @@ int c4iw_ib_query_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
 	init_attr->sq_sig_type = qhp->sq_sig_all ? IB_SIGNAL_ALL_WR : 0;
 	return 0;
 }
+
+void c4iw_drain_sq(struct ib_qp *ibqp)
+{
+	struct c4iw_qp *qp = to_c4iw_qp(ibqp);
+
+	wait_for_completion(&qp->sq_drained);
+}
+
+void c4iw_drain_rq(struct ib_qp *ibqp)
+{
+	struct c4iw_qp *qp = to_c4iw_qp(ibqp);
+
+	wait_for_completion(&qp->rq_drained);
+}
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

A message that is not in the archive (sent off-list or not yet received): 162a5429342dfd5e16881f4360c0ef858fd39cbd.1455741336.git.swise-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org

#13 lore raw

Re: [PATCH v4 2/4] iw_cxgb4: add queue drain functions

In reply to a message not in the archive
Quoted 7 lines · Steve Wise

Add completion objects, named sq_drained and rq_drained, to the c4iw_qp struct. The queue-specific completion object is signaled when the last CQE is drained from the CQ for that queue.

Add c4iw_drain_sq() to block until qp->rq_drained is completed.

Add c4iw_drain_rq() to block until qp->sq_drained is completed.

Hi Steve,
My knowledge is limited in this driver and I have no illusion that question sounds naive, but why did you add these functions to this specific driver? Do the other drivers need such update too?

Thanks
--
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

A message that is not in the archive (sent off-list or not yet received): 20160309125349.GA1977-2ukJVAZIZ/[email protected]

#15 lore raw

RE: [PATCH v4 2/4] iw_cxgb4: add queue drain functions

Quoted 7 lines · Steve Wise
Quoted 6 lines

Add completion objects, named sq_drained and rq_drained, to the c4iw_qp struct. The queue-specific completion object is signaled when the last CQE is drained from the CQ for that queue.

Add c4iw_drain_sq() to block until qp->rq_drained is completed.

Quoted 8 lines
Quoted 1 line

Add c4iw_drain_rq() to block until qp->sq_drained is completed.

Hi Steve,
My knowledge is limited in this driver and I have no illusion that question sounds naive, but why did you add these functions to this specific driver? Do the other drivers need such update too?

Thanks

Hey Leon,

I added these so iSER and other storage protocols that are or will be using the drain logic will work on cxgb4. IB devices should not need device-specific drain logic since the common core drain works for IB (and I assume RoCE). iWARP devices, however, require device-specific logic. Other iwarp drivers are iw_cxgb3 and iw_nes.

Steve.

--
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

#16 lore raw

Re: [PATCH v4 2/4] iw_cxgb4: add queue drain functions

Quoted 6 lines
Quoted 6 lines

Hi Steve,
My knowledge is limited in this driver and I have no illusion that question sounds naive, but why did you add these functions to this specific driver? Do the other drivers need such update too?

Quoted 7 lines
Quoted 1 line

Thanks

Hey Leon,

I added these so iSER and other storage protocols that are or will be using the drain logic will work on cxgb4. IB devices should not need device-specific drain logic since the common core drain works for IB (and I assume RoCE). iWARP devices, however, require device-specific logic. Other iwarp drivers are iw_cxgb3 and iw_nes.

Yes, this is iWARP specific. Steve was kind enough to fix chelsio drivers and we need someone to pick up nes so ULPs will be able to run reliably over it.
--
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

A message that is not in the archive (sent off-list or not yet received): 56E0412F.4000602-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org

#18 lore raw

Re: [PATCH v4 2/4] iw_cxgb4: add queue drain functions

Quoted 10 lines · Sagi Grimberg
Quoted 10 lines
Quoted 7 lines

Hi Steve,
My knowledge is limited in this driver and I have no illusion that question sounds naive, but why did you add these functions to this specific driver? Do the other drivers need such update too?

Thanks

Hey Leon,

Quoted 7 lines
Quoted 3 lines

I added these so iSER and other storage protocols that are or will be using the drain logic will work on cxgb4. IB devices should not need device-specific drain logic since the common core drain works for IB (and I assume RoCE). iWARP devices, however, require device-specific logic. Other iwarp drivers are iw_cxgb3 and iw_nes.

Yes, this is iWARP specific. Steve was kind enough to fix chelsio drivers and we need someone to pick up nes so ULPs will be able to run reliably over it.

Steve and Sagi,
Thank you for clarification.

I see that nes driver is supported. Is it still relevant?

NETEFFECT IWARP RNIC DRIVER (IW_NES)

M:      Faisal Latif <[email protected]>
L:      [email protected]
W:	http://www.intel.com/Products/Server/Adapters/Server-Cluster/Server-Cluster-overview.htm
S:      Supported
F:      drivers/infiniband/hw/nes/

--
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

A message that is not in the archive (sent off-list or not yet received): 20160310050558.GD1977-2ukJVAZIZ/[email protected]

#20 lore raw

Re: [PATCH v4 2/4] iw_cxgb4: add queue drain functions

ping

Quoted 9 lines · Leon Romanovsky
Quoted 4 lines · Sagi Grimberg

Yes, this is iWARP specific. Steve was kind enough to fix chelsio drivers and we need someone to pick up nes so ULPs will be able to run reliably over it.

Steve and Sagi,
Thank you for clarification.

Quoted 8 lines

I see that nes driver is supported. Is it still relevant?

NETEFFECT IWARP RNIC DRIVER (IW_NES)

M:      Faisal Latif <[email protected]>
L:      [email protected]
W:	http://www.intel.com/Products/Server/Adapters/Server-Cluster/Server-Cluster-overview.htm
S:      Supported
F:      drivers/infiniband/hw/nes/

--
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

A message that is not in the archive (sent off-list or not yet received): 20160324104659.GC11949-2ukJVAZIZ/[email protected]

#22 lore raw

Re: [PATCH v4 2/4] iw_cxgb4: add queue drain functions

Quoted 14 lines · Leon Romanovsky

ping

Quoted 11 lines · Leon Romanovsky
Quoted 4 lines · Sagi Grimberg

Yes, this is iWARP specific. Steve was kind enough to fix chelsio drivers and we need someone to pick up nes so ULPs will be able to run reliably over it.

Steve and Sagi,
Thank you for clarification.

I see that nes driver is supported. Is it still relevant?

Sorry as I was on vacation and will get the patch for nes ready.

Thanks
Faisal
--
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

#23 lore raw

PATCH v4 3/4 IB/srp: Use ib_drain_rq()

In reply to a message not in the archive
---
 drivers/infiniband/ulp/srp/ib_srp.c | 40 ++++---------------------------------
 1 file changed, 4 insertions(+), 36 deletions(-)
drivers/infiniband/ulp/srp/ib_srp.c +4 -36
index 03022f6..b6bf204 100644
@@ -446,49 +446,17 @@ static struct srp_fr_pool *srp_alloc_fr_pool(struct srp_target_port *target)
 				  dev->max_pages_per_mr);
 }
 
-static void srp_drain_done(struct ib_cq *cq, struct ib_wc *wc)
-{
-	struct srp_rdma_ch *ch = cq->cq_context;
-
-	complete(&ch->done);
-}
-
-static struct ib_cqe srp_drain_cqe = {
-	.done		= srp_drain_done,
-};
-
 /**
  * srp_destroy_qp() - destroy an RDMA queue pair
  * @ch: SRP RDMA channel.
  *
- * Change a queue pair into the error state and wait until all receive
- * completions have been processed before destroying it. This avoids that
- * the receive completion handler can access the queue pair while it is
+ * Drain the qp before destroying it.  This avoids that the receive
+ * completion handler can access the queue pair while it is
  * being destroyed.
  */
 static void srp_destroy_qp(struct srp_rdma_ch *ch)
 {
-	static struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR };
-	static struct ib_recv_wr wr = { 0 };
-	struct ib_recv_wr *bad_wr;
-	int ret;
-
-	wr.wr_cqe = &srp_drain_cqe;
-	/* Destroying a QP and reusing ch->done is only safe if not connected */
-	WARN_ON_ONCE(ch->connected);
-
-	ret = ib_modify_qp(ch->qp, &attr, IB_QP_STATE);
-	WARN_ONCE(ret, "ib_cm_init_qp_attr() returned %d\n", ret);
-	if (ret)
-		goto out;
-
-	init_completion(&ch->done);
-	ret = ib_post_recv(ch->qp, &wr, &bad_wr);
-	WARN_ONCE(ret, "ib_post_recv() returned %d\n", ret);
-	if (ret == 0)
-		wait_for_completion(&ch->done);
-
-out:
+	ib_drain_rq(ch->qp);
 	ib_destroy_qp(ch->qp);
 }
 
@@ -508,7 +476,7 @@ static int srp_create_ch_ib(struct srp_rdma_ch *ch)
 	if (!init_attr)
 		return -ENOMEM;
 
-	/* queue_size + 1 for ib_drain_qp */
+	/* queue_size + 1 for ib_drain_rq() */
 	recv_cq = ib_alloc_cq(dev->dev, ch, target->queue_size + 1,
 				ch->comp_vector, IB_POLL_SOFTIRQ);
 	if (IS_ERR(recv_cq)) {
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

A message that is not in the archive (sent off-list or not yet received): 8a0a024ce256ac8c9be5e46a05f158d6e74a3cb9.1455741336.git.swise-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org

#26 lore raw

PATCH v4 4/4 IB/iser: Use ib_drain_sq()

In reply to a message not in the archive
---
 drivers/infiniband/ulp/iser/iscsi_iser.h     |  7 -------
 drivers/infiniband/ulp/iser/iser_initiator.c |  7 -------
 drivers/infiniband/ulp/iser/iser_verbs.c     | 15 ++-------------
 3 files changed, 2 insertions(+), 27 deletions(-)
drivers/infiniband/ulp/iser/iscsi_iser.h +0 -7
index 95f0a64..0351059 100644
@@ -458,9 +458,6 @@ struct iser_fr_pool {
  * @comp:                iser completion context
  * @fr_pool:             connection fast registration poool
  * @pi_support:          Indicate device T10-PI support
- * @last:                last send wr to signal all flush errors were drained
- * @last_cqe:            cqe handler for last wr
- * @last_comp:           completes when all connection completions consumed
  */
 struct ib_conn {
 	struct rdma_cm_id           *cma_id;
@@ -472,10 +469,7 @@ struct ib_conn {
 	struct iser_comp	    *comp;
 	struct iser_fr_pool          fr_pool;
 	bool			     pi_support;
-	struct ib_send_wr	     last;
-	struct ib_cqe		     last_cqe;
 	struct ib_cqe		     reg_cqe;
-	struct completion	     last_comp;
 };
 
 /**
@@ -617,7 +611,6 @@ void iser_cmd_comp(struct ib_cq *cq, struct ib_wc *wc);
 void iser_ctrl_comp(struct ib_cq *cq, struct ib_wc *wc);
 void iser_dataout_comp(struct ib_cq *cq, struct ib_wc *wc);
 void iser_reg_comp(struct ib_cq *cq, struct ib_wc *wc);
-void iser_last_comp(struct ib_cq *cq, struct ib_wc *wc);
 
 void iser_task_rdma_init(struct iscsi_iser_task *task);
 
drivers/infiniband/ulp/iser/iser_initiator.c +0 -7
index ed54b38..81ae2e3 100644
@@ -729,13 +729,6 @@ void iser_dataout_comp(struct ib_cq *cq, struct ib_wc *wc)
 	kmem_cache_free(ig.desc_cache, desc);
 }
 
-void iser_last_comp(struct ib_cq *cq, struct ib_wc *wc)
-{
-	struct ib_conn *ib_conn = wc->qp->qp_context;
-
-	complete(&ib_conn->last_comp);
-}
-
 void iser_task_rdma_init(struct iscsi_iser_task *iser_task)
 
 {
drivers/infiniband/ulp/iser/iser_verbs.c +2 -13
index 40c0f49..47e1159 100644
@@ -663,7 +663,6 @@ void iser_conn_release(struct iser_conn *iser_conn)
 int iser_conn_terminate(struct iser_conn *iser_conn)
 {
 	struct ib_conn *ib_conn = &iser_conn->ib_conn;
-	struct ib_send_wr *bad_wr;
 	int err = 0;
 
 	/* terminate the iser conn only if the conn state is UP */
@@ -688,14 +687,8 @@ int iser_conn_terminate(struct iser_conn *iser_conn)
 			iser_err("Failed to disconnect, conn: 0x%p err %d\n",
 				 iser_conn, err);
 
-		/* post an indication that all flush errors were consumed */
-		err = ib_post_send(ib_conn->qp, &ib_conn->last, &bad_wr);
-		if (err) {
-			iser_err("conn %p failed to post last wr", ib_conn);
-			return 1;
-		}
-
-		wait_for_completion(&ib_conn->last_comp);
+		/* block until all flush errors are consumed */
+		ib_drain_sq(ib_conn->qp);
 	}
 
 	return 1;
@@ -954,10 +947,6 @@ void iser_conn_init(struct iser_conn *iser_conn)
 
 	ib_conn->post_recv_buf_count = 0;
 	ib_conn->reg_cqe.done = iser_reg_comp;
-	ib_conn->last_cqe.done = iser_last_comp;
-	ib_conn->last.wr_cqe = &ib_conn->last_cqe;
-	ib_conn->last.opcode = IB_WR_SEND;
-	init_completion(&ib_conn->last_comp);
 }
 
  /**
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

A message that is not in the archive (sent off-list or not yet received): 269ab4c6de9c30bb6beba249302d5f1d1db9b924.1455741337.git.swise-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org

A message that is not in the archive (sent off-list or not yet received): 56D80D8F.7070700-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org

#31 lore raw

RE: [PATCH v4 4/4] IB/iser: Use ib_drain_sq()

Quoted 5 lines

Steve, I prefer that iser will call ib_drain_qp(). It's harmless from that point of view.

Given that Doug pulled it into his github, I can send an incremental patch.

Sounds good.

Steve

--
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

#32 lore raw
In reply to a message not in the archive
Quoted 49 lines · Steve Wise

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

Changes since v3:

- WARN_ONCE() if the CQ is using IB_POLL_DIRECT context.

- added iser patch to use ib_drain_sq().

- fixed up function comments to specify caller requirements

- checkpatched

Changes since v2:

- created 3 drain API functions: ib_drain_rq(), ib_drain_sq(), and ib_drain_qp()

- add provider-specific drain function pointers for the sq and rq

- refactored the code a bit

- support for IB_DIRECT_POLL CQs

Changes since v1:

- added comments to the ib_drain_qp() function header specifying the consumer requirements

- in __ib_drain_qp(), if the ib_post_send() fails, still wait for the recv wr to drain since we already posted it.

- CC the SRP maintainer, [email protected]

---

Steve Wise (4):
IB: new common API for draining queues
iw_cxgb4: add queue drain functions
IB/srp: Use ib_drain_rq()
IB/iser: Use ib_drain_sq()

Quoted 12 lines
 drivers/infiniband/core/verbs.c              | 164 +++++++++++++++++++++++++++
 drivers/infiniband/hw/cxgb4/cq.c             |   9 +-
 drivers/infiniband/hw/cxgb4/iw_cxgb4.h       |   4 +
 drivers/infiniband/hw/cxgb4/provider.c       |   2 +
 drivers/infiniband/hw/cxgb4/qp.c             |  16 +++
 drivers/infiniband/ulp/iser/iscsi_iser.h     |   7 --
 drivers/infiniband/ulp/iser/iser_initiator.c |   7 --
 drivers/infiniband/ulp/iser/iser_verbs.c     |  15 +--
 drivers/infiniband/ulp/srp/ib_srp.c          |  40 +------
 include/rdma/ib_verbs.h                      |   5 +
 10 files changed, 205 insertions(+), 64 deletions(-)

Series looks good now, thanks.

Signature · 2 lines
Doug Ledford <[email protected]>
              GPG KeyID: 0E572FDD