On Thu, Jul 28, 2016 at 09:18:33AM -0400, Chuck Lever wrote: > Hi Eli- > > > On Jul 28, 2016, at 9:08 AM, Eli Cohen <eli@xxxxxxxxxxxx> wrote: > > > > I am not sure I agree with this. I intentionally put ENOMEM since this indicates that the WQ buffer is not large enough to contain all the s/g entries. I return EINVAL if the opcode is invalid for example. > > All other drivers I reviewed return EINVAL in this case. So > there is an implied API contract already. > > The functional issue here is that the consumer has attempted > to post a Send WR with an incorrect num_sge value. sq.max_gs > is a limit also specified by the consumer. There's no dynamic > memory allocation here to fail. > > ENOMEM would be correct if a dynamic memory allocation failed, > where posting again with the same arguments is likely to > succeed. In this case, an invalid parameter has been provided > by the consumer, which is a permanent error. > > Therefore EINVAL is the correct return. You are absolutely right, this is why I added my Acked-By. > > > > -----Original Message----- > > From: linux-rdma-owner@xxxxxxxxxxxxxxx [mailto:linux-rdma-owner@xxxxxxxxxxxxxxx] On Behalf Of Leon Romanovsky > > Sent: Thursday, July 28, 2016 12:23 AM > > To: Chuck Lever <chuck.lever@xxxxxxxxxx> > > Cc: Matan Barak <matanb@xxxxxxxxxxxx>; linux-rdma@xxxxxxxxxxxxxxx > > Subject: Re: [PATCH] IB-mlx5: Return EINVAL when caller specifies too many SGEs > > > > On Wed, Jul 27, 2016 at 03:38:36PM -0400, Chuck Lever wrote: > >> Other similar functions (mlx5_ib_post_recv being the closest) return > >> EINVAL in this case. > >> > >> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx> > > > > Thanks Chuck, > > You are absolutely right, it should be EINVAL error. > > Do you mind if I submit it by myself together with other mlx5 fixes? > > > > Looks ok, except title :) > > Acked-by: Leon Romanovsky <leonro@xxxxxxxxxxxx> > > > >> --- > >> Hi Matan, Leon- > >> > >> I noticed this nit while debugging a problem in xprtrdma. > >> > >> drivers/infiniband/hw/mlx5/qp.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/infiniband/hw/mlx5/qp.c > >> b/drivers/infiniband/hw/mlx5/qp.c index ce0a7ab..b0e5498 100644 > >> --- a/drivers/infiniband/hw/mlx5/qp.c > >> +++ b/drivers/infiniband/hw/mlx5/qp.c > >> @@ -3436,7 +3436,7 @@ int mlx5_ib_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr, > >> num_sge = wr->num_sge; > >> if (unlikely(num_sge > qp->sq.max_gs)) { > >> mlx5_ib_warn(dev, "\n"); > >> - err = -ENOMEM; > >> + err = -EINVAL; > >> *bad_wr = wr; > >> goto out; > >> } > >> > >> -- > >> To unsubscribe from this list: send the line "unsubscribe linux-rdma" > >> in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo > >> info at http://vger.kernel.org/majordomo-info.html > > -- > Chuck Lever > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html
Attachment:
signature.asc
Description: Digital signature
