Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1337434 > unrolled thread

[PATCH net] r8169:fix "rtl_counters_cond == 1 (loop: 1000, delay: 10)" log spam.

Started byChunhao Lin <hau@realtek.com>
First post2016-02-18 16:00 +0100
Last post2016-02-20 06:00 +0100
Articles 4 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH net] r8169:fix "rtl_counters_cond == 1 (loop: 1000, delay: 10)" log spam. Chunhao Lin <hau@realtek.com> - 2016-02-18 16:00 +0100
    Re: [PATCH net] r8169:fix "rtl_counters_cond == 1 (loop: 1000,  delay: 10)" log spam. Francois Romieu <romieu@fr.zoreil.com> - 2016-02-18 21:40 +0100
      RE: [PATCH net] r8169:fix "rtl_counters_cond == 1 (loop: 1000, delay: 10)" log spam. Hau <hau@realtek.com> - 2016-02-19 12:50 +0100
    Re: [PATCH net] r8169:fix "rtl_counters_cond == 1 (loop: 1000,  delay: 10)" log spam. David Miller <davem@davemloft.net> - 2016-02-20 06:00 +0100

#1337434 — [PATCH net] r8169:fix "rtl_counters_cond == 1 (loop: 1000, delay: 10)" log spam.

FromChunhao Lin <hau@realtek.com>
Date2016-02-18 16:00 +0100
Subject[PATCH net] r8169:fix "rtl_counters_cond == 1 (loop: 1000, delay: 10)" log spam.
Message-ID<r3ydI-77V-3@gated-at.bofh.it>
There will be a log spam when there is no cable plugged.
Please refer to following links.
https://bugzilla.kernel.org/show_bug.cgi?id=104351
https://bugzilla.kernel.org/show_bug.cgi?id=107421

This issue is caused by runtime power management. When there is no cable
plugged, the driver will be suspend (runtime suspend) by OS and NIC will
be put into the D3 state. During this time, if OS call rtl8169_get_stats64()
to dump tally counter, because NIC is in D3 state, the register value read by
driver will return all 0xff. This will let driver think tally counter flag is not
toggled and then sends the warning  message "rtl_counters_cond == 1 (loop: 1000,
delay: 10)" to kernel log.

I add checking driver's pm runtime status in rtl8169_get_stats64() to fix
this issue.

Signed-off-by: Chunhao Lin <hau@realtek.com>
---
 drivers/net/ethernet/realtek/r8169.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 537974c..798b30c 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 rtl8169_counters *counters = tp->counters;
 	unsigned int start;
 
-	if (netif_running(dev))
+	pm_runtime_get_noresume(&pdev->dev);
+
+	if (netif_running(dev) && pm_runtime_active(&pdev->dev))
 		rtl8169_rx_missed(dev, ioaddr);
 
 	do {
@@ -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);
 
 	/*
 	 * Subtract values fetched during initalization.
@@ -7774,6 +7778,8 @@ rtl8169_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
 	stats->tx_aborted_errors = le16_to_cpu(counters->tx_aborted) -
 		le16_to_cpu(tp->tc_offset.tx_aborted);
 
+	pm_runtime_put_noidle(&pdev->dev);
+
 	return stats;
 }
 
-- 
1.9.1

[toc] | [next] | [standalone]


#1337723 — Re: [PATCH net] r8169:fix "rtl_counters_cond == 1 (loop: 1000, delay: 10)" log spam.

FromFrancois Romieu <romieu@fr.zoreil.com>
Date2016-02-18 21:40 +0100
SubjectRe: [PATCH net] r8169:fix "rtl_counters_cond == 1 (loop: 1000, delay: 10)" log spam.
Message-ID<r3DwK-2Fr-3@gated-at.bofh.it>
In reply to#1337434
Chunhao Lin <hau@realtek.com> :
[...]
> I add checking driver's pm runtime status in rtl8169_get_stats64() to fix
> this issue.

Would you consider taking the device out of suspended mode during
rtl8169_get_stats64 to prevent outdated stats ?

-- 
Ueimor

[toc] | [prev] | [next] | [standalone]


#1338073

FromHau <hau@realtek.com>
Date2016-02-19 12:50 +0100
Message-ID<r3RJo-4w0-11@gated-at.bofh.it>
In reply to#1337723
> Chunhao Lin <hau@realtek.com> :
> [...]
> > I add checking driver's pm runtime status in rtl8169_get_stats64() to fix
> > this issue.
> 
> Would you consider taking the device out of suspended mode during
> rtl8169_get_stats64 to prevent outdated stats ?
> 

When in runtime suspend, it mean there is no traffic on NIC, so I did not wake the device during rtl8169_get_stats64().
Maybe we can update tally counter before entering runtime suspend mode.

------Please consider the environment before printing this e-mail.

[toc] | [prev] | [next] | [standalone]


#1338632 — Re: [PATCH net] r8169:fix "rtl_counters_cond == 1 (loop: 1000, delay: 10)" log spam.

FromDavid Miller <davem@davemloft.net>
Date2016-02-20 06:00 +0100
SubjectRe: [PATCH net] r8169:fix "rtl_counters_cond == 1 (loop: 1000, delay: 10)" log spam.
Message-ID<r47Oa-82N-1@gated-at.bofh.it>
In reply to#1337434
From: Chunhao Lin <hau@realtek.com>
Date: Thu, 18 Feb 2016 22:57:07 +0800

> There will be a log spam when there is no cable plugged.
> Please refer to following links.
> https://bugzilla.kernel.org/show_bug.cgi?id=104351
> https://bugzilla.kernel.org/show_bug.cgi?id=107421
> 
> This issue is caused by runtime power management. When there is no cable
> plugged, the driver will be suspend (runtime suspend) by OS and NIC will
> be put into the D3 state. During this time, if OS call rtl8169_get_stats64()
> to dump tally counter, because NIC is in D3 state, the register value read by
> driver will return all 0xff. This will let driver think tally counter flag is not
> toggled and then sends the warning  message "rtl_counters_cond == 1 (loop: 1000,
> delay: 10)" to kernel log.
> 
> I add checking driver's pm runtime status in rtl8169_get_stats64() to fix
> this issue.
> 
> Signed-off-by: Chunhao Lin <hau@realtek.com>

If you are going to do this, then you need to quiesce the device RX/TX
and capture all of the statistics from the chip before suspending it.

Otherwise what we're returning from rtl8169_get_stats64() is inaccurate.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web