Thread (13 messages) 13 messages, 4 authors, 2003-09-15

Re: [PATCH 1/4] Net device error logging, revised

From: Jim Keniston <hidden>
Date: 2003-08-27 01:10:23
Also in: lkml

Jeff Garzik wrote:
Jim Keniston wrote:
quoted
#define netdev_printk(sevlevel, netdev, msglevel, format, arg...)     \
do {                                                                  \
if (NETIF_MSG_##msglevel == NETIF_MSG_ALL || ((netdev)->msg_enable & NETIF_MSG_##msglevel)) { \
      char pfx[40];                                                   \
      printk(sevlevel "%s: " format , make_netdev_msg_prefix(pfx, netdev) , ## arg);  \
}} while (0)

This would make your code bigger, but not that much bigger for the common case where
the msglevel is omitted (and the 'if(...)' is optimized out).
"NETIF_MSG_" is silly and should be eliminated.
From this, I infer that you think that the option to "omit" the msglevel arg --
e.g.,
	netdev_err(dev,, "NIC is fried!\n");	/* always logged */
-- is silly.  No big deal.  Its sole purpose is to help keep netdev_* calls terse.
A separate "NETIF_MSG_ALL" test is not needed, because msg_enable is a
bitmask.  A msg_enable of 0xffffffff will naturally create a NETIF_MSG_ALL.
But how do you code a netdev_* call where you ALWAYS want the message (including
netdev_printk-style prefix) logged, regardless of the value of msg_enable?  That's
what NETIF_MSG_ALL is for (and why it might be better called NETIF_MSG_ALWAYS)...
	netdev_err(dev, ALL, "NIC is fried!\n");	/* always logged */
or
	netdev_err(dev, ALWAYS, "NIC is fried!\n");	/* always logged */
Also, whatever mechanism is created, it needs to preserve the feature of
the existing system:

        if (a quick bitmask test)
                do something

And preferably "do something" is not inlined, because printk'ing --
although it may appear in a fast path during debugging -- cannot be
considered a fast path itself.

        Jeff
Sorry, I'm not sure what you're getting at here.  netdev_* doesn't prevent
people from using the existing netif_msg_* macros; it just provides shorthand
for the (usual) case where "do something" is "printk".

Jim
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help