[PATCH for-next] iw_cxgb4: Change error/warn prints to pr_debug

[PATCH for-next] iw_cxgb4: Change error/warn prints to pr_debug

[Chelsio Cudbg]

From: Potnuri Bharat Teja <bharat-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>

These prints not neccesarily mean error, so changing them to debug.
Redifining pr_fmt to print the kernel module name, prints the module
name twice when +m flag of pr_debug is enabled, hence removing the
pr_fmt redefinition.

Signed-off-by: Potnuri Bharat Teja <bharat-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
---
 drivers/infiniband/hw/cxgb4/cm.c       | 4 ++--
 drivers/infiniband/hw/cxgb4/ev.c       | 2 +-
 drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 6 ------
 3 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index 21db3b48a617..eecdfb0c6be6 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -3567,8 +3567,8 @@ int c4iw_ep_disconnect(struct c4iw_ep *ep, int abrupt, gfp_t gfp)
 	case MORIBUND:
 	case ABORTING:
 	case DEAD:
-		pr_info("%s ignoring disconnect ep %p state %u\n",
-			__func__, ep, ep->com.state);
+		pr_debug("ignoring disconnect ep %p state %u\n",
+			 ep, ep->com.state);
 		break;
 	default:
 		WARN_ONCE(1, "Bad endpoint state %u\n", ep->com.state);
diff --git a/drivers/infiniband/hw/cxgb4/ev.c b/drivers/infiniband/hw/cxgb4/ev.c
index a252d5c40ae3..3e9d8b277ab9 100644
--- a/drivers/infiniband/hw/cxgb4/ev.c
+++ b/drivers/infiniband/hw/cxgb4/ev.c
@@ -236,7 +236,7 @@ int c4iw_ev_handler(struct c4iw_dev *dev, u32 qid)
 		if (atomic_dec_and_test(&chp->refcnt))
 			wake_up(&chp->wait);
 	} else {
-		pr_warn("%s unknown cqid 0x%x\n", __func__, qid);
+		pr_debug("unknown cqid 0x%x\n", qid);
 		spin_unlock_irqrestore(&dev->lock, flag);
 	}
 	return 0;
diff --git a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
index 470f97a79ebb..7d6ec29ec9d7 100644
--- a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
+++ b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
@@ -64,12 +64,6 @@
 #define DRV_NAME "iw_cxgb4"
 #define MOD DRV_NAME ":"
 
-#ifdef pr_fmt
-#undef pr_fmt
-#endif
-
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
 #include "t4.h"
 
 #define PBL_OFF(rdev_p, a) ((a) - (rdev_p)->lldi.vr->pbl.start)
-- 
2.5.3

--
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

Re: [PATCH for-next] iw_cxgb4: Change error/warn prints to pr_debug

[Potnuri Bharat Teja]

Sorry, Please ignore this sent with wrong username.
Shall send a proper one.

-Bharat

On Tuesday, December 12/19/17, 2017 at 15:38:28 +0530, Chelsio Cudbg wrote:
> From: Potnuri Bharat Teja <bharat-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
> 
> These prints not neccesarily mean error, so changing them to debug.
> Redifining pr_fmt to print the kernel module name, prints the module
> name twice when +m flag of pr_debug is enabled, hence removing the
> pr_fmt redefinition.
> 
> Signed-off-by: Potnuri Bharat Teja <bharat-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/infiniband/hw/cxgb4/cm.c       | 4 ++--
>  drivers/infiniband/hw/cxgb4/ev.c       | 2 +-
>  drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 6 ------
>  3 files changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
> index 21db3b48a617..eecdfb0c6be6 100644
> --- a/drivers/infiniband/hw/cxgb4/cm.c
> +++ b/drivers/infiniband/hw/cxgb4/cm.c
> @@ -3567,8 +3567,8 @@ int c4iw_ep_disconnect(struct c4iw_ep *ep, int abrupt, gfp_t gfp)
>  	case MORIBUND:
>  	case ABORTING:
>  	case DEAD:
> -		pr_info("%s ignoring disconnect ep %p state %u\n",
> -			__func__, ep, ep->com.state);
> +		pr_debug("ignoring disconnect ep %p state %u\n",
> +			 ep, ep->com.state);
>  		break;
>  	default:
>  		WARN_ONCE(1, "Bad endpoint state %u\n", ep->com.state);
> diff --git a/drivers/infiniband/hw/cxgb4/ev.c b/drivers/infiniband/hw/cxgb4/ev.c
> index a252d5c40ae3..3e9d8b277ab9 100644
> --- a/drivers/infiniband/hw/cxgb4/ev.c
> +++ b/drivers/infiniband/hw/cxgb4/ev.c
> @@ -236,7 +236,7 @@ int c4iw_ev_handler(struct c4iw_dev *dev, u32 qid)
>  		if (atomic_dec_and_test(&chp->refcnt))
>  			wake_up(&chp->wait);
>  	} else {
> -		pr_warn("%s unknown cqid 0x%x\n", __func__, qid);
> +		pr_debug("unknown cqid 0x%x\n", qid);
>  		spin_unlock_irqrestore(&dev->lock, flag);
>  	}
>  	return 0;
> diff --git a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
> index 470f97a79ebb..7d6ec29ec9d7 100644
> --- a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
> +++ b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
> @@ -64,12 +64,6 @@
>  #define DRV_NAME "iw_cxgb4"
>  #define MOD DRV_NAME ":"
>  
> -#ifdef pr_fmt
> -#undef pr_fmt
> -#endif
> -
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -
>  #include "t4.h"
>  
>  #define PBL_OFF(rdev_p, a) ((a) - (rdev_p)->lldi.vr->pbl.start)
> -- 
> 2.5.3
> 
--
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

[PATCH for-next] iw_cxgb4: Change error/warn prints to pr_debug

[Potnuri Bharat Teja]

These prints not neccesarily mean error, so changing them to debug.
Redifining pr_fmt to print the kernel module name, prints the module
name twice when +m flag of pr_debug is enabled, hence removing the
pr_fmt redefinition.

Signed-off-by: Potnuri Bharat Teja <bharat-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
---
 drivers/infiniband/hw/cxgb4/cm.c       | 4 ++--
 drivers/infiniband/hw/cxgb4/ev.c       | 2 +-
 drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 6 ------
 3 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index 21db3b48a617..eecdfb0c6be6 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -3567,8 +3567,8 @@ int c4iw_ep_disconnect(struct c4iw_ep *ep, int abrupt, gfp_t gfp)
 	case MORIBUND:
 	case ABORTING:
 	case DEAD:
-		pr_info("%s ignoring disconnect ep %p state %u\n",
-			__func__, ep, ep->com.state);
+		pr_debug("ignoring disconnect ep %p state %u\n",
+			 ep, ep->com.state);
 		break;
 	default:
 		WARN_ONCE(1, "Bad endpoint state %u\n", ep->com.state);
diff --git a/drivers/infiniband/hw/cxgb4/ev.c b/drivers/infiniband/hw/cxgb4/ev.c
index a252d5c40ae3..3e9d8b277ab9 100644
--- a/drivers/infiniband/hw/cxgb4/ev.c
+++ b/drivers/infiniband/hw/cxgb4/ev.c
@@ -236,7 +236,7 @@ int c4iw_ev_handler(struct c4iw_dev *dev, u32 qid)
 		if (atomic_dec_and_test(&chp->refcnt))
 			wake_up(&chp->wait);
 	} else {
-		pr_warn("%s unknown cqid 0x%x\n", __func__, qid);
+		pr_debug("unknown cqid 0x%x\n", qid);
 		spin_unlock_irqrestore(&dev->lock, flag);
 	}
 	return 0;
diff --git a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
index 470f97a79ebb..7d6ec29ec9d7 100644
--- a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
+++ b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
@@ -64,12 +64,6 @@
 #define DRV_NAME "iw_cxgb4"
 #define MOD DRV_NAME ":"
 
-#ifdef pr_fmt
-#undef pr_fmt
-#endif
-
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
 #include "t4.h"
 
 #define PBL_OFF(rdev_p, a) ((a) - (rdev_p)->lldi.vr->pbl.start)
-- 
2.5.3

--
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

Re: [PATCH for-next] iw_cxgb4: Change error/warn prints to pr_debug

[Jason Gunthorpe]

On Tue, Dec 19, 2017 at 04:14:16PM +0530, Potnuri Bharat Teja wrote:
> @@ -64,12 +64,6 @@
>  #define DRV_NAME "iw_cxgb4"
>  #define MOD DRV_NAME ":"
>  
> -#ifdef pr_fmt
> -#undef pr_fmt
> -#endif
> -
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -

Lots of other places in RDMA set a prefix, can we come to some agreement if we
should drop or keep this unverisally?

drivers/infiniband/core/mad.c:#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
drivers/infiniband/core/netlink.c:#define pr_fmt(fmt) "%s:%s: " fmt, KBUILD_MODNAME, __func__
drivers/infiniband/core/user_mad.c:#define pr_fmt(fmt) "user_mad: " fmt
drivers/infiniband/hw/cxgb3/cxio_hal.h:#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
drivers/infiniband/hw/cxgb4/iw_cxgb4.h:#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
drivers/infiniband/hw/hfi1/driver.c:#define pr_fmt(fmt) DRIVER_NAME ": " fmt
drivers/infiniband/hw/hfi1/file_ops.c:#define pr_fmt(fmt) DRIVER_NAME ": " fmt
drivers/infiniband/hw/hfi1/init.c:#define pr_fmt(fmt) DRIVER_NAME ": " fmt
drivers/infiniband/hw/mlx4/mlx4_ib.h:#define pr_fmt(fmt)        "<" MLX4_IB_DRV_NAME "> %s: " fmt, __func__
drivers/infiniband/hw/qib/qib_diag.c:#define pr_fmt(fmt) QIB_DRV_NAME ": " fmt
drivers/infiniband/hw/qib/qib_file_ops.c:#define pr_fmt(fmt) QIB_DRV_NAME ": " fmt
drivers/infiniband/hw/qib/qib_iba7322.c:#define pr_fmt(fmt) QIB_DRV_NAME " " fmt
drivers/infiniband/hw/qib/qib_init.c:#define pr_fmt(fmt) QIB_DRV_NAME ": " fmt
drivers/infiniband/sw/rxe/rxe.h:#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
drivers/infiniband/ulp/srp/ib_srp.c:#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
drivers/infiniband/ulp/srpt/ib_srpt.c:#define pr_fmt(fmt) DRV_NAME " " fmt
--
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

RE: [PATCH for-next] iw_cxgb4: Change error/warn prints to pr_debug

[Steve Wise]

> On Tue, Dec 19, 2017 at 04:14:16PM +0530, Potnuri Bharat Teja wrote:
> > @@ -64,12 +64,6 @@
> >  #define DRV_NAME "iw_cxgb4"
> >  #define MOD DRV_NAME ":"
> >
> > -#ifdef pr_fmt
> > -#undef pr_fmt
> > -#endif
> > -
> > -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > -
> 
> Lots of other places in RDMA set a prefix, can we come to some agreement
if
> we
> should drop or keep this unverisally?

Either way we go, pr_fmt being redefined screws up pr_debug() which
optionally prepends the function, line, module names depending on the
dynamic debug settings.  You end up with the module name embedded in the
middle of the log statement, and duplicated with +m is used.  That is why
Bharat removed it from iw_cxgb4.  

In general, I like module names in log statements, but we just need to do it
the right way to avoid the pr_debug issue...

steve


---
This email has been checked for viruses by AVG.
http://www.avg.com

--
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

Re: [PATCH for-next] iw_cxgb4: Change error/warn prints to pr_debug

[Jason Gunthorpe]

On Tue, Dec 19, 2017 at 03:12:01PM -0600, Steve Wise wrote:
> > On Tue, Dec 19, 2017 at 04:14:16PM +0530, Potnuri Bharat Teja wrote:
> > > @@ -64,12 +64,6 @@
> > >  #define DRV_NAME "iw_cxgb4"
> > >  #define MOD DRV_NAME ":"
> > >
> > > -#ifdef pr_fmt
> > > -#undef pr_fmt
> > > -#endif
> > > -
> > > -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > -
> > 
> > Lots of other places in RDMA set a prefix, can we come to some agreement
> if
> > we
> > should drop or keep this unverisally?
> 
> Either way we go, pr_fmt being redefined screws up pr_debug() which
> optionally prepends the function, line, module names depending on the
> dynamic debug settings.  You end up with the module name embedded in the
> middle of the log statement, and duplicated with +m is used.  That is why
> Bharat removed it from iw_cxgb4.  
> 
> In general, I like module names in log statements, but we just need to do it
> the right way to avoid the pr_debug issue...

Seems like a bigger problem than for linux-rdma?

Shouldn't many of these prints be using dev_* stuff and be prefixed
with their device name?

Usually pr should only be used in places where the device does not yet
exist.

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

RE: [PATCH for-next] iw_cxgb4: Change error/warn prints to pr_debug

[Steve Wise]

> 
> On Tue, Dec 19, 2017 at 03:12:01PM -0600, Steve Wise wrote:
> > > On Tue, Dec 19, 2017 at 04:14:16PM +0530, Potnuri Bharat Teja wrote:
> > > > @@ -64,12 +64,6 @@
> > > >  #define DRV_NAME "iw_cxgb4"
> > > >  #define MOD DRV_NAME ":"
> > > >
> > > > -#ifdef pr_fmt
> > > > -#undef pr_fmt
> > > > -#endif
> > > > -
> > > > -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > > -
> > >
> > > Lots of other places in RDMA set a prefix, can we come to some
agreement
> > if
> > > we
> > > should drop or keep this unverisally?
> >
> > Either way we go, pr_fmt being redefined screws up pr_debug() which
> > optionally prepends the function, line, module names depending on the
> > dynamic debug settings.  You end up with the module name embedded in the
> > middle of the log statement, and duplicated with +m is used.  That is
why
> > Bharat removed it from iw_cxgb4.
> >
> > In general, I like module names in log statements, but we just need to
do it
> > the right way to avoid the pr_debug issue...
> 
> Seems like a bigger problem than for linux-rdma?
> 

If redefining pr_fmt is actually the preferred way to add module names, then
I think pr_debug() might need
to be fixed.

> Shouldn't many of these prints be using dev_* stuff and be prefixed
> with their device name?

Yea, that makes sense.

steve


---
This email has been checked for viruses by AVG.
http://www.avg.com

--
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

Re: [PATCH for-next] iw_cxgb4: Change error/warn prints to pr_debug

[Jason Gunthorpe]

On Tue, Dec 19, 2017 at 04:32:54PM -0600, Steve Wise wrote:

> > > In general, I like module names in log statements, but we just
> > > need to do it the right way to avoid the pr_debug issue...
> > 
> > Seems like a bigger problem than for linux-rdma?
> > 
> 
> If redefining pr_fmt is actually the preferred way to add module names, then
> I think pr_debug() might need
> to be fixed.
> 
> > Shouldn't many of these prints be using dev_* stuff and be prefixed
> > with their device name?
> 
> Yea, that makes sense.

Okay, I've dropped this patch.

The two changes to pr_debug should probably become dev_dbg, and
resend?

Deal with the 'what should pr_fmt be' question someplace else :)

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

Re: [PATCH for-next] iw_cxgb4: Change error/warn prints to pr_debug

[Potnuri Bharat Teja]

On Friday, December 12/22/17, 2017 at 23:02:07 +0530, Jason Gunthorpe wrote:
> On Tue, Dec 19, 2017 at 04:32:54PM -0600, Steve Wise wrote:
> 
> > > > In general, I like module names in log statements, but we just
> > > > need to do it the right way to avoid the pr_debug issue...
> > > 
> > > Seems like a bigger problem than for linux-rdma?
> > > 
> > 
> > If redefining pr_fmt is actually the preferred way to add module names, then
> > I think pr_debug() might need
> > to be fixed.
> > 
> > > Shouldn't many of these prints be using dev_* stuff and be prefixed
> > > with their device name?
> > 
> > Yea, that makes sense.
> 
> Okay, I've dropped this patch.
> 
> The two changes to pr_debug should probably become dev_dbg, and
> resend?
For now I am resending the patch without pr_fmt change.
dev_dbg change would be an overall change if necessary.
> 
> Deal with the 'what should pr_fmt be' question someplace else :)
Agreed
> 
> 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

Re: [PATCH for-next] iw_cxgb4: Change error/warn prints to pr_debug

[Jason Gunthorpe]

On Fri, Dec 29, 2017 at 12:13:38PM +0530, Potnuri Bharat Teja wrote:
> On Friday, December 12/22/17, 2017 at 23:02:07 +0530, Jason Gunthorpe wrote:
> > On Tue, Dec 19, 2017 at 04:32:54PM -0600, Steve Wise wrote:
> > 
> > > > > In general, I like module names in log statements, but we just
> > > > > need to do it the right way to avoid the pr_debug issue...
> > > > 
> > > > Seems like a bigger problem than for linux-rdma?
> > > > 
> > > 
> > > If redefining pr_fmt is actually the preferred way to add module names, then
> > > I think pr_debug() might need
> > > to be fixed.
> > > 
> > > > Shouldn't many of these prints be using dev_* stuff and be prefixed
> > > > with their device name?
> > > 
> > > Yea, that makes sense.
> > 
> > Okay, I've dropped this patch.
> > 
> > The two changes to pr_debug should probably become dev_dbg, and
> > resend?
> For now I am resending the patch without pr_fmt change.
> dev_dbg change would be an overall change if necessary.

It is not appropriate for a driver to use pr_*, so you should consider
fixing it.

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