Re: [PATCH 3/4] net/ncsi: Use netdev_dbg for debug messages

[Samuel Mendoza-Jonas]

On Mon, 2018-06-18 at 13:53 -0700, Joe Perches wrote:
> On Mon, 2018-06-18 at 16:49 +0930, Joel Stanley wrote:
> > This moves all of the netdev_printk(KERN_DEBUG, ...) messages over to
> > netdev_dbg. There is no change in behaviour.
> 
> Not quite, but I think the patch is fine anyway.
> 
> netdev_printk(KERN_DEBUG ... is always emitted as
> long as the console level includes debug output.
> 
> netdev_dbg is not included in object code unless
> DEBUG is defined or CONFIG_DYNAMIC_DEBUG is set.
> And then, it is not emitted into the log unless
> DEBUG is set or this specific netdev_dbg is enabled
> via the dynamic debug control file.

Right this is fine for these sort of messages; very noisy and not particularly
critical.

For this and the other logging updates:
Acked-by: Samuel Mendoza-Jonas 

> 
> > Signed-off-by: Joel Stanley 
> > ---
> >  net/ncsi/ncsi-aen.c|  6 +++---
> >  net/ncsi/ncsi-manage.c | 33 +++--
> >  2 files changed, 18 insertions(+), 21 deletions(-)
> > 
> > diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
> > index f899ed61bb57..25e483e8278b 100644
> > --- a/net/ncsi/ncsi-aen.c
> > +++ b/net/ncsi/ncsi-aen.c
> > @@ -148,9 +148,9 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv 
> > *ndp,
> > hncdsc = (struct ncsi_aen_hncdsc_pkt *)h;
> > ncm->data[3] = ntohl(hncdsc->status);
> > spin_unlock_irqrestore(>lock, flags);
> > -   netdev_printk(KERN_DEBUG, ndp->ndev.dev,
> > - "NCSI: host driver %srunning on channel %u\n",
> > - ncm->data[3] & 0x1 ? "" : "not ", nc->id);
> > +   netdev_dbg(ndp->ndev.dev,
> > +  "NCSI: host driver %srunning on channel %u\n",
> > +  ncm->data[3] & 0x1 ? "" : "not ", nc->id);
> >  
> > return 0;
> >  }
> > diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> > index 716493a61ba6..091284760d21 100644
> > --- a/net/ncsi/ncsi-manage.c
> > +++ b/net/ncsi/ncsi-manage.c
> > @@ -788,8 +788,8 @@ static void ncsi_configure_channel(struct ncsi_dev_priv 
> > *ndp)
> > }
> > break;
> > case ncsi_dev_state_config_done:
> > -   netdev_printk(KERN_DEBUG, ndp->ndev.dev,
> > - "NCSI: channel %u config done\n", nc->id);
> > +   netdev_dbg(ndp->ndev.dev, "NCSI: channel %u config done\n",
> > +  nc->id);
> > spin_lock_irqsave(>lock, flags);
> > if (nc->reconfigure_needed) {
> > /* This channel's configuration has been updated
> > @@ -804,8 +804,7 @@ static void ncsi_configure_channel(struct ncsi_dev_priv 
> > *ndp)
> > list_add_tail_rcu(>link, >channel_queue);
> > spin_unlock_irqrestore(>lock, flags);
> >  
> > -   netdev_printk(KERN_DEBUG, dev,
> > - "Dirty NCSI channel state reset\n");
> > +   netdev_dbg(dev, "Dirty NCSI channel state reset\n");
> > ncsi_process_next_channel(ndp);
> > break;
> > }
> > @@ -908,9 +907,9 @@ static int ncsi_choose_active_channel(struct 
> > ncsi_dev_priv *ndp)
> > }
> >  
> > ncm = >modes[NCSI_MODE_LINK];
> > -   netdev_printk(KERN_DEBUG, ndp->ndev.dev,
> > - "NCSI: Channel %u added to queue (link %s)\n",
> > - found->id, ncm->data[2] & 0x1 ? "up" : "down");
> > +   netdev_dbg(ndp->ndev.dev,
> > +  "NCSI: Channel %u added to queue (link %s)\n",
> > +  found->id, ncm->data[2] & 0x1 ? "up" : "down");
> >  
> >  out:
> > spin_lock_irqsave(>lock, flags);
> > @@ -1316,9 +1315,9 @@ static int ncsi_kick_channels(struct ncsi_dev_priv 
> > *ndp)
> > if ((ndp->ndev.state & 0xff00) ==
> > ncsi_dev_state_config ||
> > !list_empty(>link)) {
> > -   netdev_printk(KERN_DEBUG, nd->dev,
> > - "NCSI: channel %p marked 
> > dirty\n",
> > - nc);
> > +   netdev_dbg(nd->dev,
> > +  "NCSI: channel %p marked 
> > dirty\n",
> > +  nc);
> > nc->reconfigure_needed = true;
> > }
> > spin_unlock_irqrestore(>lock, flags);
> > @@ -1336,8 +1335,7 @@ static int ncsi_kick_channels(struct ncsi_dev_priv 
> > *ndp)
> > list_add_tail_rcu(>link, >channel_queue);
> > spin_unlock_irqrestore(>lock, flags);
> >  
> > -   netdev_printk(KERN_DEBUG, nd->dev,
> > - "NCSI: kicked channel %p\n", nc);
> > +   netdev_dbg(nd->dev, "NCSI: kicked channel %p\n", nc);
> >  

Re: [PATCH 3/4] net/ncsi: Use netdev_dbg for debug messages

[Joe Perches]

On Mon, 2018-06-18 at 16:49 +0930, Joel Stanley wrote:
> This moves all of the netdev_printk(KERN_DEBUG, ...) messages over to
> netdev_dbg. There is no change in behaviour.

Not quite, but I think the patch is fine anyway.

netdev_printk(KERN_DEBUG ... is always emitted as
long as the console level includes debug output.

netdev_dbg is not included in object code unless
DEBUG is defined or CONFIG_DYNAMIC_DEBUG is set.
And then, it is not emitted into the log unless
DEBUG is set or this specific netdev_dbg is enabled
via the dynamic debug control file.

> Signed-off-by: Joel Stanley 
> ---
>  net/ncsi/ncsi-aen.c|  6 +++---
>  net/ncsi/ncsi-manage.c | 33 +++--
>  2 files changed, 18 insertions(+), 21 deletions(-)
> 
> diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
> index f899ed61bb57..25e483e8278b 100644
> --- a/net/ncsi/ncsi-aen.c
> +++ b/net/ncsi/ncsi-aen.c
> @@ -148,9 +148,9 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv 
> *ndp,
>   hncdsc = (struct ncsi_aen_hncdsc_pkt *)h;
>   ncm->data[3] = ntohl(hncdsc->status);
>   spin_unlock_irqrestore(>lock, flags);
> - netdev_printk(KERN_DEBUG, ndp->ndev.dev,
> -   "NCSI: host driver %srunning on channel %u\n",
> -   ncm->data[3] & 0x1 ? "" : "not ", nc->id);
> + netdev_dbg(ndp->ndev.dev,
> +"NCSI: host driver %srunning on channel %u\n",
> +ncm->data[3] & 0x1 ? "" : "not ", nc->id);
>  
>   return 0;
>  }
> diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> index 716493a61ba6..091284760d21 100644
> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -788,8 +788,8 @@ static void ncsi_configure_channel(struct ncsi_dev_priv 
> *ndp)
>   }
>   break;
>   case ncsi_dev_state_config_done:
> - netdev_printk(KERN_DEBUG, ndp->ndev.dev,
> -   "NCSI: channel %u config done\n", nc->id);
> + netdev_dbg(ndp->ndev.dev, "NCSI: channel %u config done\n",
> +nc->id);
>   spin_lock_irqsave(>lock, flags);
>   if (nc->reconfigure_needed) {
>   /* This channel's configuration has been updated
> @@ -804,8 +804,7 @@ static void ncsi_configure_channel(struct ncsi_dev_priv 
> *ndp)
>   list_add_tail_rcu(>link, >channel_queue);
>   spin_unlock_irqrestore(>lock, flags);
>  
> - netdev_printk(KERN_DEBUG, dev,
> -   "Dirty NCSI channel state reset\n");
> + netdev_dbg(dev, "Dirty NCSI channel state reset\n");
>   ncsi_process_next_channel(ndp);
>   break;
>   }
> @@ -908,9 +907,9 @@ static int ncsi_choose_active_channel(struct 
> ncsi_dev_priv *ndp)
>   }
>  
>   ncm = >modes[NCSI_MODE_LINK];
> - netdev_printk(KERN_DEBUG, ndp->ndev.dev,
> -   "NCSI: Channel %u added to queue (link %s)\n",
> -   found->id, ncm->data[2] & 0x1 ? "up" : "down");
> + netdev_dbg(ndp->ndev.dev,
> +"NCSI: Channel %u added to queue (link %s)\n",
> +found->id, ncm->data[2] & 0x1 ? "up" : "down");
>  
>  out:
>   spin_lock_irqsave(>lock, flags);
> @@ -1316,9 +1315,9 @@ static int ncsi_kick_channels(struct ncsi_dev_priv *ndp)
>   if ((ndp->ndev.state & 0xff00) ==
>   ncsi_dev_state_config ||
>   !list_empty(>link)) {
> - netdev_printk(KERN_DEBUG, nd->dev,
> -   "NCSI: channel %p marked 
> dirty\n",
> -   nc);
> + netdev_dbg(nd->dev,
> +"NCSI: channel %p marked 
> dirty\n",
> +nc);
>   nc->reconfigure_needed = true;
>   }
>   spin_unlock_irqrestore(>lock, flags);
> @@ -1336,8 +1335,7 @@ static int ncsi_kick_channels(struct ncsi_dev_priv *ndp)
>   list_add_tail_rcu(>link, >channel_queue);
>   spin_unlock_irqrestore(>lock, flags);
>  
> - netdev_printk(KERN_DEBUG, nd->dev,
> -   "NCSI: kicked channel %p\n", nc);
> + netdev_dbg(nd->dev, "NCSI: kicked channel %p\n", nc);
>   n++;
>   }
>   }
> @@ -1368,8 +1366,8 @@ int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16 
> proto, u16 vid)
>   list_for_each_entry_rcu(vlan, >vlan_vids, list) {
>   n_vids++;
>   if (vlan->vid == vid) {
> - netdev_printk(KERN_DEBUG, dev,
> -   

[PATCH 3/4] net/ncsi: Use netdev_dbg for debug messages

[Joel Stanley]

This moves all of the netdev_printk(KERN_DEBUG, ...) messages over to
netdev_dbg. There is no change in behaviour.

Signed-off-by: Joel Stanley 
---
 net/ncsi/ncsi-aen.c|  6 +++---
 net/ncsi/ncsi-manage.c | 33 +++--
 2 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
index f899ed61bb57..25e483e8278b 100644
--- a/net/ncsi/ncsi-aen.c
+++ b/net/ncsi/ncsi-aen.c
@@ -148,9 +148,9 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv 
*ndp,
hncdsc = (struct ncsi_aen_hncdsc_pkt *)h;
ncm->data[3] = ntohl(hncdsc->status);
spin_unlock_irqrestore(>lock, flags);
-   netdev_printk(KERN_DEBUG, ndp->ndev.dev,
- "NCSI: host driver %srunning on channel %u\n",
- ncm->data[3] & 0x1 ? "" : "not ", nc->id);
+   netdev_dbg(ndp->ndev.dev,
+  "NCSI: host driver %srunning on channel %u\n",
+  ncm->data[3] & 0x1 ? "" : "not ", nc->id);
 
return 0;
 }
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 716493a61ba6..091284760d21 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -788,8 +788,8 @@ static void ncsi_configure_channel(struct ncsi_dev_priv 
*ndp)
}
break;
case ncsi_dev_state_config_done:
-   netdev_printk(KERN_DEBUG, ndp->ndev.dev,
- "NCSI: channel %u config done\n", nc->id);
+   netdev_dbg(ndp->ndev.dev, "NCSI: channel %u config done\n",
+  nc->id);
spin_lock_irqsave(>lock, flags);
if (nc->reconfigure_needed) {
/* This channel's configuration has been updated
@@ -804,8 +804,7 @@ static void ncsi_configure_channel(struct ncsi_dev_priv 
*ndp)
list_add_tail_rcu(>link, >channel_queue);
spin_unlock_irqrestore(>lock, flags);
 
-   netdev_printk(KERN_DEBUG, dev,
- "Dirty NCSI channel state reset\n");
+   netdev_dbg(dev, "Dirty NCSI channel state reset\n");
ncsi_process_next_channel(ndp);
break;
}
@@ -908,9 +907,9 @@ static int ncsi_choose_active_channel(struct ncsi_dev_priv 
*ndp)
}
 
ncm = >modes[NCSI_MODE_LINK];
-   netdev_printk(KERN_DEBUG, ndp->ndev.dev,
- "NCSI: Channel %u added to queue (link %s)\n",
- found->id, ncm->data[2] & 0x1 ? "up" : "down");
+   netdev_dbg(ndp->ndev.dev,
+  "NCSI: Channel %u added to queue (link %s)\n",
+  found->id, ncm->data[2] & 0x1 ? "up" : "down");
 
 out:
spin_lock_irqsave(>lock, flags);
@@ -1316,9 +1315,9 @@ static int ncsi_kick_channels(struct ncsi_dev_priv *ndp)
if ((ndp->ndev.state & 0xff00) ==
ncsi_dev_state_config ||
!list_empty(>link)) {
-   netdev_printk(KERN_DEBUG, nd->dev,
- "NCSI: channel %p marked 
dirty\n",
- nc);
+   netdev_dbg(nd->dev,
+  "NCSI: channel %p marked 
dirty\n",
+  nc);
nc->reconfigure_needed = true;
}
spin_unlock_irqrestore(>lock, flags);
@@ -1336,8 +1335,7 @@ static int ncsi_kick_channels(struct ncsi_dev_priv *ndp)
list_add_tail_rcu(>link, >channel_queue);
spin_unlock_irqrestore(>lock, flags);
 
-   netdev_printk(KERN_DEBUG, nd->dev,
- "NCSI: kicked channel %p\n", nc);
+   netdev_dbg(nd->dev, "NCSI: kicked channel %p\n", nc);
n++;
}
}
@@ -1368,8 +1366,8 @@ int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16 
proto, u16 vid)
list_for_each_entry_rcu(vlan, >vlan_vids, list) {
n_vids++;
if (vlan->vid == vid) {
-   netdev_printk(KERN_DEBUG, dev,
- "NCSI: vid %u already registered\n", vid);
+   netdev_dbg(dev, "NCSI: vid %u already registered\n",
+  vid);
return 0;
}
}
@@ -1388,7 +1386,7 @@ int ncsi_vlan_rx_add_vid(struct net_device *dev, __be16 
proto, u16 vid)
vlan->vid = vid;
list_add_rcu(>list, >vlan_vids);
 
-   netdev_printk(KERN_DEBUG, dev, "NCSI: Added new vid %u\n", vid);
+