From 569a3aff70e880588fe4b3f1622ac60abbeb4a28 Mon Sep 17 00:00:00 2001 From: Prasanna S Panchamukhi Date: Thu, 19 Apr 2012 17:01:00 +0000 Subject: [PATCH 1/2] e1000e: MSI interrupt test failed, using legacy interrupt Following logs where seen on Systems with multiple NICs, while using MSI interrupts as shown below: Feb 16 15:09:32 (none) user.notice kernel: 0000:00:0d.0: lan0_0: NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX Feb 16 15:09:32 (none) user.notice kernel: 0000:40:0d.0: wan0_1: NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX Feb 16 15:09:32 (none) user.notice kernel: 0000:40:0d.0: lan0_1: NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX Feb 16 15:09:32 (none) user.warn kernel: 0000:40:0e.0: wan4_0: MSI interrupt test failed, using legacy interrupt. Feb 16 15:09:32 (none) user.notice kernel: 0000:00:0e.0: wan1_0: NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX Feb 16 15:09:33 (none) user.notice kernel: 0000:00:0e.0: lan1_0: NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX Feb 16 15:09:33 (none) user.notice kernel: 0000:00:0f.0: wan2_0: NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX Feb 16 15:09:33 (none) user.notice kernel: 0000:00:0f.0: lan2_0: NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX Feb 16 15:09:33 (none) user.notice kernel: 0000:40:0a.0: wan3_0: NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX Feb 16 15:09:33 (none) user.notice kernel: 0000:40:0a.0: lan3_0: NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX Feb 16 15:09:34 (none) user.notice kernel: 0000:40:0e.0: lan4_0: NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX Feb 16 15:09:34 (none) user.notice kernel: 0000:40:0f.0: wan5_0: NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX Feb 16 15:09:34 (none) user.notice kernel: 0000:40:0f.0: lan5_0: NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX This patch fixes this problem by increasing the msleep from 50 to 100. Signed-off-by: Prasanna S Panchamukhi Tested-by: Aaron Brown Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/e1000e/netdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c index 19ab2154802c..9520a6ac1f30 100644 --- a/drivers/net/ethernet/intel/e1000e/netdev.c +++ b/drivers/net/ethernet/intel/e1000e/netdev.c @@ -3799,7 +3799,7 @@ static int e1000_test_msi_interrupt(struct e1000_adapter *adapter) /* fire an unusual interrupt on the test handler */ ew32(ICS, E1000_ICS_RXSEQ); e1e_flush(); - msleep(50); + msleep(100); e1000_irq_disable(adapter); From 727c356f4d799b53f94cf8fe43e19d64482348c7 Mon Sep 17 00:00:00 2001 From: Jeff Kirsher Date: Fri, 20 Apr 2012 08:51:45 +0000 Subject: [PATCH 2/2] e1000e: Fix default interrupt throttle rate not set in NIC HW Based on the original patch from Ying Cai This change ensures that the itr/itr_setting adjustment logic is used, even for the default/compiled-in value. Context: When we changed the default InterruptThrottleRate value from default (3 = dynamic mode) to 8000 for example, only adapter->itr_setting (which controls interrupt coalescing mode) was set to 8000, but adapter->itr (which controls the value set in NIC register) was not updated accordingly. So from ethtool, it seemed the interrupt throttling is enabled at 8000 intr/s, but the NIC actually was running in dynamic mode which has lower CPU efficiency especially when throughput is not high. CC: Ying Cai CC: David Decotigny Signed-off-by: Jeff Kirsher Tested-by: Aaron Brown --- drivers/net/ethernet/intel/e1000e/param.c | 99 ++++++++++++----------- 1 file changed, 53 insertions(+), 46 deletions(-) diff --git a/drivers/net/ethernet/intel/e1000e/param.c b/drivers/net/ethernet/intel/e1000e/param.c index ff796e42c3eb..16adeb9418a8 100644 --- a/drivers/net/ethernet/intel/e1000e/param.c +++ b/drivers/net/ethernet/intel/e1000e/param.c @@ -106,7 +106,7 @@ E1000_PARAM(RxAbsIntDelay, "Receive Absolute Interrupt Delay"); /* * Interrupt Throttle Rate (interrupts/sec) * - * Valid Range: 100-100000 (0=off, 1=dynamic, 3=dynamic conservative) + * Valid Range: 100-100000 or one of: 0=off, 1=dynamic, 3=dynamic conservative */ E1000_PARAM(InterruptThrottleRate, "Interrupt Throttling Rate"); #define DEFAULT_ITR 3 @@ -344,53 +344,60 @@ void __devinit e1000e_check_options(struct e1000_adapter *adapter) if (num_InterruptThrottleRate > bd) { adapter->itr = InterruptThrottleRate[bd]; - switch (adapter->itr) { - case 0: - e_info("%s turned off\n", opt.name); - break; - case 1: - e_info("%s set to dynamic mode\n", opt.name); - adapter->itr_setting = adapter->itr; - adapter->itr = 20000; - break; - case 3: - e_info("%s set to dynamic conservative mode\n", - opt.name); - adapter->itr_setting = adapter->itr; - adapter->itr = 20000; - break; - case 4: - e_info("%s set to simplified (2000-8000 ints) " - "mode\n", opt.name); - adapter->itr_setting = 4; - break; - default: - /* - * Save the setting, because the dynamic bits - * change itr. - */ - if (e1000_validate_option(&adapter->itr, &opt, - adapter) && - (adapter->itr == 3)) { - /* - * In case of invalid user value, - * default to conservative mode. - */ - adapter->itr_setting = adapter->itr; - adapter->itr = 20000; - } else { - /* - * Clear the lower two bits because - * they are used as control. - */ - adapter->itr_setting = - adapter->itr & ~3; - } - break; - } + + /* + * Make sure a message is printed for non-special + * values. And in case of an invalid option, display + * warning, use default and got through itr/itr_setting + * adjustment logic below + */ + if ((adapter->itr > 4) && + e1000_validate_option(&adapter->itr, &opt, adapter)) + adapter->itr = opt.def; } else { - adapter->itr_setting = opt.def; + /* + * If no option specified, use default value and go + * through the logic below to adjust itr/itr_setting + */ + adapter->itr = opt.def; + + /* + * Make sure a message is printed for non-special + * default values + */ + if (adapter->itr > 40) + e_info("%s set to default %d\n", opt.name, + adapter->itr); + } + + adapter->itr_setting = adapter->itr; + switch (adapter->itr) { + case 0: + e_info("%s turned off\n", opt.name); + break; + case 1: + e_info("%s set to dynamic mode\n", opt.name); adapter->itr = 20000; + break; + case 3: + e_info("%s set to dynamic conservative mode\n", + opt.name); + adapter->itr = 20000; + break; + case 4: + e_info("%s set to simplified (2000-8000 ints) mode\n", + opt.name); + break; + default: + /* + * Save the setting, because the dynamic bits + * change itr. + * + * Clear the lower two bits because + * they are used as control. + */ + adapter->itr_setting &= ~3; + break; } } { /* Interrupt Mode */