>From 2f02fd78d644e076bc11b24e4f16c89883aa8a24 Mon Sep 17 00:00:00 2001 Message-Id: <2f02fd78d644e076bc11b24e4f16c89883aa8a24.1441670158.git.romieu@fr.zoreil.com> In-Reply-To: <8f1b87a115f6c340558b95f6a1748ddbf866d95f.1441670158.git.romieu@fr.zoreil.com> References: <8f1b87a115f6c340558b95f6a1748ddbf866d95f.1441670158.git.romieu@fr.zoreil.com> From: Francois Romieu Date: Tue, 8 Sep 2015 00:11:58 +0200 Subject: [PATCH 3/3] r8169: increase the lifespan of the hardware counters dump area. X-Organisation: Land of Sunshine Inc. It avoids sleepable allocation when retrieving statistics as they can happen with spinlock held (see net/core/net-sysfs.c::netstat_show). - receive ring, transmit ring and counters dump area allocation failures are all considered fatal during open(). - netif_warn is now redundant with rtl_reset_counters_cond built-in failure message. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=104031 Fixes: 6e85d5ad36a2 ("r8169: Add values missing in @get_stats64 from HW counters") Signed-off-by: Francois Romieu Cc: Corinna Vinschen --- drivers/net/ethernet/realtek/r8169.c | 112 +++++++++++++---------------------- 1 file changed, 42 insertions(+), 70 deletions(-) diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c index f8a81a9..afe7810 100644 --- a/drivers/net/ethernet/realtek/r8169.c +++ b/drivers/net/ethernet/realtek/r8169.c @@ -833,8 +833,11 @@ struct rtl8169_private { unsigned features; struct mii_if_info mii; + struct rtl8169_counters *counters; struct rtl8169_tc_offsets tc_offset; + dma_addr_t counters_map; + u32 saved_wolopts; u32 opts1_mask; @@ -2197,104 +2200,65 @@ DECLARE_RTL_COND(rtl_cmd_counters_cond) return RTL_R32(CounterAddrLow) & (CounterReset | CounterDump); } -static struct rtl8169_counters *rtl8169_map_counters(struct net_device *dev, - dma_addr_t *paddr, - u32 counter_cmd) +static int rtl8169_cmd_counters(struct net_device *dev, u32 counter_cmd) { struct rtl8169_private *tp = netdev_priv(dev); void __iomem *ioaddr = tp->mmio_addr; - struct device *d = &tp->pci_dev->dev; - struct rtl8169_counters *counters; + dma_addr_t paddr = tp->counters_map; u32 cmd; + int rc; - counters = dma_alloc_coherent(d, sizeof(*counters), paddr, GFP_KERNEL); - if (counters) { - RTL_W32(CounterAddrHigh, (u64)*paddr >> 32); - cmd = (u64)*paddr & DMA_BIT_MASK(32); - RTL_W32(CounterAddrLow, cmd); - RTL_W32(CounterAddrLow, cmd | counter_cmd); - } - return counters; -} + RTL_W32(CounterAddrHigh, (u64)paddr >> 32); + cmd = (u64)paddr & DMA_BIT_MASK(32); + RTL_W32(CounterAddrLow, cmd); + RTL_W32(CounterAddrLow, cmd | counter_cmd); -static void rtl8169_unmap_counters (struct net_device *dev, - dma_addr_t paddr, - struct rtl8169_counters *counters) -{ - struct rtl8169_private *tp = netdev_priv(dev); - void __iomem *ioaddr = tp->mmio_addr; - struct device *d = &tp->pci_dev->dev; + rc = rtl_udelay_loop_wait_low(tp, &rtl_cmd_counters_cond, 10, 1000); RTL_W32(CounterAddrLow, 0); RTL_W32(CounterAddrHigh, 0); - dma_free_coherent(d, sizeof(*counters), counters, paddr); + return rc; } -static bool rtl8169_reset_counters(struct net_device *dev) +static int rtl8169_reset_counters(struct net_device *dev) { struct rtl8169_private *tp = netdev_priv(dev); - struct rtl8169_counters *counters; - dma_addr_t paddr; - bool ret = true; /* * Versions prior to RTL_GIGA_MAC_VER_19 don't support resetting the * tally counters. */ if (tp->mac_version < RTL_GIGA_MAC_VER_19) - return true; - - counters = rtl8169_map_counters(dev, &paddr, CounterReset); - if (!counters) - return false; - - if (!rtl_udelay_loop_wait_low(tp, &rtl_cmd_counters_cond, 10, 1000)) - ret = false; - - rtl8169_unmap_counters(dev, paddr, counters); + return -EINVAL; - return ret; + return rtl8169_cmd_counters(dev, CounterReset); } -static bool rtl8169_update_counters(struct net_device *dev) +static int rtl8169_update_counters(struct net_device *dev) { struct rtl8169_private *tp = netdev_priv(dev); void __iomem *ioaddr = tp->mmio_addr; - struct rtl8169_counters *counters; - dma_addr_t paddr; - bool ret = true; /* * Some chips are unable to dump tally counters when the receiver * is disabled. */ if ((RTL_R8(ChipCmd) & CmdRxEnb) == 0) - return true; - - counters = rtl8169_map_counters(dev, &paddr, CounterDump); - if (!counters) - return false; - - if (rtl_udelay_loop_wait_low(tp, &rtl_cmd_counters_cond, 10, 1000)) - memcpy(tp->counters, counters, sizeof(*counters)); - else - ret = false; - - rtl8169_unmap_counters(dev, paddr, counters); + return -EINVAL; - return ret; + return rtl8169_cmd_counters(dev, CounterDump); } -static bool rtl8169_init_counter_offsets(struct net_device *dev) +static int rtl_init_counter_offsets(struct net_device *dev) { struct rtl8169_private *tp = netdev_priv(dev); struct rtl8169_counters *counters = tp->counters; struct rtl8169_tc_offsets *offset = &tp->tc_offset; - bool ret = false; + int rc; /* - * rtl8169_init_counter_offsets is called from rtl_open. On chip + * rtl_init_counter_offsets is called from rtl_open. On chip * versions prior to RTL_GIGA_MAC_VER_19 the tally counters are only * reset by a power cycle, while the counter values collected by the * driver are reset at every driver unload/load cycle. @@ -2303,27 +2267,29 @@ static bool rtl8169_init_counter_offsets(struct net_device *dev) * values, we collect the initial values at first open(*) and use them * as offsets to normalize the values returned by @get_stats64. * - * (*) We can't call rtl8169_init_counter_offsets from rtl_init_one + * (*) We can't call rtl_init_counter_offsets from rtl_init_one * for the reason stated in rtl8169_update_counters; CmdRxEnb is only * set at open time by rtl_hw_start. */ if (offset->inited) - return true; + return 0; /* If both, reset and update fail, propagate to caller. */ - if (rtl8169_reset_counters(dev)) - ret = true; + rc = rtl8169_reset_counters(dev); + if (!rc) + goto out; - if (rtl8169_update_counters(dev)) - ret = true; + rc = rtl8169_update_counters(dev); + if (rc < 0) + goto out; offset->tx_errors = counters->tx_errors; offset->tx_multi_collision = counters->tx_multi_collision; offset->tx_aborted = counters->tx_aborted; offset->inited = true; - - return ret; +out: + return rc; } static void rtl8169_get_ethtool_stats(struct net_device *dev, @@ -7741,7 +7707,8 @@ static int rtl_open(struct net_device *dev) rtl_hw_start(dev); - if (!rtl8169_init_counter_offsets(dev)) + retval = rtl_init_counter_offsets(dev); + if (retval < 0) netif_warn(tp, hw, dev, "counter reset/update failed\n"); netif_start_queue(dev); @@ -8020,8 +7987,6 @@ static void rtl_remove_one(struct pci_dev *pdev) unregister_netdev(dev); - kfree(tp->counters); - rtl_release_firmware(tp); if (pci_dev_run_wake(pdev)) @@ -8031,6 +7996,10 @@ static void rtl_remove_one(struct pci_dev *pdev) rtl_rar_set(tp, dev->perm_addr); rtl_disable_msi(pdev, tp); + + dma_free_coherent(&pdev->dev, sizeof(*tp->counters), tp->counters, + tp->counters_map); + rtl8169_release_board(pdev, dev, tp->mmio_addr); } @@ -8447,7 +8416,8 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) tp->rtl_fw = RTL_FIRMWARE_UNKNOWN; - tp->counters = kmalloc(sizeof(*tp->counters), GFP_KERNEL); + tp->counters = dma_alloc_coherent(&pdev->dev, sizeof(*tp->counters), + &tp->counters_map, GFP_KERNEL); if (!tp->counters) { rc = -ENOMEM; goto err_out_msi_4; @@ -8490,7 +8460,9 @@ out: return rc; err_out_counters_5: - kfree(tp->counters); + dma_free_coherent(&pdev->dev, sizeof(*tp->counters), tp->counters, + tp->counters_map); + tp->counters = NULL; err_out_msi_4: netif_napi_del(&tp->napi); rtl_disable_msi(pdev, tp); -- 2.4.3