LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: szepe@pinerecords.com
Cc: eric.dumazet@gmail.com, fw@strlen.de, romieu@fr.zoreil.com,
	hayeswang@realtek.com, edumazet@google.com, therbert@google.com,
	pupilla@hotmail.com, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org
Subject: Re: 1e918876 breaks r8169 (linux-3.18+)
Date: Sun, 22 Feb 2015 15:57:50 -0500 (EST)	[thread overview]
Message-ID: <20150222.155750.495884608409425947.davem@davemloft.net> (raw)
In-Reply-To: <20150222004151.GA4947@louise.pinerecords.com>

From: Tomas Szepe <szepe@pinerecords.com>
Date: Sun, 22 Feb 2015 01:41:51 +0100

>> > Sure, just did.  Unfortunately, 3.19.0 + 0bec3b70 + this patch results
>> > in a driver that retains the problem.
>> 
>> OK, could you test following patch instead ?
> 
> Yup, but tough luck: 3.19.0 + 0bec3b70 + this patch -> problem present.

I'm reverting the two commits for now, as below.

We can put them back in if we can resolve the problems.

====================
[PATCH] r8169: Revert BQL and xmit_more support.

There are certain regressions which are pointing to
these two commits which we are having a hard time
resolving.  So revert them for now.

Specifically this reverts:

	commit 0bec3b700d106a8b0a34227b2976d1a582f1aab7
	Author: Florian Westphal <fw@strlen.de>
	Date:   Wed Jan 7 10:49:49 2015 +0100

	    r8169: add support for xmit_more

and

	commit 1e918876853aa85435e0f17fd8b4a92dcfff53d6
	Author: Florian Westphal <fw@strlen.de>
	Date:   Wed Oct 1 13:38:03 2014 +0200

	    r8169: add support for Byte Queue Limits

There were some attempts by Eric Dumazet to address some obvious
problems in the TX flow, to see if they would fix the problems,
but none of them seem to help for the regression reporters.

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 drivers/net/ethernet/realtek/r8169.c | 30 +++++++-----------------------
 1 file changed, 7 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index ad0020a..b156092 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -5067,8 +5067,6 @@ static void rtl_hw_reset(struct rtl8169_private *tp)
 	RTL_W8(ChipCmd, CmdReset);
 
 	rtl_udelay_loop_wait_low(tp, &rtl_chipcmd_cond, 100, 100);
-
-	netdev_reset_queue(tp->dev);
 }
 
 static void rtl_request_uncached_firmware(struct rtl8169_private *tp)
@@ -7049,7 +7047,6 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
 	u32 status, len;
 	u32 opts[2];
 	int frags;
-	bool stop_queue;
 
 	if (unlikely(!TX_FRAGS_READY_FOR(tp, skb_shinfo(skb)->nr_frags))) {
 		netif_err(tp, drv, dev, "BUG! Tx Ring full when queue awake!\n");
@@ -7090,8 +7087,6 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
 
 	txd->opts2 = cpu_to_le32(opts[1]);
 
-	netdev_sent_queue(dev, skb->len);
-
 	skb_tx_timestamp(skb);
 
 	/* Force memory writes to complete before releasing descriptor */
@@ -7106,16 +7101,11 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
 
 	tp->cur_tx += frags + 1;
 
-	stop_queue = !TX_FRAGS_READY_FOR(tp, MAX_SKB_FRAGS);
+	RTL_W8(TxPoll, NPQ);
 
-	if (!skb->xmit_more || stop_queue ||
-	    netif_xmit_stopped(netdev_get_tx_queue(dev, 0))) {
-		RTL_W8(TxPoll, NPQ);
-
-		mmiowb();
-	}
+	mmiowb();
 
-	if (stop_queue) {
+	if (!TX_FRAGS_READY_FOR(tp, MAX_SKB_FRAGS)) {
 		/* Avoid wrongly optimistic queue wake-up: rtl_tx thread must
 		 * not miss a ring update when it notices a stopped queue.
 		 */
@@ -7198,7 +7188,6 @@ static void rtl8169_pcierr_interrupt(struct net_device *dev)
 static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp)
 {
 	unsigned int dirty_tx, tx_left;
-	unsigned int bytes_compl = 0, pkts_compl = 0;
 
 	dirty_tx = tp->dirty_tx;
 	smp_rmb();
@@ -7222,8 +7211,10 @@ static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp)
 		rtl8169_unmap_tx_skb(&tp->pci_dev->dev, tx_skb,
 				     tp->TxDescArray + entry);
 		if (status & LastFrag) {
-			pkts_compl++;
-			bytes_compl += tx_skb->skb->len;
+			u64_stats_update_begin(&tp->tx_stats.syncp);
+			tp->tx_stats.packets++;
+			tp->tx_stats.bytes += tx_skb->skb->len;
+			u64_stats_update_end(&tp->tx_stats.syncp);
 			dev_kfree_skb_any(tx_skb->skb);
 			tx_skb->skb = NULL;
 		}
@@ -7232,13 +7223,6 @@ static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp)
 	}
 
 	if (tp->dirty_tx != dirty_tx) {
-		netdev_completed_queue(tp->dev, pkts_compl, bytes_compl);
-
-		u64_stats_update_begin(&tp->tx_stats.syncp);
-		tp->tx_stats.packets += pkts_compl;
-		tp->tx_stats.bytes += bytes_compl;
-		u64_stats_update_end(&tp->tx_stats.syncp);
-
 		tp->dirty_tx = dirty_tx;
 		/* Sync with rtl8169_start_xmit:
 		 * - publish dirty_tx ring index (write barrier)
-- 
2.1.0


  reply	other threads:[~2015-02-22 20:57 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-03 10:08 Tomas Szepe
2015-02-03 10:42 ` Florian Westphal
2015-02-05 16:56   ` Tomáš Szépe
2015-02-06 14:04     ` Tomas Szepe
2015-02-08 12:18       ` Holger Hoffstätte
2015-02-10 15:45   ` Florian Westphal
2015-02-11  9:46     ` Tomáš Szépe
2015-02-21 10:15     ` Tomas Szepe
2015-02-21 10:31       ` Florian Westphal
2015-02-21 10:57         ` Holger Hoffstätte
2015-02-21 16:22         ` Eric Dumazet
2015-02-21 17:46           ` Florian Westphal
2015-02-21 18:09             ` Eric Dumazet
2015-02-21 18:32               ` Florian Westphal
2015-02-21 19:26             ` Tomas Szepe
2015-02-21 19:05           ` Tomas Szepe
2015-02-21 19:54             ` Eric Dumazet
2015-02-22  0:41               ` Tomas Szepe
2015-02-22 20:57                 ` David Miller [this message]
     [not found] <E1YqR6a-0007T7-Dz@feisty.vs19.net>
2015-05-07 19:22 ` Daniel Exner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150222.155750.495884608409425947.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=eric.dumazet@gmail.com \
    --cc=fw@strlen.de \
    --cc=hayeswang@realtek.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pupilla@hotmail.com \
    --cc=romieu@fr.zoreil.com \
    --cc=szepe@pinerecords.com \
    --cc=therbert@google.com \
    --subject='Re: 1e918876 breaks r8169 (linux-3.18+)' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).