[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <Y1E29kg8yuZjCV4v@unreal>
Date: Thu, 20 Oct 2022 14:54:30 +0300
From: Leon Romanovsky <leon@...nel.org>
To: Aru <aru.kolappan@...cle.com>
Cc: jgg@...pe.ca, saeedm@...dia.com, linux-rdma@...r.kernel.org,
linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
manjunath.b.patil@...cle.com, rama.nichanamatlu@...cle.com
Subject: Re: [PATCH 1/1] net/mlx5: add dynamic logging for mlx5_dump_err_cqe
On Thu, Oct 20, 2022 at 01:24:54AM -0700, Aru wrote:
> On 10/18/22 12:47 AM, Leon Romanovsky wrote:
> > On Fri, Oct 14, 2022 at 12:12:36PM -0700, Aru wrote:
> > > Hi Leon,
> > >
> > > Thank you for reviewing the patch.
> > >
> > > The method you mentioned disables the dump permanently for the kernel.
> > > We thought vendor might have enabled it for their consumption when needed.
> > > Hence we made it dynamic, so that it can be enabled/disabled at run time.
> > >
> > > Especially, in a production environment, having the option to turn this log
> > > on/off
> > > at runtime will be helpful.
> > While you are interested on/off this specific warning, your change will
> > cause "to hide" all syndromes as it is unlikely that anyone runs in
> > production with debug prints.
> >
> > - mlx5_ib_warn(dev, "dump error cqe\n");
> > + mlx5_ib_dbg(dev, "dump error cqe\n");
> >
> > Something like this will do the trick without interrupting to the others.
> >
> > diff --git a/drivers/infiniband/hw/mlx5/cq.c b/drivers/infiniband/hw/mlx5/cq.c
> > index 457f57b088c6..966206085eb3 100644
> > --- a/drivers/infiniband/hw/mlx5/cq.c
> > +++ b/drivers/infiniband/hw/mlx5/cq.c
> > @@ -267,10 +267,29 @@ static void handle_responder(struct ib_wc *wc, struct mlx5_cqe64 *cqe,
> > wc->wc_flags |= IB_WC_WITH_NETWORK_HDR_TYPE;
> > }
> > -static void dump_cqe(struct mlx5_ib_dev *dev, struct mlx5_err_cqe *cqe)
> > +static void dump_cqe(struct mlx5_ib_dev *dev, struct mlx5_err_cqe *cqe,
> > + struct ib_wc *wc, int dump)
> > {
> > - mlx5_ib_warn(dev, "dump error cqe\n");
> > - mlx5_dump_err_cqe(dev->mdev, cqe);
> > + const char *level;
> > +
> > + if (!dump)
> > + return;
> > +
> > + mlx5_ib_warn(dev, "WC error: %d, Message: %s\n", wc->status,
> > + ib_wc_status_msg(wc->status));
> > +
> > + if (dump == 1) {
> > + mlx5_ib_warn(dev, "dump error cqe\n");
> > + level = KERN_WARNING;
> > + }
> > +
> > + if (dump == 2) {
> > + mlx5_ib_dbg(dev, "dump error cqe\n");
> > + level = KERN_DEBUG;
> > + }
> > +
> > + print_hex_dump(level, "", DUMP_PREFIX_OFFSET, 16, 1, cqe, sizeof(*cqe),
> > + false);
> > }
> Hi Leon,
>
> Thank you for the reply and your suggested method to handle this debug
> logging.
>
> We set 'dump=2' for the syndromes applicable to our scenario:
> MLX5_CQE_SYNDROME_REMOTE_ACCESS_ERR,
> MLX5_CQE_SYNDROME_REMOTE_OP_ERR and MLX5_CQE_SYNDROME_LOCAL_PROT_ERR.
> We verified this code change and by default, the dump_cqe is not printed to
> syslog until
> the level is changed to KERN_DEBUG level. This works as expected.
>
> I will send out another email with the patch using your method.
>
> Is it fine with you If I add your name in the 'suggested-by' field in the
> new patch?
Whatever works for you.
Thanks
Powered by blists - more mailing lists