Re: [PATCH net v2] r8169:fix "rtl_counters_cond == 1 (loop: 1000, delay: 10)" log spam.
From: Francois Romieu <romieu@fr.zoreil.com>
Date: 2016-02-22 23:13:11
Also in:
lkml
Chunhao Lin [off-list ref] : [...]
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c index 537974c..a645f8d 100644 --- a/drivers/net/ethernet/realtek/r8169.c +++ b/drivers/net/ethernet/realtek/r8169.c@@ -7730,10 +7730,13 @@ rtl8169_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats) { struct rtl8169_private *tp = netdev_priv(dev); void __iomem *ioaddr = tp->mmio_addr; + struct pci_dev *pdev = tp->pci_dev;
+ struct device *d = &tp->pci_dev->dev; (the patch does not use pdev alone) [...]
quoted hunk ↗ jump to hunk
@@ -7761,7 +7764,8 @@ rtl8169_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats) * Fetch additonal counter values missing in stats collected by driver * from tally counters. */ - rtl8169_update_counters(dev); + if (pm_runtime_active(&pdev->dev)) + rtl8169_update_counters(dev);
pm_runtime_active() won't change after pm_runtime_get_noresume(). You may set a boolean active = pm_runtime_active(d) before testing netif_running(). [...]
quoted hunk ↗ jump to hunk
@@ -7842,6 +7848,12 @@ static int rtl8169_runtime_suspend(struct device *device) struct pci_dev *pdev = to_pci_dev(device); struct net_device *dev = pci_get_drvdata(pdev); struct rtl8169_private *tp = netdev_priv(dev); + void __iomem *ioaddr = tp->mmio_addr; + + /* Update counters before going runtime suspend */ + if (netif_running(dev)) + rtl8169_rx_missed(dev, ioaddr); + rtl8169_update_counters(dev); if (!tp->TxDescArray) return 0;
Nits:
- the tp->TxDescArray test provides the required synchronization: see
rtl8169_{open/close} and their pm_runtime_{get / put}.
- ioaddr is not really needed : tp->mmio_addr appears only once and it does
not mess the 72..80 cols limit.
- even if the device can only be automatically runtime suspended some time
after a link down event, you may address davem's point regarding stats
reliability and move rtl8169_rx_missed + rtl8169_update_counters after
rtl8169_net_suspend.
--
Ueimor