new ib_drain_qp() API
Subsystems (4)
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
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
@@ -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
@@ -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
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]
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 queuesQuoted 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
Sagi Grimberg
Reviewed-byRe: [PATCH v4 1/4] IB: new common API for draining queues
In reply to a message not in the archiveThanks 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
A message that is not in the archive (sent off-list or not yet received): 56C9D14B.9010307-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org
Devesh Sharma
Reviewed-byRe: [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
PATCH v4 2/4 iw_cxgb4: add queue drain functions
In reply to a message not in the archiveAdd 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
@@ -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
@@ -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
@@ -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
@@ -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
Sagi Grimberg
Reviewed-byRe: [PATCH v4 2/4] iw_cxgb4: add queue drain functions
Looks good,
--
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
Re: [PATCH v4 2/4] iw_cxgb4: add queue drain functions
In reply to a message not in the archiveQuoted 7 lines · Steve Wise
Add completion objects, named
sq_drainedandrq_drained, to thec4iw_qpstruct. 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 untilqp->rq_drainedis completed.Add
c4iw_drain_rq()to block untilqp->sq_drainedis 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]
RE: [PATCH v4 2/4] iw_cxgb4: add queue drain functions
Quoted 7 lines · Steve Wise
Quoted 6 lines
Add completion objects, named
sq_drainedandrq_drained, to thec4iw_qpstruct. 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 untilqp->rq_drainedis completed.
Quoted 8 lines
Quoted 1 line
Add
c4iw_drain_rq()to block untilqp->sq_drainedis 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
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_cxgb3andiw_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
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_cxgb3andiw_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]
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]
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
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
@@ -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
Sagi Grimberg
Reviewed-byRe: [PATCH v4 3/4] IB/srp: Use ib_drain_rq()
Looks good,
--
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
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
@@ -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
@@ -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
@@ -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
Sagi Grimberg
Acked-byRe: [PATCH v4 4/4] IB/iser: Use ib_drain_sq()
Looks good and works,
--
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
Re: [PATCH v4 4/4] IB/iser: Use ib_drain_sq()
In reply to a message not in the archiveSteve, 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.
Cheers,
Sagi.
--
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): 56D80D8F.7070700-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org
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
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_cxgb4support.Original thread: http://www.spinics.net/lists/linux-rdma/msg30296.html
Changes since v3:
-
WARN_ONCE()if the CQ is usingIB_POLL_DIRECTcontext.- 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(), andib_drain_qp()- add provider-specific drain function pointers for the sq and rq
- refactored the code a bit
- support for
IB_DIRECT_POLLCQsChanges since v1:
- added comments to the
ib_drain_qp()function header specifying the consumer requirements- in
__ib_drain_qp(), if theib_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: Useib_drain_rq()
IB/iser: Useib_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