[PATCH net-next-2.6] bonding: reduce noise during init

[PATCH net-next-2.6] bonding: reduce noise during init

[Andy Gospodarek]

Many are using sysfs to configure bonding rather than module options, so
there is no need for bonding to throw this warning in normal cases.

Keep the message around when debugging is enabled as it might be useful
for someone desperate enough to enable debugging, but eliminate it
otherwise.

Signed-off-by: Andy Gospodarek <andy@greyhouse.net>

---
 drivers/net/bonding/bond_main.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 61265f7..5ca23b1 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4745,7 +4745,10 @@ static int bond_check_params(struct bond_params *params)
 		/* miimon and arp_interval not set, we need one so things
 		 * work as expected, see bonding.txt for details
 		 */
-		pr_warning("Warning: either miimon or arp_interval and arp_ip_target module parameters must be specified, otherwise bonding will not detect link failures! see bonding.txt for details.\n");
+		pr_debug("Warning: either miimon or arp_interval and"
+			 " arp_ip_target module parameters must be specified,"
+			 " otherwise bonding will not detect link failures!"
+			 "  See bonding.txt for details.\n");
 	}
 
 	if (primary && !USES_PRIMARY(bond_mode)) {
-- 
1.7.4.4

Re: [PATCH net-next-2.6] bonding: reduce noise during init

[Joe Perches]

On Tue, 2011-07-26 at 17:35 -0400, Andy Gospodarek wrote:
> Many are using sysfs to configure bonding rather than module options, so
> there is no need for bonding to throw this warning in normal cases.
> Keep the message around when debugging is enabled as it might be useful
> for someone desperate enough to enable debugging, but eliminate it
> otherwise.
[]
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
[]
> @@ -4745,7 +4745,10 @@ static int bond_check_params(struct bond_params *params)
>  		/* miimon and arp_interval not set, we need one so things
>  		 * work as expected, see bonding.txt for details
>  		 */
> -		pr_warning("Warning: either miimon or arp_interval and arp_ip_target module parameters must be specified, otherwise bonding will not detect link failures! see bonding.txt for details.\n");
> +		pr_debug("Warning: either miimon or arp_interval and"
> +			 " arp_ip_target module parameters must be specified,"
> +			 " otherwise bonding will not detect link failures!"
> +			 "  See bonding.txt for details.\n");

I'd prefer you don't separate the format string
into multiple pieces.

Re: [PATCH net-next-2.6] bonding: reduce noise during init

[Jay Vosburgh]

Joe Perches <joe@perches.com> wrote:

>On Tue, 2011-07-26 at 17:35 -0400, Andy Gospodarek wrote:
>> Many are using sysfs to configure bonding rather than module options, so
>> there is no need for bonding to throw this warning in normal cases.
>> Keep the message around when debugging is enabled as it might be useful
>> for someone desperate enough to enable debugging, but eliminate it
>> otherwise.
>[]
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>[]
>> @@ -4745,7 +4745,10 @@ static int bond_check_params(struct bond_params *params)
>>  		/* miimon and arp_interval not set, we need one so things
>>  		 * work as expected, see bonding.txt for details
>>  		 */
>> -		pr_warning("Warning: either miimon or arp_interval and arp_ip_target module parameters must be specified, otherwise bonding will not detect link failures! see bonding.txt for details.\n");
>> +		pr_debug("Warning: either miimon or arp_interval and"
>> +			 " arp_ip_target module parameters must be specified,"
>> +			 " otherwise bonding will not detect link failures!"
>> +			 "  See bonding.txt for details.\n");
>
>I'd prefer you don't separate the format string
>into multiple pieces.

	Why not?  To me, it looks easier to read split into sections
that don't wrap lines.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

Re: [PATCH net-next-2.6] bonding: reduce noise during init

[Joe Perches]

On Tue, 2011-07-26 at 17:37 -0700, Jay Vosburgh wrote:
> Joe Perches <joe@perches.com> wrote:
> >I'd prefer you don't separate the format string
> >into multiple pieces.
> Why not?  To me, it looks easier to read split into sections
> that don't wrap lines.

Harder to grep for a dmesg and the
defect rate of these split formats is
typically higher than single strings
because of bad spacing between string
segments.

Re: [PATCH net-next-2.6] bonding: reduce noise during init

[Andy Gospodarek]

On Tue, Jul 26, 2011 at 05:40:27PM -0700, Joe Perches wrote:
> On Tue, 2011-07-26 at 17:37 -0700, Jay Vosburgh wrote:
> > Joe Perches <joe@perches.com> wrote:
> > >I'd prefer you don't separate the format string
> > >into multiple pieces.
> > Why not?  To me, it looks easier to read split into sections
> > that don't wrap lines.
> 
> Harder to grep for a dmesg and the
> defect rate of these split formats is
> typically higher than single strings
> because of bad spacing between string
> segments.
> 

I'll agree that grepping based on dmesg/log output can be more
difficult, but the visual benefit when looking at the code makes this
really nice.  That is why I would prefer to breakup long strings when
changing nearby code.

If spacing is incorrect it should simply be fixed -- that's not a
compelling reason to prevent splitting strings into multiple lines.

Re: [PATCH net-next-2.6] bonding: reduce noise during init

[Joe Perches]

On Tue, 2011-07-26 at 20:56 -0400, Andy Gospodarek wrote:
> On Jul 26, 2011 8:40 PM, "Joe Perches" <joe@perches.com> wrote:
> > On Tue, 2011-07-26 at 17:37 -0700, Jay Vosburgh wrote:
> > > Joe Perches <joe@perches.com> wrote:
> > > >I'd prefer you don't separate the format string
> > > >into multiple pieces.
> > > Why not?  To me, it looks easier to read split into sections
> > > that don't wrap lines.
> > Harder to grep for a dmesg and the
> > defect rate of these split formats is
> > typically higher than single strings
> > because of bad spacing between string
> > segments.
> I'll agree that grepping based on dmesg/log output can be more
> difficult, but incorrect spacing should simply be fixed rather than a
> wholesale ban.

I didn't know I can ban things.
Cool.  Do I get a sceptre too?

> The visual benefit when looking at the code makes this really nice, so
> I would prefer to breakup long strings when changing nearby code. 

Up to you.

One other possible benefit of keeping formats
together is that if the message is overly long,
(ideally dmesg should have messages < 80 chars)
then it's more obvious that it should be somehow
shortened.

cheers, Joe

Re: [PATCH net-next-2.6 v2] bonding: reduce noise during init

[Andy Gospodarek]

On Tue, Jul 26, 2011 at 05:40:27PM -0700, Joe Perches wrote:
> On Tue, 2011-07-26 at 17:37 -0700, Jay Vosburgh wrote:
> > Joe Perches <joe@perches.com> wrote:
> > >I'd prefer you don't separate the format string
> > >into multiple pieces.
> > Why not?  To me, it looks easier to read split into sections
> > that don't wrap lines.
> 
> Harder to grep for a dmesg and the
> defect rate of these split formats is
> typically higher than single strings
> because of bad spacing between string
> segments.
> 

I noticed that you took some time back in late 2009 to 'consolidate' the
split format-strings present in the bonding driver at the time and I've
decided I'm fine to leave them the way they are.  The main point of my
patch was to change the output and I would like to get that included.
Here is my updated patch...


Subject: [PATCH net-next-2.6 v2] bonding: reduce noise during init

Many are using sysfs to configure bonding rather than module options, so
there is no need for bonding to throw this warning in normal cases.

Keep the message around when debugging is enabled as it might be useful
for someone desperate enough to enable debugging, but eliminate it
otherwise.

Signed-off-by: Andy Gospodarek <andy@greyhouse.net>

---
 drivers/net/bonding/bond_main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 61265f7..b37c602 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4745,7 +4745,7 @@ static int bond_check_params(struct bond_params *params)
 		/* miimon and arp_interval not set, we need one so things
 		 * work as expected, see bonding.txt for details
 		 */
-		pr_warning("Warning: either miimon or arp_interval and arp_ip_target module parameters must be specified, otherwise bonding will not detect link failures! see bonding.txt for details.\n");
+		pr_debug("Warning: either miimon or arp_interval and arp_ip_target module parameters must be specified, otherwise bonding will not detect link failures! see bonding.txt for details.\n");
 	}
 
 	if (primary && !USES_PRIMARY(bond_mode)) {
-- 
1.7.4.4

Re: [PATCH net-next-2.6 v2] bonding: reduce noise during init

[David Miller]

From: Andy Gospodarek <andy@greyhouse.net>
Date: Wed, 27 Jul 2011 16:09:26 -0400

> Subject: [PATCH net-next-2.6 v2] bonding: reduce noise during init
> 
> Many are using sysfs to configure bonding rather than module options, so
> there is no need for bonding to throw this warning in normal cases.
> 
> Keep the message around when debugging is enabled as it might be useful
> for someone desperate enough to enable debugging, but eliminate it
> otherwise.
> 
> Signed-off-by: Andy Gospodarek <andy@greyhouse.net>

Applied.