[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20091102155744.GA10894@linux-ox1b.qlogic.org>
Date: Mon, 2 Nov 2009 07:57:44 -0800
From: Ron Mercer <ron.mercer@...gic.com>
To: David Miller <davem@...emloft.net>
Cc: "joe@...ches.com" <joe@...ches.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [net-next PATCH 3/4] qlge: Reduce debug print output.
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@...ches.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@...gic.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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists