| Message ID | 20180102210419.26104.32513.stgit@phlsvlogin03.ph.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, Jan 02, 2018 at 04:04:20PM -0500, Alex Estrin wrote: > If IB_CQ_REPORT_MISSED_EVENTS flag is passed in ib_req_notify_cq() > it may return positive value indicating non-empty CQ. > If return code not verified the log might be flooded with false > warning messages "request notify on send CQ failed". > > Fixes: commit 8966e28d2e40 ("IB/ipoib: Use NAPI in UD/TX flows") Hurm... It was right before that commit as well.. Erez, why did you change it? + if (ib_req_notify_cq(priv->send_cq, IB_CQ_NEXT_COMP | + IB_CQ_REPORT_MISSED_EVENTS)) { + ipoib_warn(priv, "IPoIB/CM:request notify on send CQ failed\n"); ... - rc = ib_req_notify_cq(priv->send_cq, - IB_CQ_NEXT_COMP | IB_CQ_REPORT_MISSED_EVENTS); - if (rc < 0) - ipoib_warn(priv, "request notify on send CQ failed\n"); Alex, that is not the right format for the Fixes: line.. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 2, 2018 at 11:24 PM, Jason Gunthorpe <jgg@mellanox.com> wrote: > On Tue, Jan 02, 2018 at 04:04:20PM -0500, Alex Estrin wrote: >> If IB_CQ_REPORT_MISSED_EVENTS flag is passed in ib_req_notify_cq() >> it may return positive value indicating non-empty CQ. >> If return code not verified the log might be flooded with false >> warning messages "request notify on send CQ failed". >> >> Fixes: commit 8966e28d2e40 ("IB/ipoib: Use NAPI in UD/TX flows") > > Hurm... It was right before that commit as well.. > > Erez, why did you change it? I don't think i changed it. anyway, Alex's patch makes sense, even it is a rare case anyway, and no flooded will happened. > > + if (ib_req_notify_cq(priv->send_cq, IB_CQ_NEXT_COMP | > + IB_CQ_REPORT_MISSED_EVENTS)) { > + ipoib_warn(priv, "IPoIB/CM:request notify on send CQ failed\n"); > ... > - rc = ib_req_notify_cq(priv->send_cq, > - IB_CQ_NEXT_COMP | IB_CQ_REPORT_MISSED_EVENTS); > - if (rc < 0) > - ipoib_warn(priv, "request notify on send CQ failed\n"); > > Alex, that is not the right format for the Fixes: line.. > > Jason > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma- > owner@vger.kernel.org] On Behalf Of Jason Gunthorpe > Sent: Tuesday, January 2, 2018 4:24 PM > To: Estrin, Alex <alex.estrin@intel.com>; Erez Shitrit <erezsh@mellanox.com> > Cc: linux-rdma@vger.kernel.org > Subject: Re: [PATCH] IB/ipoib: Fix for notify send CQ failure messages > > On Tue, Jan 02, 2018 at 04:04:20PM -0500, Alex Estrin wrote: > > If IB_CQ_REPORT_MISSED_EVENTS flag is passed in ib_req_notify_cq() > > it may return positive value indicating non-empty CQ. > > If return code not verified the log might be flooded with false > > warning messages "request notify on send CQ failed". > > > > Fixes: commit 8966e28d2e40 ("IB/ipoib: Use NAPI in UD/TX flows") > > Hurm... It was right before that commit as well.. > > Erez, why did you change it? > > + if (ib_req_notify_cq(priv->send_cq, IB_CQ_NEXT_COMP | > + IB_CQ_REPORT_MISSED_EVENTS)) { > + ipoib_warn(priv, "IPoIB/CM:request notify on send CQ failed\n"); > ... > - rc = ib_req_notify_cq(priv->send_cq, > - IB_CQ_NEXT_COMP | IB_CQ_REPORT_MISSED_EVENTS); > - if (rc < 0) > - ipoib_warn(priv, "request notify on send CQ failed\n"); > > Alex, that is not the right format for the Fixes: line.. Jason, thanks for noticing. I'll resend v2 Alex. > Jason > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 03, 2018 at 11:33:05AM +0200, Erez Shitrit wrote: > On Tue, Jan 2, 2018 at 11:24 PM, Jason Gunthorpe <jgg@mellanox.com> wrote: > > On Tue, Jan 02, 2018 at 04:04:20PM -0500, Alex Estrin wrote: > >> If IB_CQ_REPORT_MISSED_EVENTS flag is passed in ib_req_notify_cq() > >> it may return positive value indicating non-empty CQ. > >> If return code not verified the log might be flooded with false > >> warning messages "request notify on send CQ failed". > >> > >> Fixes: commit 8966e28d2e40 ("IB/ipoib: Use NAPI in UD/TX flows") > > > > Hurm... It was right before that commit as well.. > > > > Erez, why did you change it? > > I don't think i changed it. Okay, then it is a typo when you reworked things for NAPI that the >0 case started making warnings. I will apply this patch. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c index 2c13123..8152724 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c @@ -766,12 +766,14 @@ void ipoib_cm_send(struct net_device *dev, struct sk_buff *skb, struct ipoib_cm_ skb_orphan(skb); skb_dst_drop(skb); - if (netif_queue_stopped(dev)) - if (ib_req_notify_cq(priv->send_cq, IB_CQ_NEXT_COMP | - IB_CQ_REPORT_MISSED_EVENTS)) { + if (netif_queue_stopped(dev)) { + rc = ib_req_notify_cq(priv->send_cq, IB_CQ_NEXT_COMP | + IB_CQ_REPORT_MISSED_EVENTS); + if (unlikely(rc < 0)) ipoib_warn(priv, "IPoIB/CM:request notify on send CQ failed\n"); + else if (rc) napi_schedule(&priv->send_napi); - } + } rc = post_send(priv, tx, tx->tx_head & (ipoib_sendq_size - 1), tx_req); if (unlikely(rc)) { diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c b/drivers/infiniband/ulp/ipoib/ipoib_ib.c index e6151a2..2865808 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c @@ -644,7 +644,7 @@ int ipoib_send(struct net_device *dev, struct sk_buff *skb, if (netif_queue_stopped(dev)) if (ib_req_notify_cq(priv->send_cq, IB_CQ_NEXT_COMP | - IB_CQ_REPORT_MISSED_EVENTS)) + IB_CQ_REPORT_MISSED_EVENTS) < 0) ipoib_warn(priv, "request notify on send CQ failed\n"); rc = post_send(priv, priv->tx_head & (ipoib_sendq_size - 1),
If IB_CQ_REPORT_MISSED_EVENTS flag is passed in ib_req_notify_cq() it may return positive value indicating non-empty CQ. If return code not verified the log might be flooded with false warning messages "request notify on send CQ failed". Fixes: commit 8966e28d2e40 ("IB/ipoib: Use NAPI in UD/TX flows") Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com> Signed-off-by: Alex Estrin <alex.estrin@intel.com> --- drivers/infiniband/ulp/ipoib/ipoib_cm.c | 10 ++++++---- drivers/infiniband/ulp/ipoib/ipoib_ib.c | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html