Re: [net-next PATCH 3/4] qlge: Reduce debug print output.

[Ron Mercer <ron.mercer@qlogic.com>]

Dave and Joe,

Thanks for your feedback on the printk macro.  I agree there is good
infrastructure for debug printing for network devices.  I will drop this
patch for now.  The main reason I did it this way was to remove compares from the data path without removing the messages.
I will respin the last patch.

Thanks,
Ron

On Mon, Nov 02, 2009 at 12:03:54AM -0800, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Fri, 30 Oct 2009 15:44:46 -0700
> 
> > On Fri, 2009-10-30 at 15:13 -0700, Ron Mercer wrote:
> >> Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
> > []
> >> diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
> >> index b9f65e0..502c3af 100644
> >> --- a/drivers/net/qlge/qlge.h
> >> +++ b/drivers/net/qlge/qlge.h
> >> @@ -27,6 +27,18 @@
> >>  		dev_printk(KERN_##klevel, &((qdev)->pdev->dev),	\
> >>  			   "%s: " fmt, __func__, ##args);  \
> >>         } while (0)
> >> +#if 0
> >> +#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...)	\
> >> +	do {	\
> >> +		if (!((qdev)->msg_enable & NETIF_MSG_##nlevel))	\
> >> +			;					\
> >> +		else						\
> >> +			dev_printk(KERN_##klevel, &((qdev)->pdev->dev),	\
> >> +					"%s: " fmt, __func__, ##args);  \
> >> +	} while (0)
> >> +#else
> >> +#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...)
> >> +#endif
> > 
> > This uses an inverted test and it doesn't verify the args to
> > dev_printk when not #defined.
> > 
> > How about:
> 
> I also don't like this kind of change for another reason.
> 
> The message levels are pointless if you're going to adhere to them
> or not based upon some CPP define.
> 
> Either do it, or don't.  Like every other driver does.
> 
> If for some reason the default is problematic, adjust the default that
> you pass to netif_msg_init() or, alternatively, adjust what level the
> debugging messages are assigned to.
> 
> We have all of this wonderful, full, infrastructure for message
> levelling.  And you can change the setting either at module load time
> or via ethtool.  The default is also up to you as well.
> 
> And you're going to stick a "#if 0" CPP control in there? :-/
> 
> No way, I absolutely won't accept this kind of change, there is no
> need for it.  There is more than enough dynamic flexibility, both at
> run-time via module option and ethtool message level selections, and
> at compile time via the default you can choose hoever you like.