| Message ID | 20170118121036.32642-3-leon@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Wed, Jan 18, 2017 at 02:10:31PM +0200, Leon Romanovsky wrote: > From: Leon Romanovsky <leonro@mellanox.com> > > In case of unsupported singature, we returned positive > value, while the better approach is to return -EINVAL. > > In addition, in this change, the error print is enriched > to provide an actual supplied signature type. What's the reason for the empty warnings i see in callers of this function? ("mlx5_ib_warn(dev, "\n");". Can we remove these while we are here? > > Fixes: e6631814fb3a ("IB/mlx5: Support IB_WR_REG_SIG_MR") > Cc: Sagi Grimberg <sagi@grimberg.me> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com> > Signed-off-by: Leon Romanovsky <leon@kernel.org> > --- > drivers/infiniband/hw/mlx5/qp.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c > index 6a83fb3..9021074 100644 > --- a/drivers/infiniband/hw/mlx5/qp.c > +++ b/drivers/infiniband/hw/mlx5/qp.c > @@ -3637,8 +3637,9 @@ static int set_psv_wr(struct ib_sig_domain *domain, > psv_seg->ref_tag = cpu_to_be32(domain->sig.dif.ref_tag); > break; > default: > - pr_err("Bad signature type given.\n"); > - return 1; > + pr_err("Bad signature type (%d) is given.\n", > + domain->sig_type); > + return -EINVAL; > } > > *seg += sizeof(*psv_seg); > -- > 2.10.2 > > -- > 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 18, 2017 at 10:37:09PM +0200, Yuval Shaia wrote: > On Wed, Jan 18, 2017 at 02:10:31PM +0200, Leon Romanovsky wrote: > > From: Leon Romanovsky <leonro@mellanox.com> > > > > In case of unsupported singature, we returned positive > > value, while the better approach is to return -EINVAL. > > > > In addition, in this change, the error print is enriched > > to provide an actual supplied signature type. > > What's the reason for the empty warnings i see in callers of this function? > ("mlx5_ib_warn(dev, "\n");". mlx5_ib_warn prints line and function name and in this format it mimics tracepoint. > > Can we remove these while we are here? mlx5 prints deserve complete series, there are a lot of them need to be revised. I prefer to do it all it once. > > > > > Fixes: e6631814fb3a ("IB/mlx5: Support IB_WR_REG_SIG_MR") > > Cc: Sagi Grimberg <sagi@grimberg.me> > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > > Signed-off-by: Leon Romanovsky <leonro@mellanox.com> > > Signed-off-by: Leon Romanovsky <leon@kernel.org> > > --- > > drivers/infiniband/hw/mlx5/qp.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c > > index 6a83fb3..9021074 100644 > > --- a/drivers/infiniband/hw/mlx5/qp.c > > +++ b/drivers/infiniband/hw/mlx5/qp.c > > @@ -3637,8 +3637,9 @@ static int set_psv_wr(struct ib_sig_domain *domain, > > psv_seg->ref_tag = cpu_to_be32(domain->sig.dif.ref_tag); > > break; > > default: > > - pr_err("Bad signature type given.\n"); > > - return 1; > > + pr_err("Bad signature type (%d) is given.\n", > > + domain->sig_type); > > + return -EINVAL; > > } > > > > *seg += sizeof(*psv_seg); > > -- > > 2.10.2 > > > > -- > > 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
>> What's the reason for the empty warnings i see in callers of this function? >> ("mlx5_ib_warn(dev, "\n");". > > mlx5_ib_warn prints line and function name and in this format it mimics tracepoint. > >> >> Can we remove these while we are here? > > mlx5 prints deserve complete series, there are a lot of them need to be > revised. I prefer to do it all it once. I have to agree with Leon, that would help a lot. -- 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/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c index 6a83fb3..9021074 100644 --- a/drivers/infiniband/hw/mlx5/qp.c +++ b/drivers/infiniband/hw/mlx5/qp.c @@ -3637,8 +3637,9 @@ static int set_psv_wr(struct ib_sig_domain *domain, psv_seg->ref_tag = cpu_to_be32(domain->sig.dif.ref_tag); break; default: - pr_err("Bad signature type given.\n"); - return 1; + pr_err("Bad signature type (%d) is given.\n", + domain->sig_type); + return -EINVAL; } *seg += sizeof(*psv_seg);