RDMA/cxgb3: When a user QP is marked in error, also mark the CQs in error.
PATCH 1/1 RDMA/cxgb3: When a user QP is marked in error, also mark the CQs in error.
The flushing of work requests for user QPs is implemented entirely in the user mode library. The only kernel interaction is to mark the user QP object indicating it is in error when the QP exits RTS. When the user QP operations are called by the application (eg: post_send, post_recv), the QP in error bit is checked and if set, the library flushes the QP. If, however, the application is not doing IO, but rather just polling the CQ, it will never get flushed work requests. This breaks some classes of applications.
This patch adds logic to mark user CQs in error when a QP that is bound to the CQ is marked in error. The library poll code can then notice the CQ is in error and flush all the in error QPs bound to that CQ.
Design:
- add 1 extra CQE entry to the CQ memory that will be used to indicate in error status.
- return the desired CQ memory size that should be mapped by the library
- bump the ABI since the create_cq uverbs response changes.
- detect older libraries and reduce the mmap size accordingly.
---
drivers/infiniband/hw/cxgb3/cxio_hal.c | 1 + drivers/infiniband/hw/cxgb3/cxio_wr.h | 16 ++++++++++++++++ drivers/infiniband/hw/cxgb3/iwch_ev.c | 17 +++++++++-------- drivers/infiniband/hw/cxgb3/iwch_provider.c | 19 +++++++++++++++---- drivers/infiniband/hw/cxgb3/iwch_qp.c | 25 ++++++++++++++++++------- drivers/infiniband/hw/cxgb3/iwch_user.h | 1 + 6 files changed, 60 insertions(+), 19 deletions(-)
drivers/infiniband/hw/cxgb3/cxio_hal.c +1 -0
@@ -160,6 +160,7 @@ int cxio_create_cq(struct cxio_rdev *rdev_p, struct t3_cq *cq, int kernel)struct rdma_cq_setup setup;int size = (1UL << (cq->size_log2)) *sizeof (struct t3_cqe ); + size +=1 ;/* one extra page for storing cq-in-err state */ cq->cqid =cxio_hal_get_cqid (rdev_p->rscp);if (!cq->cqid)return -ENOMEM ;
drivers/infiniband/hw/cxgb3/cxio_wr.h +16 -0
@@ -728,6 +728,22 @@ struct t3_cq {#define CQ_VLD_ENTRY (ptr,size_log2,cqe) (Q_GENBIT(ptr,size_log2) == \ CQE_GENBIT(*cqe)) +struct t3_cq_status_page { +u32 cq_err; +}; + +static inline int cxio_cq_in_error (struct t3_cq *cq) +{ +return ((struct t3_cq_status_page *) + &cq->queue[1 << cq->size_log2])->cq_err; +} + +static inline void cxio_set_cq_in_error (struct t3_cq *cq) +{ + ((struct t3_cq_status_page *) + &cq->queue[1 << cq->size_log2])->cq_err =1 ; +} +static inline void cxio_set_wq_in_error (struct t3_wq *wq) { wq->queue->wq_in_err.err |=1 ;
drivers/infiniband/hw/cxgb3/iwch_ev.c +9 -8
@@ -76,6 +76,14 @@ static void post_qp_event(struct iwch_dev *rnicp, struct iwch_cq *chp,atomic_inc (&qhp->refcnt);spin_unlock (&rnicp->lock); +if (qhp->attr.state ==IWCH_QP_STATE_RTS ) { + attrs.next_state =IWCH_QP_STATE_TERMINATE ; +iwch_modify_qp (qhp->rhp, qhp,IWCH_QP_ATTR_NEXT_STATE , + &attrs,1 ); +if (send_term) +iwch_post_terminate (qhp, rsp_msg); + } + event.event = ib_event; event.device = chp->ibcq.device;if (ib_event ==IB_EVENT_CQ_ERR )
@@ -86,13 +94,7 @@ static void post_qp_event(struct iwch_dev *rnicp, struct iwch_cq *chp,if (qhp->ibqp.event_handler) (*qhp->ibqp.event_handler)(&event, qhp->ibqp.qp_context); -if (qhp->attr.state ==IWCH_QP_STATE_RTS ) { - attrs.next_state =IWCH_QP_STATE_TERMINATE ; -iwch_modify_qp (qhp->rhp, qhp,IWCH_QP_ATTR_NEXT_STATE , - &attrs,1 ); -if (send_term) -iwch_post_terminate (qhp, rsp_msg); - } + (*chp->ibcq.comp_handler)(&chp->ibcq, chp->ibcq.cq_context);if (atomic_dec_and_test (&qhp->refcnt))wake_up (&qhp->wait);
@@ -179,7 +181,6 @@ void iwch_ev_dispatch(struct cxio_rdev *rdev_p, struct sk_buff *skb)case TPT_ERR_BOUND :case TPT_ERR_INVALIDATE_SHARED_MR :case TPT_ERR_INVALIDATE_MR_WITH_MW_BOUND : - (*chp->ibcq.comp_handler)(&chp->ibcq, chp->ibcq.cq_context);post_qp_event (rnicp, chp, rsp_msg,IB_EVENT_QP_ACCESS_ERR ,1 );break ;
drivers/infiniband/hw/cxgb3/iwch_provider.c +15 -4
@@ -154,6 +154,7 @@ static struct ib_cq *iwch_create_cq(struct ib_device *ibdev, int entries, int vestruct iwch_create_cq_resp uresp;struct iwch_create_cq_req ureq;struct iwch_ucontext *ucontext =NULL ; +static int warned;PDBG ("%s ib_dev %p entries %d\n" , __func__, ibdev, entries); rhp =to_iwch_dev (ibdev);
@@ -217,15 +218,24 @@ static struct ib_cq *iwch_create_cq(struct ib_device *ibdev, int entries, int ve uresp.key = ucontext->key; ucontext->key +=PAGE_SIZE ;spin_unlock (&ucontext->mmap_lock); + mm->key = uresp.key; + mm->addr =virt_to_phys (chp->cq.queue); +if (udata->outlen <sizeof uresp) { +if (!warned++) + printk(KERN_WARNING MOD "Warning - " +"downlevel libcxgb3 (non-fatal).\n" ); + mm->len =PAGE_ALIGN ((1UL << uresp.size_log2) * +sizeof (struct t3_cqe )); + }else { + mm->len =PAGE_ALIGN (((1UL << uresp.size_log2) +1 ) * +sizeof (struct t3_cqe )); + uresp.memsize = mm->len; + }if (ib_copy_to_udata (udata, &uresp,sizeof (uresp))) {kfree (mm);iwch_destroy_cq (&chp->ibcq);return ERR_PTR (-EFAULT ); } - mm->key = uresp.key; - mm->addr =virt_to_phys (chp->cq.queue); - mm->len =PAGE_ALIGN ((1UL << uresp.size_log2) * -sizeof (struct t3_cqe ));insert_mmap (ucontext, mm); } PDBG("created cqid 0x%0x chp %p size 0x%0x, dma_addr 0x%0llx\n",
@@ -1414,6 +1424,7 @@ int iwch_register_device(struct iwch_dev *dev) dev->ibdev.post_send = iwch_post_send; dev->ibdev.post_recv = iwch_post_receive; dev->ibdev.get_protocol_stats = iwch_get_mib; + dev->ibdev.uverbs_abi_ver =IWCH_UVERBS_ABI_VERSION ; dev->ibdev.iwcm =kmalloc (sizeof (struct iw_cm_verbs ),GFP_KERNEL );if (!dev->ibdev.iwcm)
drivers/infiniband/hw/cxgb3/iwch_qp.c +18 -7
@@ -802,14 +802,12 @@ int iwch_post_terminate(struct iwch_qp *qhp, struct respQ_msg_t *rsp_msg)/* * Assumes qhp lock is held. */ -static void __flush_qp (struct iwch_qp *qhp,unsigned long *flag) +static void __flush_qp (struct iwch_qp *qhp,struct iwch_cq *rchp, +struct iwch_cq *schp,unsigned long *flag) { -struct iwch_cq *rchp, *schp;int count;int flushed; - rchp =get_chp (qhp->rhp, qhp->attr.rcq); - schp =get_chp (qhp->rhp, qhp->attr.scq);PDBG ("%s qhp %p rchp %p schp %p\n" , __func__, qhp, rchp, schp);/* take a ref on the qhp since we must release the lock */
@@ -847,10 +845,23 @@ static void __flush_qp(struct iwch_qp *qhp, unsigned long *flag)static void flush_qp (struct iwch_qp *qhp,unsigned long *flag) { -if (qhp->ibqp.uobject) +struct iwch_cq *rchp, *schp; + + rchp =get_chp (qhp->rhp, qhp->attr.rcq); + schp =get_chp (qhp->rhp, qhp->attr.scq); + +if (qhp->ibqp.uobject) {cxio_set_wq_in_error (&qhp->wq); -else -__flush_qp (qhp, flag); +cxio_set_cq_in_error (&rchp->cq); + (*rchp->ibcq.comp_handler)(&rchp->ibcq, rchp->ibcq.cq_context); +if (schp != rchp) { +cxio_set_cq_in_error (&schp->cq); + (*schp->ibcq.comp_handler)(&schp->ibcq, + schp->ibcq.cq_context); + } +return ; + } +__flush_qp (qhp, rchp, schp, flag); }
drivers/infiniband/hw/cxgb3/iwch_user.h +1 -0
@@ -49,6 +49,7 @@ struct iwch_create_cq_resp {__u64 key;__u32 cqid;__u32 size_log2; +__u32 memsize; };struct iwch_create_qp_resp {
--
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): 20101020214432.4869.50981.stgit-T4OLL4TyM9aNDNWfRnPdfg@public.gmane.org
Quoted 3 lines
- bump the ABI since the
create_cquverbs response changes.- detect older libraries and reduce the mmap size accordingly.
If you bump the ABI, will older libraries still run with the new driver? Or will they just bail out with "unsupported ABI version"?
If you can work with both old and new userspace libraries, is there any reason for an ABI break anyway?
- R.
--
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]
Quoted 8 lines · Roland Dreier
Quoted 3 lines
- bump the ABI since the
create_cquverbs response changes.- detect older libraries and reduce the mmap size accordingly.
If you bump the ABI, will older libraries still run with the new driver? Or will they just bail out with "unsupported ABI version"?
Well, the existing lib versions don't look at the ABI at all. :) So we're ok. And I put logic in the driver to detect the old libs vs ABI 1 libs.
Quoted 3 lines
If you can work with both old and new userspace libraries, is there any reason for an ABI break anyway?
The ABI version bump tells the new library that the kernel driver supports this new functionality. I don't know another way to do this without the ABI bump. The kernel driver detects the library ABI via the size of the create_cq response that's passed in from the lib. But in the other direction, I think the only was is with the ABI number.
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
PiBAQCAtMjE3LDE1ICsyMTgsMjQgQEAgc3RhdGljIHN0cnVjdCBpYl9jcSAqaXdjaF9jcmVhdGVf Y3Eoc3RydWN0IGliX2RldmljZQ0KPiAqaWJkZXYsIGludCBlbnRyaWVzLCBpbnQgdmUNCj4gIAkJ dXJlc3Aua2V5ID0gdWNvbnRleHQtPmtleTsNCj4gIAkJdWNvbnRleHQtPmtleSArPSBQQUdFX1NJ WkU7DQo+ICAJCXNwaW5fdW5sb2NrKCZ1Y29udGV4dC0+bW1hcF9sb2NrKTsNCj4gKwkJbW0tPmtl eSA9IHVyZXNwLmtleTsNCj4gKwkJbW0tPmFkZHIgPSB2aXJ0X3RvX3BoeXMoY2hwLT5jcS5xdWV1 ZSk7DQo+ICsJCWlmICh1ZGF0YS0+b3V0bGVuIDwgc2l6ZW9mIHVyZXNwKSB7DQo+ICsJCQlpZiAo IXdhcm5lZCsrKQ0KPiArCQkJCXByaW50ayhLRVJOX1dBUk5JTkcgTU9EICJXYXJuaW5nIC0gIg0K PiArCQkJCSAgICAgICAiZG93bmxldmVsIGxpYmN4Z2IzIChub24tZmF0YWwpLlxuIik7DQo+ICsJ CQltbS0+bGVuID0gUEFHRV9BTElHTigoMVVMIDw8IHVyZXNwLnNpemVfbG9nMikgKg0KPiArCQkJ CQkgICAgIHNpemVvZihzdHJ1Y3QgdDNfY3FlKSk7DQo+ICsJCX0gZWxzZSB7DQo+ICsJCQltbS0+ bGVuID0gUEFHRV9BTElHTigoKDFVTCA8PCB1cmVzcC5zaXplX2xvZzIpICsgMSkgKg0KPiArCQkJ CQkgICAgIHNpemVvZihzdHJ1Y3QgdDNfY3FlKSk7DQo+ICsJCQl1cmVzcC5tZW1zaXplID0gbW0t PmxlbjsNCj4gKwkJfQ0KPiAgCQlpZiAoaWJfY29weV90b191ZGF0YSh1ZGF0YSwgJnVyZXNwLCBz aXplb2YgKHVyZXNwKSkpIHsNCg0KSXMgdGhpcyBjb3JyZWN0PyAgdWRhdGEtPm91dGxlbiBjYW4g YmUgbGVzcyB0aGFuIHNpemVvZiB1cmVzcCBoZXJlLg0KDQo=
--
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): CF9C39F99A89134C9CF9C4CCB68B8DDF25B801FC1F-osO9UTpF0USkrb+BlOpmy7fspsVTdybXVpNB7YpNyf8@public.gmane.org
Quoted 13 lines · Sean
Quoted 13 lines
drivers/infiniband/hw/cxgb3/iwch_provider.c +8
-0@@ -217,15 +218,24 @@ static struct ib_cq *iwch_create_cq(struct ib_device *ibdev, int entries, int ve uresp.key = ucontext->key; ucontext->key +=PAGE_SIZE ;spin_unlock (&ucontext->mmap_lock); + mm->key = uresp.key; + mm->addr =virt_to_phys (chp->cq.queue); +if (udata->outlen<sizeof uresp) { +if (!warned++) + printk(KERN_WARNING MOD "Warning - " +"downlevel libcxgb3 (non-fatal).\n" ); + mm->len =PAGE_ALIGN ((1UL << uresp.size_log2) * +sizeof (struct t3_cqe ));
Quoted 10 lines
drivers/infiniband/hw/cxgb3/iwch_provider.c static struct ib_cq *iwch_create_cq(struct ib_device *ibdev, int entries, int ve
Quoted 7 lines
drivers/infiniband/hw/cxgb3/iwch_provider.c +5
-0+ }else { + mm->len =PAGE_ALIGN (((1UL << uresp.size_log2) +1 ) * +sizeof (struct t3_cqe )); + uresp.memsize = mm->len; + }if (ib_copy_to_udata (udata,&uresp,sizeof (uresp))) {Is this correct?
udata->outlencan be less than sizeof uresp here.
The only way it can be less is if the library is using the ABI 0 version of the response struct. Or am I missing something?
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
A message that is not in the archive (sent off-list or not yet received): [email protected]
WW91IGhhdmUgdGhpcyBjaGVjazoNCg0KPiA+PiArCQlpZiAodWRhdGEtPm91dGxlbjwgIHNpemVv ZiB1cmVzcCkgew0KPiA+PiArCQkJaWYgKCF3YXJuZWQrKykNCj4gPj4gKwkJCQlwcmludGsoS0VS Tl9XQVJOSU5HIE1PRCAiV2FybmluZyAtICINCj4gPj4gKwkJCQkgICAgICAgImRvd25sZXZlbCBs aWJjeGdiMyAobm9uLWZhdGFsKS5cbiIpOw0KPiA+PiArCQkJbW0tPmxlbiA9IFBBR0VfQUxJR04o KDFVTDw8ICB1cmVzcC5zaXplX2xvZzIpICoNCj4gPj4gKwkJCQkJICAgICBzaXplb2Yoc3RydWN0 IHQzX2NxZSkpOw0KDQp0aGVuIGZhbGwgdGhyb3VnaCB0byBoZXJlOg0KDQo+ID4+ICAgCQlpZiAo aWJfY29weV90b191ZGF0YSh1ZGF0YSwmdXJlc3AsIHNpemVvZiAodXJlc3ApKSkgew0KDQoNCj4g VGhlIG9ubHkgd2F5IGl0IGNhbiBiZSBsZXNzIGlzIGlmIHRoZSBsaWJyYXJ5IGlzIHVzaW5nIHRo ZSBBQkkgMCB2ZXJzaW9uDQo+IG9mIHRoZSByZXNwb25zZSBzdHJ1Y3QuICBPciBhbSBJIG1pc3Np bmcgc29tZXRoaW5nPw0KDQpUbyBtZSwgdGhpcyBsb29rcyBsaWtlIHRoZSBidWZmZXIgcmVmZXJl bmNlZCBieSB1ZGF0YSB3aWxsIGJlIHNtYWxsZXIgdGhhbiB0aGUgc2l6ZSBvZiB0aGUgZGF0YSBi ZWluZyBjb3BpZWQsIHdoaWNoIEkgZXhwZWN0IHdvdWxkIGVpdGhlciBmYWlsLCBvciB0aGUga2Vy bmVsIHdpbGwgb3ZlcndyaXRlIHNvbWUgdW5zdXNwZWN0aW5nIHVzZXIgbWVtb3J5Lg0K --
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): CF9C39F99A89134C9CF9C4CCB68B8DDF25B801FCA3-osO9UTpF0USkrb+BlOpmy7fspsVTdybXVpNB7YpNyf8@public.gmane.org
Quoted 10 lines · Sean
You have this check:
Quoted 6 lines
Quoted 6 lines
Review
Quoted 6 lines
(quoted diff) +6
-0+ if (udata->outlen< sizeof uresp) { + if (!warned++) + printk(KERN_WARNING MOD "Warning - " + "downlevel libcxgb3 (non-fatal).\n"); + mm->len = PAGE_ALIGN((1UL<< uresp.size_log2) * + sizeof(struct t3_cqe));then fall through to here:
Quoted 5 lines
Quoted 4 lines
Quoted 1 line
Quoted 1 line
if (ib_copy_to_udata(udata,&uresp, sizeof (uresp))) {The only way it can be less is if the library is using the ABI 0 version of the response struct. Or am I missing something?
To me, this looks like the buffer referenced by udata will be smaller than the size of the data being copied, which I expect would either fail, or the kernel will overwrite some unsuspecting user memory.
You're right: the ib_copy_to_udata() size needs to be adjusted accordingly. Good catch. Although the testing didn't cause any faults.
--
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