* [PATCH libmlx5] fix err return values to match ibv_post_send expectations @ 2016-08-05 17:33 Jarod Wilson [not found] ` <1470418439-59245-1-git-send-email-jarod-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Jarod Wilson @ 2016-08-05 17:33 UTC (permalink / raw) To: linux-rdma-u79uwXL29TY76Z2rM5mHXA; +Cc: Jarod Wilson, Yishai Hadas The man page for ibv_post_send says: RETURN VALUE ibv_post_send() returns 0 on success, or the value of errno on failure (which indicates the failure reason). QEMU looks for the return value, and in the ENOMEM case, waits and retries, but with mlx5, it ends up dropping requests and hanging, because of the unexpected -1 return instead of ENOMEM. The fix is simple: set err = E<whatever> instead of -1, and eliminate use of errno = in _mlx5_post_send, just have mlx5_post_send assign the return from _mlx5_post_send in errno instead. This fix has been confirmed to resolves the issues seen with QEMU. Reported-by: Dr. David Alan Gilbert <dgilbert-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> Tested-by: Dr. David Alan Gilbert <dgilbert-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> CC: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> Signed-off-by: Jarod Wilson <jarod-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> --- src/qp.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/qp.c b/src/qp.c index 51e1176..2ad3ac0 100644 --- a/src/qp.c +++ b/src/qp.c @@ -590,8 +590,7 @@ static inline int _mlx5_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr, if (unlikely(wr->opcode < 0 || wr->opcode >= sizeof mlx5_ib_opcode / sizeof mlx5_ib_opcode[0])) { mlx5_dbg(fp, MLX5_DBG_QP_SEND, "bad opcode %d\n", wr->opcode); - errno = EINVAL; - err = -1; + err = EINVAL; *bad_wr = wr; goto out; } @@ -599,8 +598,7 @@ static inline int _mlx5_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr, if (unlikely(mlx5_wq_overflow(&qp->sq, nreq, to_mcq(qp->ibv_qp->send_cq)))) { mlx5_dbg(fp, MLX5_DBG_QP_SEND, "work queue overflow\n"); - errno = ENOMEM; - err = -1; + err = ENOMEM; *bad_wr = wr; goto out; } @@ -608,8 +606,7 @@ static inline int _mlx5_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr, if (unlikely(wr->num_sge > qp->sq.max_gs)) { mlx5_dbg(fp, MLX5_DBG_QP_SEND, "max gs exceeded %d (max = %d)\n", wr->num_sge, qp->sq.max_gs); - errno = ENOMEM; - err = -1; + err = ENOMEM; *bad_wr = wr; goto out; } @@ -918,7 +915,8 @@ int mlx5_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr, } #endif - return _mlx5_post_send(ibqp, wr, bad_wr); + errno = _mlx5_post_send(ibqp, wr, bad_wr); + return errno; } int mlx5_bind_mw(struct ibv_qp *qp, struct ibv_mw *mw, -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <1470418439-59245-1-git-send-email-jarod-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH libmlx5] fix err return values to match ibv_post_send expectations [not found] ` <1470418439-59245-1-git-send-email-jarod-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2016-08-05 18:01 ` Jason Gunthorpe [not found] ` <20160805180107.GA31674-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Jason Gunthorpe @ 2016-08-05 18:01 UTC (permalink / raw) To: Jarod Wilson; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Yishai Hadas On Fri, Aug 05, 2016 at 01:33:59PM -0400, Jarod Wilson wrote: > - return _mlx5_post_send(ibqp, wr, bad_wr); > + errno = _mlx5_post_send(ibqp, wr, bad_wr); > + return errno; Why still set errno? It was wrong in the first place.. Likely every use of errno in this provider should be reviewed. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20160805180107.GA31674-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]
* Re: [PATCH libmlx5] fix err return values to match ibv_post_send expectations [not found] ` <20160805180107.GA31674-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> @ 2016-08-06 18:48 ` Jarod Wilson [not found] ` <20160806184823.GT6329-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Jarod Wilson @ 2016-08-06 18:48 UTC (permalink / raw) To: Jason Gunthorpe; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Yishai Hadas On Fri, Aug 05, 2016 at 12:01:07PM -0600, Jason Gunthorpe wrote: > On Fri, Aug 05, 2016 at 01:33:59PM -0400, Jarod Wilson wrote: > > - return _mlx5_post_send(ibqp, wr, bad_wr); > > + errno = _mlx5_post_send(ibqp, wr, bad_wr); > > + return errno; > > Why still set errno? It was wrong in the first place.. > > Likely every use of errno in this provider should be reviewed. Wasn't sure if something else might actually consume errno elsewhere, since it's a global, and the other code path in that function set errno. Could certainly just stick with returning the _mlx5_post_send() result directly. -- Jarod Wilson jarod-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20160806184823.GT6329-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH libmlx5] fix err return values to match ibv_post_send expectations [not found] ` <20160806184823.GT6329-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2016-08-08 16:30 ` Jason Gunthorpe [not found] ` <20160808163018.GA1819-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Jason Gunthorpe @ 2016-08-08 16:30 UTC (permalink / raw) To: Jarod Wilson; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Yishai Hadas On Sat, Aug 06, 2016 at 02:48:23PM -0400, Jarod Wilson wrote: > On Fri, Aug 05, 2016 at 12:01:07PM -0600, Jason Gunthorpe wrote: > > On Fri, Aug 05, 2016 at 01:33:59PM -0400, Jarod Wilson wrote: > > > - return _mlx5_post_send(ibqp, wr, bad_wr); > > > + errno = _mlx5_post_send(ibqp, wr, bad_wr); > > > + return errno; > > > > Why still set errno? It was wrong in the first place.. > > > > Likely every use of errno in this provider should be reviewed. > > Wasn't sure if something else might actually consume errno elsewhere, > since it's a global, and the other code path in that function set errno. > Could certainly just stick with returning the _mlx5_post_send() result > directly. If the man page doesn't document the function destroying errno, then it should preserve it. Do not just randomly set errno. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20160808163018.GA1819-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]
* Re: [PATCH libmlx5] fix err return values to match ibv_post_send expectations [not found] ` <20160808163018.GA1819-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> @ 2016-08-08 16:52 ` Jarod Wilson [not found] ` <20160808165213.GA64520-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Jarod Wilson @ 2016-08-08 16:52 UTC (permalink / raw) To: Jason Gunthorpe; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Yishai Hadas On Mon, Aug 08, 2016 at 10:30:18AM -0600, Jason Gunthorpe wrote: > On Sat, Aug 06, 2016 at 02:48:23PM -0400, Jarod Wilson wrote: > > On Fri, Aug 05, 2016 at 12:01:07PM -0600, Jason Gunthorpe wrote: > > > On Fri, Aug 05, 2016 at 01:33:59PM -0400, Jarod Wilson wrote: > > > > - return _mlx5_post_send(ibqp, wr, bad_wr); > > > > + errno = _mlx5_post_send(ibqp, wr, bad_wr); > > > > + return errno; > > > > > > Why still set errno? It was wrong in the first place.. > > > > > > Likely every use of errno in this provider should be reviewed. > > > > Wasn't sure if something else might actually consume errno elsewhere, > > since it's a global, and the other code path in that function set errno. > > Could certainly just stick with returning the _mlx5_post_send() result > > directly. > > If the man page doesn't document the function destroying errno, then > it should preserve it. Do not just randomly set errno. $ man ibv_post_send ... RETURN VALUE ibv_post_send() returns 0 on success, or the value of errno on failure (which indicates the failure reason). ... This is a little bit terse, but looks to me like it means to say do set errno in ibv_post_send. I really am not terribly familiar with this code though, so I have no idea for sure if my interpretation is correct. Looks like libmlx4 does NOT twiddle errno though. I can certainly update the patch if consensus is that errno shouldn't be touched. Looks like mlx5_post_send() will need some additional work as well if that's the case though. -- Jarod Wilson jarod-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20160808165213.GA64520-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH libmlx5] fix err return values to match ibv_post_send expectations [not found] ` <20160808165213.GA64520-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2016-08-08 17:33 ` Jason Gunthorpe [not found] ` <20160808173333.GA26622-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Jason Gunthorpe @ 2016-08-08 17:33 UTC (permalink / raw) To: Jarod Wilson; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Yishai Hadas On Mon, Aug 08, 2016 at 12:52:13PM -0400, Jarod Wilson wrote: > > If the man page doesn't document the function destroying errno, then > > it should preserve it. Do not just randomly set errno. > > $ man ibv_post_send > ... > RETURN VALUE > ibv_post_send() returns 0 on success, or the value of errno on > failure (which indicates the failure reason). > ... > > This is a little bit terse, but looks to me like it means to say do set > errno in ibv_post_send. I really am not terribly familiar with this > code Yes, our man pages are a little baroque. The phrase 'returns the value of errno' means it returns a E* value, not errno itself and leaves errno alone. Using the phrase 'it returns an error number' would be clearer and more consistent with glibc documentation. > patch if consensus is that errno shouldn't be touched. Looks like > mlx5_post_send() will need some additional work as well if that's the case > though. Most likely. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20160808173333.GA26622-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]
* [PATCH v2 libmlx5] fix err return values to match ibv_post_send expectations [not found] ` <20160808173333.GA26622-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> @ 2016-08-08 18:04 ` Jarod Wilson [not found] ` <1470679455-28911-1-git-send-email-jarod-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Jarod Wilson @ 2016-08-08 18:04 UTC (permalink / raw) To: linux-rdma-u79uwXL29TY76Z2rM5mHXA Cc: Jarod Wilson, Jason Gunthorpe, Yishai Hadas The man page for ibv_post_send says: RETURN VALUE ibv_post_send() returns 0 on success, or the value of errno on failure (which indicates the failure reason). QEMU looks for the return value, and in the ENOMEM case, waits and retries, but with mlx5, it ends up dropping requests and hanging, because of the unexpected -1 return instead of ENOMEM. The fix is simple: set err = E<whatever> instead of -1, and eliminate use of errno = in _mlx5_post_send, just have mlx5_post_send return the err from _mlx5_post_send instead. This fix has been confirmed to resolves the issues seen with QEMU. While we're at it, fix the MW_DEBUG code paths to no muck with errno either. v2: per discussion with Jason Gunthorpe, don't set errno in mlx5_post_send Reported-by: Dr. David Alan Gilbert <dgilbert-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> Tested-by: Dr. David Alan Gilbert <dgilbert-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> CC: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> CC: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> Signed-off-by: Jarod Wilson <jarod-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> --- src/qp.c | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/src/qp.c b/src/qp.c index 51e1176..38352e9 100644 --- a/src/qp.c +++ b/src/qp.c @@ -590,8 +590,7 @@ static inline int _mlx5_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr, if (unlikely(wr->opcode < 0 || wr->opcode >= sizeof mlx5_ib_opcode / sizeof mlx5_ib_opcode[0])) { mlx5_dbg(fp, MLX5_DBG_QP_SEND, "bad opcode %d\n", wr->opcode); - errno = EINVAL; - err = -1; + err = EINVAL; *bad_wr = wr; goto out; } @@ -599,8 +598,7 @@ static inline int _mlx5_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr, if (unlikely(mlx5_wq_overflow(&qp->sq, nreq, to_mcq(qp->ibv_qp->send_cq)))) { mlx5_dbg(fp, MLX5_DBG_QP_SEND, "work queue overflow\n"); - errno = ENOMEM; - err = -1; + err = ENOMEM; *bad_wr = wr; goto out; } @@ -608,8 +606,7 @@ static inline int _mlx5_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr, if (unlikely(wr->num_sge > qp->sq.max_gs)) { mlx5_dbg(fp, MLX5_DBG_QP_SEND, "max gs exceeded %d (max = %d)\n", wr->num_sge, qp->sq.max_gs); - errno = ENOMEM; - err = -1; + err = ENOMEM; *bad_wr = wr; goto out; } @@ -899,22 +896,16 @@ int mlx5_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr, { #ifdef MW_DEBUG if (wr->opcode == IBV_WR_BIND_MW) { - if (wr->bind_mw.mw->type == IBV_MW_TYPE_1) { - errno = EINVAL; - return errno; - } + if (wr->bind_mw.mw->type == IBV_MW_TYPE_1) + return EINVAL; if (!wr->bind_mw.bind_info.mr || !wr->bind_mw.bind_info.addr || - !wr->bind_mw.bind_info.length) { - errno = EINVAL; - return errno; - } + !wr->bind_mw.bind_info.length) + return EINVAL; - if (wr->bind_mw.bind_info.mr->pd != wr->bind_mw.mw->pd) { - errno = EINVAL; - return errno; - } + if (wr->bind_mw.bind_info.mr->pd != wr->bind_mw.mw->pd) + return EINVAL; } #endif -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 10+ messages in thread
[parent not found: <1470679455-28911-1-git-send-email-jarod-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH v2 libmlx5] fix err return values to match ibv_post_send expectations [not found] ` <1470679455-28911-1-git-send-email-jarod-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2016-08-10 11:53 ` Yishai Hadas [not found] ` <b3afb94e-529f-38c4-6cdc-071f8ca3301b-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Yishai Hadas @ 2016-08-10 11:53 UTC (permalink / raw) To: Jarod Wilson Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Jason Gunthorpe, Yishai Hadas, Majd Dibbiny On 8/8/2016 9:04 PM, Jarod Wilson wrote: > The man page for ibv_post_send says: > > RETURN VALUE > > ibv_post_send() returns 0 on success, or the value of errno on failure > (which indicates the failure reason). > > QEMU looks for the return value, and in the ENOMEM case, waits and > retries, but with mlx5, it ends up dropping requests and hanging, because > of the unexpected -1 return instead of ENOMEM. > > The fix is simple: set err = E<whatever> instead of -1, and eliminate use > of errno = in _mlx5_post_send, just have mlx5_post_send return the err from > _mlx5_post_send instead. This fix has been confirmed to resolves the issues > seen with QEMU. > > While we're at it, fix the MW_DEBUG code paths to no muck with errno either. > > v2: per discussion with Jason Gunthorpe, don't set errno in mlx5_post_send Your patch missed few other flows that should be fixed as part of mlx5_post_send. (e.g. set_data_inl_seg which returns -1, set_bind_wr which touches errno, etc). Just fixed that with some other minor typos/changes to the commit log, will send it shortly as V3, left you as the Author to give you the credit for. In addition, Both mlx5_post_recv and mlx5_post_srq_recv should be fixed in the same manner, prepared another patch as some candidate fix will send it shortly as well. Once the new candidate patches will pass our regression testing will take them officially into libmlx5. > > Reported-by: Dr. David Alan Gilbert <dgilbert-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > Tested-by: Dr. David Alan Gilbert <dgilbert-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > CC: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> > CC: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> > Signed-off-by: Jarod Wilson <jarod-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > --- > src/qp.c | 27 +++++++++------------------ > 1 file changed, 9 insertions(+), 18 deletions(-) > > diff --git a/src/qp.c b/src/qp.c > index 51e1176..38352e9 100644 > --- a/src/qp.c > +++ b/src/qp.c > @@ -590,8 +590,7 @@ static inline int _mlx5_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr, > if (unlikely(wr->opcode < 0 || > wr->opcode >= sizeof mlx5_ib_opcode / sizeof mlx5_ib_opcode[0])) { > mlx5_dbg(fp, MLX5_DBG_QP_SEND, "bad opcode %d\n", wr->opcode); > - errno = EINVAL; > - err = -1; > + err = EINVAL; > *bad_wr = wr; > goto out; > } > @@ -599,8 +598,7 @@ static inline int _mlx5_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr, > if (unlikely(mlx5_wq_overflow(&qp->sq, nreq, > to_mcq(qp->ibv_qp->send_cq)))) { > mlx5_dbg(fp, MLX5_DBG_QP_SEND, "work queue overflow\n"); > - errno = ENOMEM; > - err = -1; > + err = ENOMEM; > *bad_wr = wr; > goto out; > } > @@ -608,8 +606,7 @@ static inline int _mlx5_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr, > if (unlikely(wr->num_sge > qp->sq.max_gs)) { > mlx5_dbg(fp, MLX5_DBG_QP_SEND, "max gs exceeded %d (max = %d)\n", > wr->num_sge, qp->sq.max_gs); > - errno = ENOMEM; > - err = -1; > + err = ENOMEM; > *bad_wr = wr; > goto out; > } > @@ -899,22 +896,16 @@ int mlx5_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr, > { > #ifdef MW_DEBUG > if (wr->opcode == IBV_WR_BIND_MW) { > - if (wr->bind_mw.mw->type == IBV_MW_TYPE_1) { > - errno = EINVAL; > - return errno; > - } > + if (wr->bind_mw.mw->type == IBV_MW_TYPE_1) > + return EINVAL; > > if (!wr->bind_mw.bind_info.mr || > !wr->bind_mw.bind_info.addr || > - !wr->bind_mw.bind_info.length) { > - errno = EINVAL; > - return errno; > - } > + !wr->bind_mw.bind_info.length) > + return EINVAL; > > - if (wr->bind_mw.bind_info.mr->pd != wr->bind_mw.mw->pd) { > - errno = EINVAL; > - return errno; > - } > + if (wr->bind_mw.bind_info.mr->pd != wr->bind_mw.mw->pd) > + return EINVAL; > } > #endif > > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <b3afb94e-529f-38c4-6cdc-071f8ca3301b-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>]
* Re: [PATCH v2 libmlx5] fix err return values to match ibv_post_send expectations [not found] ` <b3afb94e-529f-38c4-6cdc-071f8ca3301b-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> @ 2016-08-11 4:05 ` Jarod Wilson [not found] ` <20160811040539.GI29760-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 0 siblings, 1 reply; 10+ messages in thread From: Jarod Wilson @ 2016-08-11 4:05 UTC (permalink / raw) To: Yishai Hadas Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Jason Gunthorpe, Yishai Hadas, Majd Dibbiny On Wed, Aug 10, 2016 at 02:53:45PM +0300, Yishai Hadas wrote: > On 8/8/2016 9:04 PM, Jarod Wilson wrote: > >The man page for ibv_post_send says: > > > > RETURN VALUE > > > > ibv_post_send() returns 0 on success, or the value of errno on failure > > (which indicates the failure reason). > > > >QEMU looks for the return value, and in the ENOMEM case, waits and > >retries, but with mlx5, it ends up dropping requests and hanging, because > >of the unexpected -1 return instead of ENOMEM. > > > >The fix is simple: set err = E<whatever> instead of -1, and eliminate use > >of errno = in _mlx5_post_send, just have mlx5_post_send return the err from > >_mlx5_post_send instead. This fix has been confirmed to resolves the issues > >seen with QEMU. > > > >While we're at it, fix the MW_DEBUG code paths to no muck with errno either. > > > >v2: per discussion with Jason Gunthorpe, don't set errno in mlx5_post_send > > Your patch missed few other flows that should be fixed as part of > mlx5_post_send. (e.g. set_data_inl_seg which returns -1, set_bind_wr > which touches errno, etc). > > Just fixed that with some other minor typos/changes to the commit > log, will send it shortly as V3, left you as the Author to give you > the credit for. > > In addition, > Both mlx5_post_recv and mlx5_post_srq_recv should be fixed in the > same manner, prepared another patch as some candidate fix will send > it shortly as well. > > Once the new candidate patches will pass our regression testing will > take them officially into libmlx5. Works for me, thanks much. -- Jarod Wilson jarod-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <20160811040539.GI29760-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH v2 libmlx5] fix err return values to match ibv_post_send expectations [not found] ` <20160811040539.GI29760-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2016-08-11 10:18 ` Yishai Hadas 0 siblings, 0 replies; 10+ messages in thread From: Yishai Hadas @ 2016-08-11 10:18 UTC (permalink / raw) To: Jarod Wilson Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Jason Gunthorpe, Yishai Hadas, Majd Dibbiny On 8/11/2016 7:05 AM, Jarod Wilson wrote: >> Just fixed that with some other minor typos/changes to the commit >> log, will send it shortly as V3, left you as the Author to give you >> the credit for. >> >> In addition, >> Both mlx5_post_recv and mlx5_post_srq_recv should be fixed in the >> same manner, prepared another patch as some candidate fix will send >> it shortly as well. >> >> Once the new candidate patches will pass our regression testing will >> take them officially into libmlx5. > > Works for me, thanks much. > Fine, V3 and second patch to fix the recv flow were applied. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-08-11 10:18 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-08-05 17:33 [PATCH libmlx5] fix err return values to match ibv_post_send expectations Jarod Wilson [not found] ` <1470418439-59245-1-git-send-email-jarod-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2016-08-05 18:01 ` Jason Gunthorpe [not found] ` <20160805180107.GA31674-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 2016-08-06 18:48 ` Jarod Wilson [not found] ` <20160806184823.GT6329-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2016-08-08 16:30 ` Jason Gunthorpe [not found] ` <20160808163018.GA1819-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 2016-08-08 16:52 ` Jarod Wilson [not found] ` <20160808165213.GA64520-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2016-08-08 17:33 ` Jason Gunthorpe [not found] ` <20160808173333.GA26622-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> 2016-08-08 18:04 ` [PATCH v2 " Jarod Wilson [not found] ` <1470679455-28911-1-git-send-email-jarod-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2016-08-10 11:53 ` Yishai Hadas [not found] ` <b3afb94e-529f-38c4-6cdc-071f8ca3301b-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org> 2016-08-11 4:05 ` Jarod Wilson [not found] ` <20160811040539.GI29760-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2016-08-11 10:18 ` Yishai Hadas
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.