| Message ID | 1421324368-6860-11-git-send-email-jeffrey.t.kirsher@intel.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Jeff Kirsher > From: Shannon Nelson <shannon.nelson@intel.com> > > The netdev name at init time often defaults to eth0 but later gets changed > by UDEV, so printing it here is misleading. Without the interface name you stand zero chance of working out which one it is. With it, and provided all the interface renames get into dmesg, you stand at least some chance. If you have several interfaces that use the same driver, but are subtly different you can have a hard enough time as it is working out what is going on. I know it is too late now, but UDEV should never have been allowed to rename devices, instead it should have added 'alias' names. Then you would stand some chance of working out which was which. > This patch removes the name > from the log messages, and removes the use of __func__ as we're not using > that any more in the driver. I can't remember, do these messages contain the name of the driver (as well as the device id string). You might need some help when grepping the kernel source to find where the messages are coming from. David -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> From: David Laight [mailto:David.Laight@ACULAB.COM] > Sent: Thursday, January 15, 2015 4:35 AM > > From: Jeff Kirsher > > From: Shannon Nelson <shannon.nelson@intel.com> > > > > The netdev name at init time often defaults to eth0 but later gets > changed > > by UDEV, so printing it here is misleading. > > Without the interface name you stand zero chance of working out which > one it is. > With it, and provided all the interface renames get into dmesg, you > stand > at least some chance. The dev_info() messages have the PCI device and function number in the string, so it's really not too hard to track down the resulting netdev port: Jan 13 15:46:55 snelson3-cup kernel: [621235.401627] i40e 0000:84:00.1: PHC enabled Later messages use netdev_info() and have both the device number and the netdev name: Jan 13 15:46:56 snelson3-cup kernel: [621236.508868] i40e 0000:04:00.1 p261p2: NIC Link is Up 10 Gbps Full Duplex, Flow Control: None Note that the driver name appears in both as well. sln -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: "Nelson, Shannon" <shannon.nelson@intel.com> Date: Thu, 15 Jan 2015 15:01:42 +0000 > The dev_info() messages have the PCI device and function number in the string, so it's really not too hard to track down the resulting netdev port: > Jan 13 15:46:55 snelson3-cup kernel: [621235.401627] i40e 0000:84:00.1: PHC enabled > > Later messages use netdev_info() and have both the device number and the netdev name: > Jan 13 15:46:56 snelson3-cup kernel: [621236.508868] i40e 0000:04:00.1 p261p2: NIC Link is Up 10 Gbps Full Duplex, Flow Control: None > > Note that the driver name appears in both as well. I think we should consistently provide the netdev name as soon as register_netdevice() returns. It is unwise for every driver to have their own private policy on this. Please change this back. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> From: David Miller [mailto:davem@davemloft.net] > Sent: Thursday, January 15, 2015 12:39 PM > > From: "Nelson, Shannon" <shannon.nelson@intel.com> > Date: Thu, 15 Jan 2015 15:01:42 +0000 > > > The dev_info() messages have the PCI device and function number in the > string, so it's really not too hard to track down the resulting netdev > port: > > Jan 13 15:46:55 snelson3-cup kernel: [621235.401627] i40e > 0000:84:00.1: PHC enabled > > > > Later messages use netdev_info() and have both the device number and > the netdev name: > > Jan 13 15:46:56 snelson3-cup kernel: [621236.508868] i40e 0000:04:00.1 > p261p2: NIC Link is Up 10 Gbps Full Duplex, Flow Control: None > > > > Note that the driver name appears in both as well. > > I think we should consistently provide the netdev name as soon as > register_netdevice() returns. > > It is unwise for every driver to have their own private policy on > this. > > Please change this back. Sure, we can simply drop this patch. <useless whine> Yes, the driver should print the unvarnished truth. What's frustrating about UDEV is that it ends up making the driver look like it is confused and/or lying in the log messages. We'd prefer to minimize this unnecessary user-confusion where possible. </useless whine> sln -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: "Nelson, Shannon" <shannon.nelson@intel.com> Date: Thu, 15 Jan 2015 22:44:48 +0000 > Yes, the driver should print the unvarnished truth. What's > frustrating about UDEV is that it ends up making the driver look > like it is confused and/or lying in the log messages. We'd prefer > to minimize this unnecessary user-confusion where possible. The only way to resolve this is to have some kind of synchronized event wherein when a device is instantiated we get userland to assign the final name before the device has any visibility anywhere (basically before register_netdevice() puts the device into the hashes where it can be found). -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c index a152878..4f69a84 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c @@ -285,8 +285,7 @@ void i40e_ptp_rx_hang(struct i40e_vsi *vsi) pf->last_rx_ptp_check = jiffies; pf->rx_hwtstamp_cleared++; dev_warn(&vsi->back->pdev->dev, - "%s: clearing Rx timestamp hang\n", - __func__); + "clearing PTP Rx timestamp hang\n"); } } @@ -632,7 +631,6 @@ static long i40e_ptp_create_clock(struct i40e_pf *pf) **/ void i40e_ptp_init(struct i40e_pf *pf) { - struct net_device *netdev = pf->vsi[pf->lan_vsi]->netdev; struct i40e_hw *hw = &pf->hw; u32 pf_id; long err; @@ -644,9 +642,7 @@ void i40e_ptp_init(struct i40e_pf *pf) I40E_PRTTSYN_CTL0_PF_ID_SHIFT; if (hw->pf_id != pf_id) { pf->flags &= ~I40E_FLAG_PTP; - dev_info(&pf->pdev->dev, "%s: PTP not supported on %s\n", - __func__, - netdev->name); + dev_info(&pf->pdev->dev, "PTP not supported on this device\n"); return; } @@ -659,14 +655,13 @@ void i40e_ptp_init(struct i40e_pf *pf) err = i40e_ptp_create_clock(pf); if (err) { pf->ptp_clock = NULL; - dev_err(&pf->pdev->dev, "%s: ptp_clock_register failed\n", - __func__); + dev_err(&pf->pdev->dev, + "PTP clock register failed: %ld\n", err); } else { struct timespec ts; u32 regval; - dev_info(&pf->pdev->dev, "%s: added PHC on %s\n", __func__, - netdev->name); + dev_info(&pf->pdev->dev, "PHC enabled\n"); pf->flags |= I40E_FLAG_PTP; /* Ensure the clocks are running. */ @@ -711,7 +706,7 @@ void i40e_ptp_stop(struct i40e_pf *pf) if (pf->ptp_clock) { ptp_clock_unregister(pf->ptp_clock); pf->ptp_clock = NULL; - dev_info(&pf->pdev->dev, "%s: removed PHC on %s\n", __func__, + dev_info(&pf->pdev->dev, "removed PHC from %s\n", pf->vsi[pf->lan_vsi]->netdev->name); } }