netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] e1000e: Change wthresh to 1 to avoid possible Tx stalls.
@ 2012-06-06  8:43 Hiroaki SHIMODA
  2012-06-06  8:46 ` Jeff Kirsher
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Hiroaki SHIMODA @ 2012-06-06  8:43 UTC (permalink / raw)
  To: jeffrey.t.kirsher, davem; +Cc: denys, eric.dumazet, therbert, netdev

Denys Fedoryshchenko reported Tx stalls on e1000e with BQL enabled.

e1000e has WTHRESH which determines when Tx descripters are written
back and successive Tx interrupts are generated, and setting WTHRESH
to 5 gives efficient bus utilization but this cause possible Tx stalls,
especially on BQL enabled system.

To avoid possible Tx stalls, change WTHRESH to 1.

Reported-by: Denys Fedoryshchenko <denys@visp.net.lb>
Tested-by: Denys Fedoryshchenko <denys@visp.net.lb>
Signed-off-by: Hiroaki SHIMODA <shimoda.hiroaki@gmail.com>
---
 drivers/net/ethernet/intel/e1000e/e1000.h  |    6 +++---
 drivers/net/ethernet/intel/e1000e/netdev.c |    2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h
index 6e6fffb..dc28078 100644
--- a/drivers/net/ethernet/intel/e1000e/e1000.h
+++ b/drivers/net/ethernet/intel/e1000e/e1000.h
@@ -175,13 +175,13 @@ struct e1000_info;
 /*
  * in the case of WTHRESH, it appears at least the 82571/2 hardware
  * writes back 4 descriptors when WTHRESH=5, and 3 descriptors when
- * WTHRESH=4, and since we want 64 bytes at a time written back, set
- * it to 5
+ * WTHRESH=4, so setting 5 gives most efficient bus utilization but
+ * to avoid possible Tx hangs, set it to 1
  */
 #define E1000_TXDCTL_DMA_BURST_ENABLE                          \
 	(E1000_TXDCTL_GRAN | /* set descriptor granularity */  \
 	 E1000_TXDCTL_COUNT_DESC |                             \
-	 (5 << 16) | /* wthresh must be +1 more than desired */\
+	 (1 << 16) | /* wthresh must be +1 more than desired */\
 	 (1 << 8)  | /* hthresh */                             \
 	 0x1f)       /* pthresh */
 
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index a4b0435..b031312 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -2806,7 +2806,7 @@ static void e1000_configure_tx(struct e1000_adapter *adapter)
 		 * set up some performance related parameters to encourage the
 		 * hardware to use the bus more efficiently in bursts, depends
 		 * on the tx_int_delay to be enabled,
-		 * wthresh = 5 ==> burst write a cacheline (64 bytes) at a time
+		 * wthresh = 1 ==> burst write is disabled to consider Tx hangs
 		 * hthresh = 1 ==> prefetch when one or more available
 		 * pthresh = 0x1f ==> prefetch if internal cache 31 or less
 		 * BEWARE: this seems to work but should be considered first if
-- 
1.7.8.6

^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH NET] e1000e: Change wthresh to 1 to avoid possible Tx stalls
@ 2012-10-11  1:34 Jesse Brandeburg
  2012-10-11  3:00 ` David Miller
  0 siblings, 1 reply; 16+ messages in thread
From: Jesse Brandeburg @ 2012-10-11  1:34 UTC (permalink / raw)
  To: netdev; +Cc: Hiroaki SHIMODA, eric.dumazet, therbert, Jesse Brandeburg

From: Hiroaki SHIMODA <shimoda.hiroaki@gmail.com>

This patch originated from Hiroaki SHIMODA but has been modified
by Intel with some minor cleanups and additional commit log text.

Denys Fedoryshchenko and others reported Tx stalls on e1000e with
BQL enabled.  Issue was root caused to hardware delays. They were
introduced because some of the e1000e hardware with transmit
writeback bursting enabled, waits until the driver does an
explict flush OR there are WTHRESH descriptors to write back.

Sometimes the delays in question were on the order of seconds,
causing visible lag for ssh sessions and unacceptable tx
completion latency, especially for BQL enabled kernels.

To avoid possible Tx stalls, change WTHRESH back to 1.

The current plan is to investigate a method for re-enabling
WTHRESH while not harming BQL, but those patches will be later
for net-next if they work.

please enqueue for stable since v3.3 as this bug was introduced in
commit 3f0cfa3bc11e7f00c9994e0f469cbc0e7da7b00c
Author: Tom Herbert <therbert@google.com>
Date:   Mon Nov 28 16:33:16 2011 +0000

    e1000e: Support for byte queue limits

    Changes to e1000e to use byte queue limits.

Reported-by: Denys Fedoryshchenko <denys@visp.net.lb>
Tested-by: Denys Fedoryshchenko <denys@visp.net.lb>
Signed-off-by: Hiroaki SHIMODA <shimoda.hiroaki@gmail.com>
CC: eric.dumazet@gmail.com
CC: therbert@google.com
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---

 drivers/net/ethernet/intel/e1000e/e1000.h  |    6 +++---
 drivers/net/ethernet/intel/e1000e/netdev.c |    2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h
index cb3356c..04668b4 100644
--- a/drivers/net/ethernet/intel/e1000e/e1000.h
+++ b/drivers/net/ethernet/intel/e1000e/e1000.h
@@ -175,13 +175,13 @@ struct e1000_info;
 /*
  * in the case of WTHRESH, it appears at least the 82571/2 hardware
  * writes back 4 descriptors when WTHRESH=5, and 3 descriptors when
- * WTHRESH=4, and since we want 64 bytes at a time written back, set
- * it to 5
+ * WTHRESH=4, so a setting of 5 gives the most efficient bus
+ * utilization but to avoid possible Tx stalls, set it to 1
  */
 #define E1000_TXDCTL_DMA_BURST_ENABLE                          \
 	(E1000_TXDCTL_GRAN | /* set descriptor granularity */  \
 	 E1000_TXDCTL_COUNT_DESC |                             \
-	 (5 << 16) | /* wthresh must be +1 more than desired */\
+	 (1 << 16) | /* wthresh must be +1 more than desired */\
 	 (1 << 8)  | /* hthresh */                             \
 	 0x1f)       /* pthresh */
 
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index de57a2b..f444eb0 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -2831,7 +2831,7 @@ static void e1000_configure_tx(struct e1000_adapter *adapter)
 		 * set up some performance related parameters to encourage the
 		 * hardware to use the bus more efficiently in bursts, depends
 		 * on the tx_int_delay to be enabled,
-		 * wthresh = 5 ==> burst write a cacheline (64 bytes) at a time
+		 * wthresh = 1 ==> burst write is disabled to avoid Tx stalls
 		 * hthresh = 1 ==> prefetch when one or more available
 		 * pthresh = 0x1f ==> prefetch if internal cache 31 or less
 		 * BEWARE: this seems to work but should be considered first if

^ permalink raw reply related	[flat|nested] 16+ messages in thread
end of thread, other threads:[~2012-10-11  3:00 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-06  8:43 [PATCH net] e1000e: Change wthresh to 1 to avoid possible Tx stalls Hiroaki SHIMODA
2012-06-06  8:46 ` Jeff Kirsher
2012-06-06  8:50 ` Eric Dumazet
2012-06-07  0:59 ` Jeff Kirsher
2012-06-07  1:34   ` David Miller
2012-06-07  4:24   ` Eric Dumazet
2012-06-07  4:52     ` Jeff Kirsher
2012-06-07  5:03       ` Eric Dumazet
2012-10-09  0:25       ` Frank Reppin
2012-10-09  6:07         ` Eric Dumazet
2012-10-09 17:36           ` Jesse Brandeburg
2012-10-09 21:02             ` Eric Dumazet
2012-10-09 22:48             ` Andre Tomt
2012-10-10  1:25               ` Frank Reppin
  -- strict thread matches above, loose matches on Subject: below --
2012-10-11  1:34 [PATCH NET] " Jesse Brandeburg
2012-10-11  3:00 ` David Miller

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).