IB: Return "maybe missed event" hint from ib_req_notify_cq()
authorRoland Dreier <rolandd@cisco.com>
Mon, 7 May 2007 04:02:48 +0000 (21:02 -0700)
committerRoland Dreier <rolandd@cisco.com>
Mon, 7 May 2007 04:18:11 +0000 (21:18 -0700)
The semantics defined by the InfiniBand specification say that
completion events are only generated when a completions is added to a
completion queue (CQ) after completion notification is requested.  In
other words, this means that the following race is possible:

while (CQ is not empty)
ib_poll_cq(CQ);
// new completion is added after while loop is exited
ib_req_notify_cq(CQ);
// no event is generated for the existing completion

To close this race, the IB spec recommends doing another poll of the
CQ after requesting notification.

However, it is not always possible to arrange code this way (for
example, we have found that NAPI for IPoIB cannot poll after
requesting notification).  Also, some hardware (eg Mellanox HCAs)
actually will generate an event for completions added before the call
to ib_req_notify_cq() -- which is allowed by the spec, since there's
no way for any upper-layer consumer to know exactly when a completion
was really added -- so the extra poll of the CQ is just a waste.

Motivated by this, we add a new flag "IB_CQ_REPORT_MISSED_EVENTS" for
ib_req_notify_cq() so that it can return a hint about whether the a
completion may have been added before the request for notification.
The return value of ib_req_notify_cq() is extended so:

 < 0 means an error occurred while requesting notification
== 0 means notification was requested successfully, and if
IB_CQ_REPORT_MISSED_EVENTS was passed in, then no
events were missed and it is safe to wait for another
event.
 > 0 is only returned if IB_CQ_REPORT_MISSED_EVENTS was
passed in.  It means that the consumer must poll the
CQ again to make sure it is empty to avoid the race
described above.

We add a flag to enable this behavior rather than turning it on
unconditionally, because checking for missed events may incur
significant overhead for some low-level drivers, and consumers that
don't care about the results of this test shouldn't be forced to pay
for the test.

Signed-off-by: Roland Dreier <rolandd@cisco.com>
12 files changed:
drivers/infiniband/hw/amso1100/c2.h
drivers/infiniband/hw/amso1100/c2_cq.c
drivers/infiniband/hw/cxgb3/cxio_hal.c
drivers/infiniband/hw/cxgb3/iwch_provider.c
drivers/infiniband/hw/ehca/ehca_iverbs.h
drivers/infiniband/hw/ehca/ehca_reqs.c
drivers/infiniband/hw/ehca/ipz_pt_fn.h
drivers/infiniband/hw/ipath/ipath_cq.c
drivers/infiniband/hw/ipath/ipath_verbs.h
drivers/infiniband/hw/mthca/mthca_cq.c
drivers/infiniband/hw/mthca/mthca_dev.h
include/rdma/ib_verbs.h

index 04a9db5de881ec61792f98a0644cf77ab868d4ae..fa58200217a1af1b1d3c4a18effcf08dd76da3a0 100644 (file)
@@ -519,7 +519,7 @@ extern void c2_free_cq(struct c2_dev *c2dev, struct c2_cq *cq);
 extern void c2_cq_event(struct c2_dev *c2dev, u32 mq_index);
 extern void c2_cq_clean(struct c2_dev *c2dev, struct c2_qp *qp, u32 mq_index);
 extern int c2_poll_cq(struct ib_cq *ibcq, int num_entries, struct ib_wc *entry);
-extern int c2_arm_cq(struct ib_cq *ibcq, enum ib_cq_notify notify);
+extern int c2_arm_cq(struct ib_cq *ibcq, enum ib_cq_notify_flags flags);
 
 /* CM */
 extern int c2_llp_connect(struct iw_cm_id *cm_id,
index 5175c99ee586cc34000450cc68d1a56b0fe1773c..d2b3366786d6db11f670b0986a73567ea36330f3 100644 (file)
@@ -217,17 +217,19 @@ int c2_poll_cq(struct ib_cq *ibcq, int num_entries, struct ib_wc *entry)
        return npolled;
 }
 
-int c2_arm_cq(struct ib_cq *ibcq, enum ib_cq_notify notify)
+int c2_arm_cq(struct ib_cq *ibcq, enum ib_cq_notify_flags notify_flags)
 {
        struct c2_mq_shared __iomem *shared;
        struct c2_cq *cq;
+       unsigned long flags;
+       int ret = 0;
 
        cq = to_c2cq(ibcq);
        shared = cq->mq.peer;
 
-       if (notify == IB_CQ_NEXT_COMP)
+       if ((notify_flags & IB_CQ_SOLICITED_MASK) == IB_CQ_NEXT_COMP)
                writeb(C2_CQ_NOTIFICATION_TYPE_NEXT, &shared->notification_type);
-       else if (notify == IB_CQ_SOLICITED)
+       else if ((notify_flags & IB_CQ_SOLICITED_MASK) == IB_CQ_SOLICITED)
                writeb(C2_CQ_NOTIFICATION_TYPE_NEXT_SE, &shared->notification_type);
        else
                return -EINVAL;
@@ -241,7 +243,13 @@ int c2_arm_cq(struct ib_cq *ibcq, enum ib_cq_notify notify)
         */
        readb(&shared->armed);
 
-       return 0;
+       if (notify_flags & IB_CQ_REPORT_MISSED_EVENTS) {
+               spin_lock_irqsave(&cq->lock, flags);
+               ret = !c2_mq_empty(&cq->mq);
+               spin_unlock_irqrestore(&cq->lock, flags);
+       }
+
+       return ret;
 }
 
 static void c2_free_cq_buf(struct c2_dev *c2dev, struct c2_mq *mq)
index f5e9aeec6f6e68748a54ad3d43f7edd520e5e191..76049afc76554ef2a4c0487b348faca768dfc3d7 100644 (file)
@@ -114,7 +114,10 @@ int cxio_hal_cq_op(struct cxio_rdev *rdev_p, struct t3_cq *cq,
                                return -EIO;
                        }
                }
+
+               return 1;
        }
+
        return 0;
 }
 
index 78a495f5332eaf76fad5e8ad9f9ce0e961cf61ba..a891493fd34017adbab711ce5a3cd0125c074c24 100644 (file)
@@ -292,7 +292,7 @@ static int iwch_resize_cq(struct ib_cq *cq, int cqe, struct ib_udata *udata)
 #endif
 }
 
-static int iwch_arm_cq(struct ib_cq *ibcq, enum ib_cq_notify notify)
+static int iwch_arm_cq(struct ib_cq *ibcq, enum ib_cq_notify_flags flags)
 {
        struct iwch_dev *rhp;
        struct iwch_cq *chp;
@@ -303,7 +303,7 @@ static int iwch_arm_cq(struct ib_cq *ibcq, enum ib_cq_notify notify)
 
        chp = to_iwch_cq(ibcq);
        rhp = chp->rhp;
-       if (notify == IB_CQ_SOLICITED)
+       if ((flags & IB_CQ_SOLICITED_MASK) == IB_CQ_SOLICITED)
                cq_op = CQ_ARM_SE;
        else
                cq_op = CQ_ARM_AN;
@@ -317,9 +317,11 @@ static int iwch_arm_cq(struct ib_cq *ibcq, enum ib_cq_notify notify)
        PDBG("%s rptr 0x%x\n", __FUNCTION__, chp->cq.rptr);
        err = cxio_hal_cq_op(&rhp->rdev, &chp->cq, cq_op, 0);
        spin_unlock_irqrestore(&chp->lock, flag);
-       if (err)
+       if (err < 0)
                printk(KERN_ERR MOD "Error %d rearming CQID 0x%x\n", err,
                       chp->cq.cqid);
+       if (err > 0 && !(flags & IB_CQ_REPORT_MISSED_EVENTS))
+               err = 0;
        return err;
 }
 
index aff96ac4fd125d04cef04fa533ba9f7e7eb0b7ce..e14b029332c8a59d23be4399003ec54bc31b39ba 100644 (file)
@@ -135,7 +135,7 @@ int ehca_poll_cq(struct ib_cq *cq, int num_entries, struct ib_wc *wc);
 
 int ehca_peek_cq(struct ib_cq *cq, int wc_cnt);
 
-int ehca_req_notify_cq(struct ib_cq *cq, enum ib_cq_notify cq_notify);
+int ehca_req_notify_cq(struct ib_cq *cq, enum ib_cq_notify_flags notify_flags);
 
 struct ib_qp *ehca_create_qp(struct ib_pd *pd,
                             struct ib_qp_init_attr *init_attr,
index 08d3f892d9f35695b7f83b17cfbd44b680d8d00f..caec9dee09e1b0df65a0e03fed29d2e16324d4c3 100644 (file)
@@ -634,11 +634,13 @@ poll_cq_exit0:
        return ret;
 }
 
-int ehca_req_notify_cq(struct ib_cq *cq, enum ib_cq_notify cq_notify)
+int ehca_req_notify_cq(struct ib_cq *cq, enum ib_cq_notify_flags notify_flags)
 {
        struct ehca_cq *my_cq = container_of(cq, struct ehca_cq, ib_cq);
+       unsigned long spl_flags;
+       int ret = 0;
 
-       switch (cq_notify) {
+       switch (notify_flags & IB_CQ_SOLICITED_MASK) {
        case IB_CQ_SOLICITED:
                hipz_set_cqx_n0(my_cq, 1);
                break;
@@ -649,5 +651,11 @@ int ehca_req_notify_cq(struct ib_cq *cq, enum ib_cq_notify cq_notify)
                return -EINVAL;
        }
 
-       return 0;
+       if (notify_flags & IB_CQ_REPORT_MISSED_EVENTS) {
+               spin_lock_irqsave(&my_cq->spinlock, spl_flags);
+               ret = ipz_qeit_is_valid(&my_cq->ipz_queue);
+               spin_unlock_irqrestore(&my_cq->spinlock, spl_flags);
+       }
+
+       return ret;
 }
index 8199c45768a321a9c89d54f18d0d03c759a5a7e0..57f141a36bceb2c6a5579ab4548f9d2c1cae3b2e 100644 (file)
@@ -140,6 +140,14 @@ static inline void *ipz_qeit_get_inc_valid(struct ipz_queue *queue)
        return cqe;
 }
 
+static inline int ipz_qeit_is_valid(struct ipz_queue *queue)
+{
+       struct ehca_cqe *cqe = ipz_qeit_get(queue);
+       u32 cqe_flags = cqe->cqe_flags;
+
+       return cqe_flags >> 7 == (queue->toggle_state & 1);
+}
+
 /*
  * returns and resets Queue Entry iterator
  * returns address (kv) of first Queue Entry
index 00d3eb9bc6969537f9f3bea31b9b3b186b2ce1af..3e9241badba059859fe500cd53661c9ce3dbf693 100644 (file)
@@ -334,17 +334,18 @@ int ipath_destroy_cq(struct ib_cq *ibcq)
 /**
  * ipath_req_notify_cq - change the notification type for a completion queue
  * @ibcq: the completion queue
- * @notify: the type of notification to request
+ * @notify_flags: the type of notification to request
  *
  * Returns 0 for success.
  *
  * This may be called from interrupt context.  Also called by
  * ib_req_notify_cq() in the generic verbs code.
  */
-int ipath_req_notify_cq(struct ib_cq *ibcq, enum ib_cq_notify notify)
+int ipath_req_notify_cq(struct ib_cq *ibcq, enum ib_cq_notify_flags notify_flags)
 {
        struct ipath_cq *cq = to_icq(ibcq);
        unsigned long flags;
+       int ret = 0;
 
        spin_lock_irqsave(&cq->lock, flags);
        /*
@@ -352,9 +353,15 @@ int ipath_req_notify_cq(struct ib_cq *ibcq, enum ib_cq_notify notify)
         * any other transitions (see C11-31 and C11-32 in ch. 11.4.2.2).
         */
        if (cq->notify != IB_CQ_NEXT_COMP)
-               cq->notify = notify;
+               cq->notify = notify_flags & IB_CQ_SOLICITED_MASK;
+
+       if ((notify_flags & IB_CQ_REPORT_MISSED_EVENTS) &&
+           cq->queue->head != cq->queue->tail)
+               ret = 1;
+
        spin_unlock_irqrestore(&cq->lock, flags);
-       return 0;
+
+       return ret;
 }
 
 /**
index 2d734fb6eff78a87168a37a527efea9ab6237fc6..7064fc22272765c8f943de8a7a5f35b176120f17 100644 (file)
@@ -741,7 +741,7 @@ struct ib_cq *ipath_create_cq(struct ib_device *ibdev, int entries, int comp_vec
 
 int ipath_destroy_cq(struct ib_cq *ibcq);
 
-int ipath_req_notify_cq(struct ib_cq *ibcq, enum ib_cq_notify notify);
+int ipath_req_notify_cq(struct ib_cq *ibcq, enum ib_cq_notify_flags notify_flags);
 
 int ipath_resize_cq(struct ib_cq *ibcq, int cqe, struct ib_udata *udata);
 
index efd79ef109a6295b0544779914ed81317a4d7614..cf0868f6e9653c5389a55b9dd15786ed22a387ae 100644 (file)
@@ -726,11 +726,12 @@ repoll:
        return err == 0 || err == -EAGAIN ? npolled : err;
 }
 
-int mthca_tavor_arm_cq(struct ib_cq *cq, enum ib_cq_notify notify)
+int mthca_tavor_arm_cq(struct ib_cq *cq, enum ib_cq_notify_flags flags)
 {
        __be32 doorbell[2];
 
-       doorbell[0] = cpu_to_be32((notify == IB_CQ_SOLICITED ?
+       doorbell[0] = cpu_to_be32(((flags & IB_CQ_SOLICITED_MASK) ==
+                                  IB_CQ_SOLICITED ?
                                   MTHCA_TAVOR_CQ_DB_REQ_NOT_SOL :
                                   MTHCA_TAVOR_CQ_DB_REQ_NOT)      |
                                  to_mcq(cq)->cqn);
@@ -743,7 +744,7 @@ int mthca_tavor_arm_cq(struct ib_cq *cq, enum ib_cq_notify notify)
        return 0;
 }
 
-int mthca_arbel_arm_cq(struct ib_cq *ibcq, enum ib_cq_notify notify)
+int mthca_arbel_arm_cq(struct ib_cq *ibcq, enum ib_cq_notify_flags flags)
 {
        struct mthca_cq *cq = to_mcq(ibcq);
        __be32 doorbell[2];
@@ -755,7 +756,8 @@ int mthca_arbel_arm_cq(struct ib_cq *ibcq, enum ib_cq_notify notify)
 
        doorbell[0] = ci;
        doorbell[1] = cpu_to_be32((cq->cqn << 8) | (2 << 5) | (sn << 3) |
-                                 (notify == IB_CQ_SOLICITED ? 1 : 2));
+                                 ((flags & IB_CQ_SOLICITED_MASK) ==
+                                  IB_CQ_SOLICITED ? 1 : 2));
 
        mthca_write_db_rec(doorbell, cq->arm_db);
 
@@ -766,7 +768,7 @@ int mthca_arbel_arm_cq(struct ib_cq *ibcq, enum ib_cq_notify notify)
        wmb();
 
        doorbell[0] = cpu_to_be32((sn << 28)                       |
-                                 (notify == IB_CQ_SOLICITED ?
+                                 ((flags & IB_CQ_SOLICITED_MASK) == IB_CQ_SOLICITED ?
                                   MTHCA_ARBEL_CQ_DB_REQ_NOT_SOL :
                                   MTHCA_ARBEL_CQ_DB_REQ_NOT)      |
                                  cq->cqn);
index b7e42efaf43df30a5981503af8d80f3111a17038..9bae3cc606039660884a72e97e06cfa7db765c43 100644 (file)
@@ -495,8 +495,8 @@ void mthca_unmap_eq_icm(struct mthca_dev *dev);
 
 int mthca_poll_cq(struct ib_cq *ibcq, int num_entries,
                  struct ib_wc *entry);
-int mthca_tavor_arm_cq(struct ib_cq *cq, enum ib_cq_notify notify);
-int mthca_arbel_arm_cq(struct ib_cq *cq, enum ib_cq_notify notify);
+int mthca_tavor_arm_cq(struct ib_cq *cq, enum ib_cq_notify_flags flags);
+int mthca_arbel_arm_cq(struct ib_cq *cq, enum ib_cq_notify_flags flags);
 int mthca_init_cq(struct mthca_dev *dev, int nent,
                  struct mthca_ucontext *ctx, u32 pdn,
                  struct mthca_cq *cq);
index 17cc309d03ef53cee9da92f34af2282647961c05..5342ac64ed1a786147a34546970234f57c2e1868 100644 (file)
@@ -431,9 +431,11 @@ struct ib_wc {
        u8                      port_num;       /* valid only for DR SMPs on switches */
 };
 
-enum ib_cq_notify {
-       IB_CQ_SOLICITED,
-       IB_CQ_NEXT_COMP
+enum ib_cq_notify_flags {
+       IB_CQ_SOLICITED                 = 1 << 0,
+       IB_CQ_NEXT_COMP                 = 1 << 1,
+       IB_CQ_SOLICITED_MASK            = IB_CQ_SOLICITED | IB_CQ_NEXT_COMP,
+       IB_CQ_REPORT_MISSED_EVENTS      = 1 << 2,
 };
 
 enum ib_srq_attr_mask {
@@ -990,7 +992,7 @@ struct ib_device {
                                              struct ib_wc *wc);
        int                        (*peek_cq)(struct ib_cq *cq, int wc_cnt);
        int                        (*req_notify_cq)(struct ib_cq *cq,
-                                                   enum ib_cq_notify cq_notify);
+                                                   enum ib_cq_notify_flags flags);
        int                        (*req_ncomp_notif)(struct ib_cq *cq,
                                                      int wc_cnt);
        struct ib_mr *             (*get_dma_mr)(struct ib_pd *pd,
@@ -1419,14 +1421,34 @@ int ib_peek_cq(struct ib_cq *cq, int wc_cnt);
 /**
  * ib_req_notify_cq - Request completion notification on a CQ.
  * @cq: The CQ to generate an event for.
- * @cq_notify: If set to %IB_CQ_SOLICITED, completion notification will
- *   occur on the next solicited event. If set to %IB_CQ_NEXT_COMP,
- *   notification will occur on the next completion.
+ * @flags:
+ *   Must contain exactly one of %IB_CQ_SOLICITED or %IB_CQ_NEXT_COMP
+ *   to request an event on the next solicited event or next work
+ *   completion at any type, respectively. %IB_CQ_REPORT_MISSED_EVENTS
+ *   may also be |ed in to request a hint about missed events, as
+ *   described below.
+ *
+ * Return Value:
+ *    < 0 means an error occurred while requesting notification
+ *   == 0 means notification was requested successfully, and if
+ *        IB_CQ_REPORT_MISSED_EVENTS was passed in, then no events
+ *        were missed and it is safe to wait for another event.  In
+ *        this case is it guaranteed that any work completions added
+ *        to the CQ since the last CQ poll will trigger a completion
+ *        notification event.
+ *    > 0 is only returned if IB_CQ_REPORT_MISSED_EVENTS was passed
+ *        in.  It means that the consumer must poll the CQ again to
+ *        make sure it is empty to avoid missing an event because of a
+ *        race between requesting notification and an entry being
+ *        added to the CQ.  This return value means it is possible
+ *        (but not guaranteed) that a work completion has been added
+ *        to the CQ since the last poll without triggering a
+ *        completion notification event.
  */
 static inline int ib_req_notify_cq(struct ib_cq *cq,
-                                  enum ib_cq_notify cq_notify)
+                                  enum ib_cq_notify_flags flags)
 {
-       return cq->device->req_notify_cq(cq, cq_notify);
+       return cq->device->req_notify_cq(cq, flags);
 }
 
 /**