Re: [PATCH rdma-rc 7/9] IB/mlx5: Add VERBOSITY Kconfig option

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2994 bytes --]

On Fri, Sep 02, 2016 at 01:52:59PM -0400, Doug Ledford wrote:
> On 8/28/2016 3:58 AM, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> > This patch introduces new MLX5_INFINIBAND_VERBOSE Kconfig
> > option to enable the output of dump_wqe() which was never
> > printed.
> >
> > Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > ---
> >  drivers/infiniband/hw/mlx5/Kconfig | 6 ++++++
> >  drivers/infiniband/hw/mlx5/qp.c    | 2 +-
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/hw/mlx5/Kconfig b/drivers/infiniband/hw/mlx5/Kconfig
> > index bce263b..789fef4 100644
> > --- a/drivers/infiniband/hw/mlx5/Kconfig
> > +++ b/drivers/infiniband/hw/mlx5/Kconfig
> > @@ -6,3 +6,9 @@ config MLX5_INFINIBAND
> >  	  Mellanox Connect-IB PCI Express host channel adapters (HCAs).
> >  	  This is required to use InfiniBand protocols such as
> >  	  IP-over-IB or SRP with these devices.
> > +
> > +config MLX5_INFINIBAND_VERBOSE
> > +	bool "Enable ConnectX-4/Connect-IB verbosity"
> > +	depends on MLX5_INFINIBAND
> > +	---help---
> > +	  This is a configuration option to enable verbose output to easy debug.
> > diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
> > index 174d09b..67b58f7 100644
> > --- a/drivers/infiniband/hw/mlx5/qp.c
> > +++ b/drivers/infiniband/hw/mlx5/qp.c
> > @@ -3984,7 +3984,7 @@ int mlx5_ib_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
> >  			   get_fence(fence, wr), next_fence,
> >  			   mlx5_ib_opcode[wr->opcode]);
> >  skip_psv:
> > -		if (0)
> > +		if (IS_BUILTIN(CONFIG_MLX5_INFINIBAND_VERBOSE))
> >  			dump_wqe(qp, idx, size);
> >  	}
>
> This patch is just a big old bunch of no.  There is no way I'm sending
> in Kconfig changes in a late -rc.

Totally understandable, it was a combination of bad luck with my faults.
1. This patch was prepared before merge window.
2. Shared code was posted late.
3. The whole series was targeted to be sent for 4.9 and not for RCs.
Yuval's patch caused to change in my plans.

> Further, there is no way I'm sending
> in a Kconfig option for a single debug print function.

The goal of this config option is to preserver this print function which
is extremely useful for debug and development. Currently it is under if(0)
and can be removed by mistake.

> Third, there is
> no way I'm sending in a Kconfig setting for what should be a runtime
> debug switch or something like that.

It is in data-path and I wanted to ensure that no change in performance
will be observed. Dynamic configuration won't allow to achieve it.

Will it be acceptable by you to take this patch for for-next to ensure
no one is removing it by mistake? And i will explore the commonalities
in the different drivers to create common VERBOSE flag for whole
subsystem?

Thanks


>
>
> --
> Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>     GPG Key ID: 0E572FDD
>




[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]